Skip to content
Open
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Gruntfile.js
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ module.exports = function(grunt) {
},
dist_admin: {
src: [
'public/javascripts/Admin/src/util/*.js',
'public/javascripts/Admin/src/*.js',
'public/javascripts/common/UtilitiesSidewalk.js',
'public/javascripts/common/Panomarker.js',
Expand Down
6 changes: 4 additions & 2 deletions app/controllers/AdminController.scala
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,8 @@ class AdminController @Inject() (
"label_type" -> label.labelType,
"severity" -> label.severity,
"correct" -> label.correct,
"high_quality_user" -> label.highQualityUser
"high_quality_user" -> label.highQualityUser,
"ai_generated" -> label.aiGenerated
)
)
}.seq
Expand Down Expand Up @@ -187,7 +188,8 @@ class AdminController @Inject() (
"has_validations" -> label.hasValidations,
"ai_validation" -> label.aiValidation.map(LabelValidationTable.validationOptions.get),
"expired" -> label.expired,
"high_quality_user" -> label.highQualityUser
"high_quality_user" -> label.highQualityUser,
"ai_generated" -> label.aiGenerated
)
)
}.seq
Expand Down
7 changes: 5 additions & 2 deletions app/formats/json/LabelFormats.scala
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,8 @@ object LabelFormats {
(__ \ "validations").write[Map[String, Int]] and
(__ \ "tags").write[List[String]] and
(__ \ "low_quality_incomplete_stale_flags").write[(Boolean, Boolean, Boolean)] and
(__ \ "comments").write[Option[Seq[String]]]
(__ \ "comments").write[Option[Seq[String]]] and
(__ \ "ai_generated").write[Boolean]
)(unlift(LabelMetadata.unapply))

def validationLabelMetadataToJson(
Expand Down Expand Up @@ -95,6 +96,7 @@ object LabelFormats {
"ai_validation" -> labelMetadata.aiValidation.map(LabelValidationTable.validationOptions.get),
"tags" -> labelMetadata.tags,
"ai_tags" -> labelMetadata.aiTags,
"ai_generated" -> labelMetadata.aiGenerated,
"admin_data" -> adminData.map(ad =>
Json.obj(
"username" -> ad.username,
Expand Down Expand Up @@ -133,7 +135,8 @@ object LabelFormats {
"num_disagree" -> labelMetadata.validations("disagree"),
"num_unsure" -> labelMetadata.validations("unsure"),
"comments" -> labelMetadata.comments,
"tags" -> labelMetadata.tags
"tags" -> labelMetadata.tags,
"ai_generated" -> labelMetadata.aiGenerated
)
}

Expand Down
98 changes: 59 additions & 39 deletions app/models/label/LabelTable.scala
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,8 @@ case class LabelForLabelMap(
aiValidation: Option[Int],
expired: Boolean,
highQualityUser: Boolean,
severity: Option[Int]
severity: Option[Int],
aiGenerated: Boolean
)

case class TagCount(labelType: String, tag: String, count: Int)
Expand Down Expand Up @@ -144,7 +145,8 @@ case class LabelMetadata(
validations: Map[String, Int],
tags: List[String],
lowQualityIncompleteStaleFlags: (Boolean, Boolean, Boolean),
comments: Option[Seq[String]]
comments: Option[Seq[String]],
aiGenerated: Boolean
)

// Extra data to include with validations for Admin Validate. Includes usernames and previous validators.
Expand Down Expand Up @@ -226,7 +228,8 @@ case class LabelValidationMetadata(
tags: Seq[String],
cameraLat: Option[Float],
cameraLng: Option[Float],
aiTags: Option[Seq[String]]
aiTags: Option[Seq[String]],
aiGenerated: Boolean
) extends BasicLabelMetadata

class LabelTableDef(tag: slick.lifted.Tag) extends Table[Label](tag, "label") {
Expand Down Expand Up @@ -321,7 +324,8 @@ object LabelTable {
List[String], // tags
Option[Float], // cameraLat
Option[Float], // cameraLng
Option[List[String]] // aiTags
Option[List[String]], // aiTags
Boolean // aiGenerated
)
type LabelValidationMetadataTupleRep = (
Rep[Int], // labelId
Expand All @@ -343,15 +347,16 @@ object LabelTable {
Rep[List[String]], // tags
Rep[Option[Float]], // cameraLat
Rep[Option[Float]], // cameraLng
Rep[Option[List[String]]] // aiTags
Rep[Option[List[String]]], // aiTags
Rep[Boolean] // aiGenerated
)

// Define an implicit conversion from the tuple representation to the case class.
implicit val labelValidationMetadataConverter: TupleConverter[LabelValidationMetadataTuple, LabelValidationMetadata] =
new TupleConverter[LabelValidationMetadataTuple, LabelValidationMetadata] {
def fromTuple(t: LabelValidationMetadataTuple): LabelValidationMetadata = LabelValidationMetadata(
t._1, t._2, t._3, t._4, t._5, t._6.get, t._7.get, POV.tupled(t._8), LocationXY.tupled(t._9), t._10, t._11,
t._12, t._13, LabelValidationInfo.tupled(t._14), t._15, t._16, t._17, t._18, t._19, t._20
t._12, t._13, LabelValidationInfo.tupled(t._14), t._15, t._16, t._17, t._18, t._19, t._20, t._21
)
}

Expand Down Expand Up @@ -486,8 +491,11 @@ class LabelTable @Inject() (

val aiValidations =
labelValidations.join(sidewalkUserTable.aiUsers).on(_.userId === _.userId).map(_._1).distinctOn(_.labelId)
val neighborhoods = regions.filter(_.deleted === false)
val usersWithoutExcluded = usersUnfiltered
private val aiUsersWithRole =
userRoles.join(roleTable).on(_.roleId === _.roleId).filter(_._2.role === "AI").map(_._1.userId)
private def isAiUser(userId: Rep[String]): Rep[Boolean] = aiUsersWithRole.filter(_ === userId).exists
val neighborhoods = regions.filter(_.deleted === false)
val usersWithoutExcluded = usersUnfiltered
.join(userStats)
.on(_.userId === _.userId)
.filterNot(_._2.excluded) // Exclude users with excluded = true
Expand Down Expand Up @@ -552,7 +560,8 @@ class LabelTable @Inject() (
r.nextString().split(',').map(x => x.split(':')).map { y => (y(0), y(1).toInt) }.toMap,
r.nextString().split(",").filter(_.nonEmpty).toList,
(r.nextBoolean(), r.nextBoolean(), r.nextBoolean()),
r.nextStringOption().filter(_.nonEmpty).map(_.split(":").filter(_.nonEmpty).toSeq)
r.nextStringOption().filter(_.nonEmpty).map(_.split(":").filter(_.nonEmpty).toSeq),
r.nextBoolean()
)
}

Expand Down Expand Up @@ -743,7 +752,13 @@ class LabelTable @Inject() (
at.low_quality,
at.incomplete,
at.stale,
comment.comments
comment.comments,
EXISTS (
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that, instead of adding a subquery here, it would be more efficient to just do the joins in the main query. So within the query it would look like

...
INNER JOIN sidewalk_user AS u ON at.user_id = u.user_id
INNER JOIN user_role AS ur ON u.user_id = ur.user_id
INNER JOIN role AS r ON ur.role_id = r.role_id
INNER JOIN label_point AS lp ON lb1.label_id = lp.label_id
...

And then you could just refer to it as r.role = 'AI' rather than doing the subquery in the SELECT statement.

This comment was marked as resolved.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@misaugstad I removed the per-row EXISTS subquery in getRecentLabelsMetadata and pulled the role check into the main query: we now join user_role/role once and select (ai_user.user_id IS NOT NULL) AS ai_generated, using r.role = 'AI' from the join instead of a subquery.

SELECT 1
FROM user_role ur
INNER JOIN role r ON ur.role_id = r.role_id
WHERE ur.user_id = lb1.user_id AND r.role = 'AI'
) AS ai_generated
FROM label AS lb1
INNER JOIN gsv_data ON lb1.gsv_panorama_id = gsv_data.gsv_panorama_id
INNER JOIN audit_task AS at ON lb1.audit_task_id = at.audit_task_id
Expand Down Expand Up @@ -930,7 +945,8 @@ class LabelTable @Inject() (
gd.lng,
// Include AI tags if requested.
if (includeAiTags) la.flatMap(_.tags).getOrElse(List.empty[String].bind).asColumnOf[Option[List[String]]]
else None.asInstanceOf[Option[List[String]]].asColumnOf[Option[List[String]]]
else None.asInstanceOf[Option[List[String]]].asColumnOf[Option[List[String]]],
isAiUser(l.userId)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The same comment above applies to these Slick queries as well. Rather than having this function that essentially creates a subquery for each row, we can do the joins above:

val _labelInfo = for {
      (_lb, _at, _us) <- labelsWithAuditTasksAndUserStats
      _ur <- userRoles if _us.userId === _ur.userId
      _r <- roleTable if _ur.roleId === _r.roleId
      ...

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would apply this to all the queries that you have, even if I didn't explicitly comment on them.

)
}

Expand Down Expand Up @@ -1040,32 +1056,35 @@ class LabelTable @Inject() (

// Join with user validations.
val _userValidations = labelValidations.filter(_.userId === userId)
val _labelInfoWithUserVals = for {
((_lb, _lp, _lt, _gd, _ser, _aiv), _uv) <-
_labelsFilteredByAiValidation.joinLeft(_userValidations).on(_._1.labelId === _.labelId)
} yield (
_lb.labelId,
_lt.labelType,
_lb.gsvPanoramaId,
_gd.captureDate,
_lb.timeCreated,
_lp.lat,
_lp.lng,
(_lp.heading.asColumnOf[Double], _lp.pitch.asColumnOf[Double], _lp.zoom),
(_lp.canvasX, _lp.canvasY),
_lb.severity,
_lb.description,
_lb.streetEdgeId,
_ser.regionId,
(_lb.agreeCount, _lb.disagreeCount, _lb.unsureCount, _lb.correct),
_uv.map(_.validationResult), // userValidation
_aiv.map(_.validationResult), // aiValidation
_lb.tags,
_gd.lat,
_gd.lng,
// Placeholder for AI tags, since we don't show those on Gallery right now.
None.asInstanceOf[Option[List[String]]].asColumnOf[Option[List[String]]]
)
val _labelInfoWithUserVals = _labelsFilteredByAiValidation
.joinLeft(_userValidations)
.on(_._1.labelId === _.labelId)
.map { case ((_lb, _lp, _lt, _gd, _ser, _aiv), _uv) =>
(
_lb.labelId,
_lt.labelType,
_lb.gsvPanoramaId,
_gd.captureDate,
_lb.timeCreated,
_lp.lat,
_lp.lng,
(_lp.heading.asColumnOf[Double], _lp.pitch.asColumnOf[Double], _lp.zoom),
(_lp.canvasX, _lp.canvasY),
_lb.severity,
_lb.description,
_lb.streetEdgeId,
_ser.regionId,
(_lb.agreeCount, _lb.disagreeCount, _lb.unsureCount, _lb.correct),
_uv.map(_.validationResult), // userValidation
_aiv.map(_.validationResult), // aiValidation
_lb.tags,
_gd.lat,
_gd.lng,
// Placeholder for AI tags, since we don't show those on Gallery right now.
None.asInstanceOf[Option[List[String]]].asColumnOf[Option[List[String]]],
isAiUser(_lb.userId)
)
}

// Remove duplicates if needed and randomize.
val rand = SimpleFunction.nullary[Double]("random")
Expand Down Expand Up @@ -1168,7 +1187,8 @@ class LabelTable @Inject() (
query.map { case (_l, _us, _lt, _lp, _gsv, _ser, _aiv) =>
val hasValidations = _l.agreeCount > 0 || _l.disagreeCount > 0 || _l.unsureCount > 0
(_l.labelId, _l.auditTaskId, _lt.labelType, _lp.lat, _lp.lng, _l.correct, hasValidations,
_aiv.map(_.validationResult), _gsv.expired, _us.highQuality, _l.severity, _ser.streetEdgeId)
_aiv.map(_.validationResult), _gsv.expired, _us.highQuality, _l.severity, _ser.streetEdgeId,
isAiUser(_l.userId))
}
}

Expand All @@ -1189,7 +1209,7 @@ class LabelTable @Inject() (
// error, which is why we couldn't use `.tupled` here. This was the error message:
// SlickException: Expected an option type, found Float/REAL
_labelsNearRoute.result.map(
_.map(l => LabelForLabelMap(l._1, l._2, l._3, l._4.get, l._5.get, l._6, l._7, l._8, l._9, l._10, l._11))
_.map(l => LabelForLabelMap(l._1, l._2, l._3, l._4.get, l._5.get, l._6, l._7, l._8, l._9, l._10, l._11, l._13))
)
}

Expand Down
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
9 changes: 8 additions & 1 deletion public/javascripts/Admin/src/Admin.GSVCommentView.js
Original file line number Diff line number Diff line change
Expand Up @@ -45,10 +45,17 @@ function AdminGSVCommentView(admin) {
}

function setLabel(labelMetadata) {
const isAiGenerated = Boolean(labelMetadata['ai_generated']);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think that we should be using Boolean() here... I think that it's meant to coerce anything to a boolean primitive. But if our API is returning something other than a boolean primitive, then we have bigger problems and we shouldn't be hiding it like this.

But that's just based on my cursory googling, since I've never used the Boolean() function before. Did you have a reason for using it..?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@misaugstad I used Boolean() just to normalize potential null/undefined to a boolean when the field was missing, not because I expected non-boolean values. I agree we shouldn’t mask bad data - happy to drop the wrapper and rely on the API returning a real boolean. Let me switch it to use the value directly.

var adminPanoramaLabel = AdminPanoramaLabel(labelMetadata['label_id'], labelMetadata['label_type'],
labelMetadata['canvas_x'], labelMetadata['canvas_y'], util.EXPLORE_CANVAS_WIDTH, util.EXPLORE_CANVAS_HEIGHT,
labelMetadata['heading'], labelMetadata['pitch'], labelMetadata['zoom']);
labelMetadata['heading'], labelMetadata['pitch'], labelMetadata['zoom'], labelMetadata['street_edge_id'],
labelMetadata['severity'], labelMetadata['tags'], isAiGenerated);
self.panorama.setLabel(adminPanoramaLabel);
if (isAiGenerated && self.panorama && self.panorama.labelMarkers && self.panorama.labelMarkers.length) {
const lastMarker = self.panorama.labelMarkers[self.panorama.labelMarkers.length - 1];
const indicator = lastMarker.marker.querySelector('.admin-ai-icon-marker');
ensureAiTooltip(indicator);
}
}

_init();
Expand Down
19 changes: 7 additions & 12 deletions public/javascripts/Admin/src/Admin.GSVLabelView.js
Original file line number Diff line number Diff line change
Expand Up @@ -387,18 +387,10 @@ function AdminGSVLabelView(admin, source) {
if (aiValidation) {
self.modalAiValidation.html(i18next.t('labelmap:ai-val-included', { aiVal: aiValidation.toLowerCase() }));

// Create the AI icon.
let aiIcon = document.createElement('img')
aiIcon.className = 'label-view-ai-icon';
aiIcon.src = '/assets/images/icons/ai-icon-transparent-small.png';
aiIcon.alt = 'AI indicator';
// Create the AI icon with the shared tooltip text.
const aiIcon = AiLabelIndicator(['label-view-ai-icon'], 'top');
self.modalAiValidationHeader.append(aiIcon);

// Create the AI validation tooltip that we show in the header.
aiIcon.setAttribute('data-toggle', 'tooltip');
aiIcon.setAttribute('data-placement', 'top');
aiIcon.setAttribute('title', i18next.t('common:ai-disclaimer', { aiVal: aiValidation.toLowerCase() }));
$(aiIcon).tooltip('hide');
ensureAiTooltip(aiIcon);
} else {
self.modalAiValidation.html(i18next.t('common:none'));
}
Expand Down Expand Up @@ -565,10 +557,11 @@ function AdminGSVLabelView(admin, source) {
self.panorama.setPano(labelMetadata['gsv_panorama_id'], labelMetadata['heading'],
labelMetadata['pitch'], labelMetadata['zoom'], panoCallback);

const isAiGenerated = Boolean(labelMetadata['ai_generated']);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You missed that you used Boolean() here. You should just be able to pass in labelMetadata['ai_generated'] directly to AdminPanoramaLabel() like we do for all the other parameters (you shouldn't need to check if it's equal to true either).

var adminPanoramaLabel = AdminPanoramaLabel(labelMetadata['label_id'], labelMetadata['label_type'],
labelMetadata['canvas_x'], labelMetadata['canvas_y'], util.EXPLORE_CANVAS_WIDTH, util.EXPLORE_CANVAS_HEIGHT,
labelMetadata['heading'], labelMetadata['pitch'], labelMetadata['zoom'], labelMetadata['street_edge_id'],
labelMetadata['severity'], labelMetadata['tags']);
labelMetadata['severity'], labelMetadata['tags'], isAiGenerated);
self.panorama.setLabel(adminPanoramaLabel);

self.validationCounts['Agree'] = labelMetadata['num_agree'];
Expand All @@ -587,6 +580,8 @@ function AdminGSVLabelView(admin, source) {
var imageCaptureDate = moment(new Date(labelMetadata['image_capture_date']));
// Change modal title
self.modalTitle.html(`${i18next.t('labelmap:label-type')}: ${i18next.t('common:' + camelToKebab(labelMetadata['label_type']))}`);
// Remove any previously injected AI header icon and do not add a new one.
self.modalTitle.find('.admin-ai-icon-header').remove();
self.modalSeverity.html(labelMetadata['severity'] != null ? labelMetadata['severity'] : "No severity");
// Create a list of translated tags that's parsable by i18next.
var translatedTags = labelMetadata['tags'].map(tag => i18next.t(`common:tag.${tag.replace(/:/g, '-')}`));
Expand Down
5 changes: 3 additions & 2 deletions public/javascripts/Admin/src/Admin.Panorama.Label.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
* @constructor
*/
function AdminPanoramaLabel(labelId, labelType, canvasX, canvasY, originalCanvasWidth, originalCanvasHeight,
heading, pitch, zoom, streetEdgeId, severity, tags) {
heading, pitch, zoom, streetEdgeId, severity, tags, aiGenerated = false) {
var self = { className: "AdminPanoramaLabel" };

/**
Expand All @@ -37,10 +37,11 @@ function AdminPanoramaLabel(labelId, labelType, canvasX, canvasY, originalCanvas
self.newSeverity = severity;
self.oldTags = tags;
self.newTags = tags;
self.aiGenerated = aiGenerated;
return this;
}

_init();

return self;
}
}
38 changes: 30 additions & 8 deletions public/javascripts/Admin/src/Admin.Panorama.js
Original file line number Diff line number Diff line change
Expand Up @@ -209,17 +209,19 @@ function AdminPanorama(svHolder, buttonHolder, admin) {
var url = icons[label['label_type']];
var pos = getPosition(label['canvasX'], label['canvasY'], label['originalCanvasWidth'],
label['originalCanvasHeight'], label['zoom'], label['heading'], label['pitch']);
const panoMarker = new PanoMarker({
container: self.panoCanvas,
pano: self.panorama,
position: {heading: pos.heading, pitch: pos.pitch},
icon: url,
size: new google.maps.Size(20, 20),
anchor: new google.maps.Point(10, 10)
});
self.labelMarkers.push({
panoId: self.panorama.getPano(),
marker: new PanoMarker({
container: self.panoCanvas,
pano: self.panorama,
position: {heading: pos.heading, pitch: pos.pitch},
icon: url,
size: new google.maps.Size(20, 20),
anchor: new google.maps.Point(10, 10)
})
marker: panoMarker
});
attachAiIndicatorToMarker(panoMarker, label.aiGenerated);
return this;
}

Expand Down Expand Up @@ -275,6 +277,26 @@ function AdminPanorama(svHolder, buttonHolder, admin) {
};
}

function attachAiIndicatorToMarker(panoMarker, showIndicator, retryCount = 0) {
if (!showIndicator) return;

if (!panoMarker.marker_) {
if (retryCount < 5) {
setTimeout(() => attachAiIndicatorToMarker(panoMarker, showIndicator, retryCount + 1), 100);
}
return;
}

if (!panoMarker.marker_.querySelector('.admin-ai-icon-marker')) {
const indicator = AiLabelIndicator(['admin-ai-icon-marker'], 'top', true);
panoMarker.marker_.appendChild(indicator);
const $indicator = ensureAiTooltip(indicator);
// Fall back to parent events in case Google marker swallows child events.
$(panoMarker.marker_).on('mouseenter', () => $indicator.tooltip('show'));
$(panoMarker.marker_).on('mouseleave', () => $indicator.tooltip('hide'));
}
}

/**
* Calculates heading & position for placing this Label onto the pano from the same POV when the label was placed.
* @returns {{heading: number, pitch: number}}
Expand Down
Loading