Skip to content
Open
Show file tree
Hide file tree
Changes from all 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
6 changes: 6 additions & 0 deletions .coderabbit.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
reviews:
auto_review:
base_branches:
- main
- code-review
drafts: true
4 changes: 0 additions & 4 deletions NGCHM/WebContent/css/NGCHM.css
Original file line number Diff line number Diff line change
Expand Up @@ -188,10 +188,6 @@ div.paneHeader {
padding: 5px 5px 5px 5px;
}

div.paneHeader.activePane {
/* background-color: #ccccff; */
}

div.paneTitle {
margin: 2px;
display: inline-block;
Expand Down
10 changes: 5 additions & 5 deletions NGCHM/WebContent/javascript/CovariateCommands.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@
output.write(`${UTIL.toTitleCase(axis)} covariates:`);
output.write();
output.indent();
const order = req.heatMap.getAxisCovariateOrder(axis);
const order = req.heatMap.getCovariateOrder(axis);
order.forEach((key) => output.write(key));
output.unindent();
output.write();
Expand Down Expand Up @@ -205,7 +205,7 @@
EXEC.noMoreParams,
function (req, res, next) {
// Check value appropriate for covariate data type.
const order = req.heatMap.getAxisCovariateOrder(req.axis);
const order = req.heatMap.getCovariateOrder(req.axis);
if (order.includes(req.name)) {
throw `${UTIL.toTitleCase(req.axis)} ${req.name} already exists`;
}
Expand Down Expand Up @@ -234,7 +234,7 @@
EXEC.reqCovariate,
reqIndex,
function (req, res) {
const order = req.heatMap.getAxisCovariateOrder(req.axis);
const order = req.heatMap.getCovariateOrder(req.axis);
if (req.index > order.length) {
throw `index must be between zero and the number of covariates, inclusive`;
}
Expand All @@ -253,7 +253,7 @@
order.splice(oldIndex, 1);
const insertIndex = Math.min(req.index, order.length);
order.splice(insertIndex, 0, req.covariateName);
req.heatMap.setAxisCovariateOrder(req.axis, order);
req.heatMap.setCovariateOrder(req.axis, order);
res.output.write(`Reordered ${req.axis} covariates`);
}
]
Expand All @@ -275,7 +275,7 @@
EXEC.getHeatMap,
EXEC.reqAxis,
function (req, res) {
const order = req.heatMap.getAxisCovariateOrder(req.axis);
const order = req.heatMap.getCovariateOrder(req.axis);
res.value = order.slice();
}
]
Expand Down
4 changes: 2 additions & 2 deletions NGCHM/WebContent/javascript/DetailHeatMapEvent.js
Original file line number Diff line number Diff line change
Expand Up @@ -263,7 +263,7 @@
const rowClassBars = heatMap.getRowClassificationData();
const rowClassConfig = heatMap.getRowClassificationConfig();
pixelInfo.rowCovariates = heatMap
.getRowClassificationOrder()
.getCovariateOrder("row")
.filter((key) => rowClassConfig[key].show === "Y")
.map((key) => ({
name: key,
Expand All @@ -272,7 +272,7 @@
const colClassBars = heatMap.getColClassificationData();
const colClassConfig = heatMap.getColClassificationConfig();
pixelInfo.colCovariates = heatMap
.getColClassificationOrder()
.getCovariateOrder("column")
.filter((key) => colClassConfig[key].show === "Y")
.map((key) => ({
name: key,
Expand Down
3 changes: 2 additions & 1 deletion NGCHM/WebContent/javascript/DetailHeatMapManager.js
Original file line number Diff line number Diff line change
Expand Up @@ -335,7 +335,7 @@

// Set a detail panel for heatMap as primary, if possible.
DMM.setPrimaryForHeatmap = function setPrimaryForHeatmap (heatMap) {
const maps = DVW.detailMaps.filter(mapItem => mapItem.heatMap == heatMap);
const maps = DVW.detailMaps.filter(mapItem => mapItem.heatMap === heatMap);
DMM.switchToPrimary(maps.length > 0 ? maps[0] : null);
};

Expand Down Expand Up @@ -622,6 +622,7 @@
document.getElementById("primary_btn" + mapNumber).dataset.version =
isPrimary ? "P" : "S";
PANE.setPaneTitle(loc, "Heat Map Detail");
PANE.setPaneDecor(loc, mapItem.heatMap.decor);
PANE.registerPaneEventHandler(loc.pane, "empty", emptyDetailPane);
PANE.registerPaneEventHandler(loc.pane, "resize", resizeDetailPane);
DET.setDrawDetailTimeout(mapItem, 0, false);
Expand Down
8 changes: 4 additions & 4 deletions NGCHM/WebContent/javascript/DetailHeatMapViews.js
Original file line number Diff line number Diff line change
Expand Up @@ -86,11 +86,11 @@
};

/*********************************************************************************************
* FUNCTION: anyVisible - Return true if any Detail View is visible.
* FUNCTION: anyVisible - Return true if any Detail View is visible for the specified heatMap.
*********************************************************************************************/
DVW.anyVisible = function anyVisible() {
for (let i = 0; i < DVW.detailMaps.length; i++) {
if (DVW.detailMaps[i].isVisible()) {
DVW.anyVisible = function anyVisible(heatMap) {
for (const mapItem of DVW.detailMaps) {
if (mapItem.heatMap === heatMap && mapItem.isVisible()) {
return true;
}
}
Expand Down
138 changes: 47 additions & 91 deletions NGCHM/WebContent/javascript/HeatMapClass.js
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@
this.endColTile = endColTile;
this.numColumnTiles = endColTile - startColTile + 1;
this.totalTiles = (endRowTile - startRowTile + 1) * this.numColumnTiles;
this.tiles = new Array({ length: this.totalTiles });
this.tiles = new Array(this.totalTiles);

// Get hard references to tile data if it's already in the TileCache.
// Set _allTilesAvailable to false if any tile's data is not available.
Expand Down Expand Up @@ -558,8 +558,9 @@
this.currentTileRequests = []; // Tiles we are currently reading
this.pendingTileRequests = []; // Tiles we want to read
this.searchOptions = []; // Search options specific to this heatMap.
this.decor = { hue: null, avatar: null }; // UI decorations for distinguishing one heatMap from another.

this.updatedOnLoad = false;
this.mapUpdatedOnLoad = false;
this.onready = onready;
this.initEventListeners(updateCallbacks); // Initialize this.eventListeners
this.initTileWindows(); // Initialize this.tileWindowListeners and this.tileWindowRefs
Expand Down Expand Up @@ -753,12 +754,6 @@
}
};

HeatMap.prototype.getAxisCovariateOrder = function (axis) {
return MAPREP.isRow(axis)
? this.getRowClassificationOrder()
: this.getColClassificationOrder();
};

HeatMap.prototype.getRowClassificationConfig = function () {
return this.mapConfig.row_configuration.classifications;
};
Expand Down Expand Up @@ -803,7 +798,7 @@
//
HeatMap.prototype.genAllCovars = function* genAllCovars() {
for (const axis of ["row", "col"]) {
const covariates = this.getAxisCovariateOrder(axis);
const covariates = this.getCovariateOrder(axis);
for (const key of covariates) {
yield { axis, key };
}
Expand Down Expand Up @@ -832,59 +827,40 @@
}
}

HeatMap.prototype.getRowClassificationOrder = function (showOnly) {
var rowClassBarsOrder = this.mapConfig.row_configuration.classifications_order;
// If configuration not found, create order from classifications config
if (typeof rowClassBarsOrder === "undefined") {
rowClassBarsOrder = [];
for (key in this.mapConfig.row_configuration.classifications) {
rowClassBarsOrder.push(key);
}
}
// Filter order for ONLY shown bars (if requested)
if (typeof showOnly === "undefined") {
return rowClassBarsOrder;
} else {
const filterRowClassBarsOrder = [];
for (var i = 0; i < rowClassBarsOrder.length; i++) {
var newKey = rowClassBarsOrder[i];
var currConfig = this.mapConfig.row_configuration.classifications[newKey];
if (currConfig.show == "Y") {
filterRowClassBarsOrder.push(newKey);
}
}
return filterRowClassBarsOrder;
}
// Return the order in which the covariates on the specified axis should be
// displayed.
// If showOnly is specified, return only the visible covariates.
HeatMap.prototype.getCovariateOrder = function (axis, showOnly) {
const axisConfig = this.getAxisConfig(axis);
// If barOrder not defined, create order from classifications config.
const barOrder = Array.isArray(axisConfig.classifications_order)
? axisConfig.classifications_order.slice()
: Object.keys(axisConfig.classifications);
// Filter order for only shown bars (if requested)
return showOnly === true || showOnly === "show"
? barOrder.filter((key) => axisConfig.classifications[key].show == "Y")
: barOrder;
};
Comment on lines +830 to 843
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Harden getCovariateOrder: filter stale keys and optionally append new ones.

If classifications_order contains unknown keys (stale config), downstream uses that index into axisConfig.classifications[key] can throw. Sanitize here to prevent runtime errors.

Apply:

-HeatMap.prototype.getCovariateOrder = function (axis, showOnly) {
-  const axisConfig = this.getAxisConfig(axis);
-  // If barOrder not defined, create order from classifications config.
-  const barOrder = Array.isArray(axisConfig.classifications_order)
-    ? axisConfig.classifications_order.slice()
-    : Object.keys(axisConfig.classifications);
-  // Filter order for only shown bars (if requested)
-  return showOnly === true || showOnly === "show"
-    ? barOrder.filter((key) => axisConfig.classifications[key].show == "Y")
-    : barOrder;
-};
+HeatMap.prototype.getCovariateOrder = function (axis, showOnly) {
+  const axisConfig = this.getAxisConfig(axis);
+  const allKeys = Object.keys(axisConfig.classifications);
+  const baseOrder = Array.isArray(axisConfig.classifications_order)
+    ? axisConfig.classifications_order.slice()
+    : allKeys;
+  // Drop unknown/stale keys to avoid undefined lookups.
+  const validOrder = baseOrder.filter((k) => axisConfig.classifications.hasOwnProperty(k));
+  // Ensure newly added covariates are included (append to end, stable behavior).
+  for (const k of allKeys) if (!validOrder.includes(k)) validOrder.push(k);
+  if (showOnly === true || showOnly === "show") {
+    return validOrder.filter((k) => axisConfig.classifications[k] && axisConfig.classifications[k].show == "Y");
+  }
+  return validOrder;
+};
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// Return the order in which the covariates on the specified axis should be
// displayed.
// If showOnly is specified, return only the visible covariates.
HeatMap.prototype.getCovariateOrder = function (axis, showOnly) {
const axisConfig = this.getAxisConfig(axis);
// If barOrder not defined, create order from classifications config.
const barOrder = Array.isArray(axisConfig.classifications_order)
? axisConfig.classifications_order.slice()
: Object.keys(axisConfig.classifications);
// Filter order for only shown bars (if requested)
return showOnly === true || showOnly === "show"
? barOrder.filter((key) => axisConfig.classifications[key].show == "Y")
: barOrder;
};
// Return the order in which the covariates on the specified axis should be
// displayed.
// If showOnly is specified, return only the visible covariates.
HeatMap.prototype.getCovariateOrder = function (axis, showOnly) {
const axisConfig = this.getAxisConfig(axis);
const allKeys = Object.keys(axisConfig.classifications);
const baseOrder = Array.isArray(axisConfig.classifications_order)
? axisConfig.classifications_order.slice()
: allKeys;
// Drop unknown/stale keys to avoid undefined lookups.
const validOrder = baseOrder.filter((k) =>
axisConfig.classifications.hasOwnProperty(k)
);
// Ensure newly added covariates are included (append to end, stable behavior).
for (const k of allKeys) {
if (!validOrder.includes(k)) validOrder.push(k);
}
if (showOnly === true || showOnly === "show") {
return validOrder.filter(
(k) =>
axisConfig.classifications[k] &&
axisConfig.classifications[k].show == "Y"
);
}
return validOrder;
};
🤖 Prompt for AI Agents
In NGCHM/WebContent/javascript/HeatMapClass.js around lines 830 to 843, the
getCovariateOrder function must sanitize classifications_order to avoid stale
keys and ensure new classification keys are included; update the logic to: build
initial order from classifications_order if array else from
Object.keys(classifications), then filter that order to only keep keys that
exist in axisConfig.classifications, then compute any missing keys by taking
Object.keys(axisConfig.classifications) minus the filtered order and append
those missing keys to the end, and finally apply the showOnly filter (if
showOnly === true || showOnly === "show") by keeping only keys whose
axisConfig.classifications[key].show === "Y".


HeatMap.prototype.setRowClassificationOrder = function () {
// Set the order in which the covariates on the specified axis should be
// displayed.
// If order is not specified and there is no existing order, set a
// default order.
HeatMap.prototype.setCovariateOrder = function (axis, order) {
if (this.mapConfig !== null) {
this.mapConfig.row_configuration.classifications_order = this.getRowClassificationOrder();
}
};

HeatMap.prototype.getColClassificationOrder = function (showOnly) {
var colClassBarsOrder = this.mapConfig.col_configuration.classifications_order;
// If configuration not found, create order from classifications config
if (typeof colClassBarsOrder === "undefined") {
colClassBarsOrder = [];
for (key in this.mapConfig.col_configuration.classifications) {
colClassBarsOrder.push(key);
}
}
// Filter order for ONLY shown bars (if requested)
if (typeof showOnly === "undefined") {
return colClassBarsOrder;
} else {
const filterColClassBarsOrder = [];
for (var i = 0; i < colClassBarsOrder.length; i++) {
var newKey = colClassBarsOrder[i];
var currConfig = this.mapConfig.col_configuration.classifications[newKey];
if (currConfig.show == "Y") {
filterColClassBarsOrder.push(newKey);
const axisConfig = this.getAxisConfig(axis);
if (order) {
if (!Array.isArray(order)) {
console.error("setCovariateOrder: if specified, order must be an array", { axis, order });
return;
}
axisConfig.classifications_order = order.slice();
this.setUnAppliedChanges(true);
return;
}
if (typeof axisConfig.classifications_order === "undefined") {
axisConfig.classifications_order = Object.keys(axisConfig.classifications);
}
return filterColClassBarsOrder;
}
};
Comment on lines +845 to 865
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Validate inputs in setCovariateOrder to prevent crashes and maintain invariants.

A bad order (unknown keys, duplicates, missing entries) can break callers that assume every key maps to a config. Sanitize and dedupe.

Apply:

 HeatMap.prototype.setCovariateOrder = function (axis, order) {
   if (this.mapConfig !== null) {
     const axisConfig = this.getAxisConfig(axis);
     if (order) {
       if (!Array.isArray(order)) {
         console.error("setCovariateOrder: if specified, order must be an array", { axis, order });
         return;
       }
-      axisConfig.classifications_order = order.slice();
+      const allKeys = Object.keys(axisConfig.classifications);
+      // Deduplicate and drop unknown keys.
+      const sanitized = [...new Set(order)].filter((k) => allKeys.includes(k));
+      // Append any missing keys to preserve full coverage.
+      for (const k of allKeys) if (!sanitized.includes(k)) sanitized.push(k);
+      axisConfig.classifications_order = sanitized;
       this.setUnAppliedChanges(true);
       return;
     }
     if (typeof axisConfig.classifications_order === "undefined") {
       axisConfig.classifications_order = Object.keys(axisConfig.classifications);
     }
   }
 };
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// Set the order in which the covariates on the specified axis should be
// displayed.
// If order is not specified and there is no existing order, set a
// default order.
HeatMap.prototype.setCovariateOrder = function (axis, order) {
if (this.mapConfig !== null) {
this.mapConfig.row_configuration.classifications_order = this.getRowClassificationOrder();
}
};
HeatMap.prototype.getColClassificationOrder = function (showOnly) {
var colClassBarsOrder = this.mapConfig.col_configuration.classifications_order;
// If configuration not found, create order from classifications config
if (typeof colClassBarsOrder === "undefined") {
colClassBarsOrder = [];
for (key in this.mapConfig.col_configuration.classifications) {
colClassBarsOrder.push(key);
}
}
// Filter order for ONLY shown bars (if requested)
if (typeof showOnly === "undefined") {
return colClassBarsOrder;
} else {
const filterColClassBarsOrder = [];
for (var i = 0; i < colClassBarsOrder.length; i++) {
var newKey = colClassBarsOrder[i];
var currConfig = this.mapConfig.col_configuration.classifications[newKey];
if (currConfig.show == "Y") {
filterColClassBarsOrder.push(newKey);
const axisConfig = this.getAxisConfig(axis);
if (order) {
if (!Array.isArray(order)) {
console.error("setCovariateOrder: if specified, order must be an array", { axis, order });
return;
}
axisConfig.classifications_order = order.slice();
this.setUnAppliedChanges(true);
return;
}
if (typeof axisConfig.classifications_order === "undefined") {
axisConfig.classifications_order = Object.keys(axisConfig.classifications);
}
return filterColClassBarsOrder;
}
};
// Set the order in which the covariates on the specified axis should be
// displayed.
// If order is not specified and there is no existing order, set a
// default order.
HeatMap.prototype.setCovariateOrder = function (axis, order) {
if (this.mapConfig !== null) {
const axisConfig = this.getAxisConfig(axis);
if (order) {
if (!Array.isArray(order)) {
console.error("setCovariateOrder: if specified, order must be an array", { axis, order });
return;
}
// Sanitize the provided order: dedupe, drop unknown keys, then append any missing keys
const allKeys = Object.keys(axisConfig.classifications);
const sanitized = [...new Set(order)].filter((k) => allKeys.includes(k));
for (const k of allKeys) {
if (!sanitized.includes(k)) {
sanitized.push(k);
}
}
axisConfig.classifications_order = sanitized;
this.setUnAppliedChanges(true);
return;
}
if (typeof axisConfig.classifications_order === "undefined") {
axisConfig.classifications_order = Object.keys(axisConfig.classifications);
}
}
};
🤖 Prompt for AI Agents
In NGCHM/WebContent/javascript/HeatMapClass.js around lines 845 to 865,
setCovariateOrder currently accepts any array and can introduce unknown keys,
duplicates or omissions which break callers; update the function to validate and
sanitize the supplied order: ensure order is an array, filter it to only known
classification keys (log an error for unknown keys), remove duplicates while
preserving first-seen order, append any missing classification keys (preserving
existing axisConfig.classifications order) so every key is present exactly once,
then assign the sanitized array to axisConfig.classifications_order and call
this.setUnAppliedChanges(true); keep the early return for non-array inputs and
preserve existing behavior when order is undefined.


Expand Down Expand Up @@ -965,22 +941,6 @@
);
};

HeatMap.prototype.setColClassificationOrder = function () {
if (this.mapConfig !== null) {
this.mapConfig.col_configuration.classifications_order = this.getColClassificationOrder();
}
};

// Persist the axis covariate order and mark the heatmap as changed.
HeatMap.prototype.setAxisCovariateOrder = function (axis, order) {
if (!Array.isArray(order)) {
console.error("setAxisCovariateOrder requires an order array", { axis, order });
return;
}
this.getAxisConfig(axis).classifications_order = order.slice();
this.setUnAppliedChanges(true);
};

HeatMap.prototype.getMapInformation = function () {
return this.mapConfig.data_configuration.map_information;
};
Expand Down Expand Up @@ -1340,66 +1300,62 @@

/*********** Methods for accessing datalevels ****************/

//Return the total number of detail rows
HeatMap.prototype.getTotalRows = function () {
return this.datalevels[MAPREP.DETAIL_LEVEL].totalRows;
};

//Return the summary row ratio
// Return the summary row ratio.
HeatMap.prototype.getSummaryRowRatio = function () {
if (this.datalevels[MAPREP.SUMMARY_LEVEL] !== null) {
if (this.datalevels[MAPREP.SUMMARY_LEVEL]) {
return this.datalevels[MAPREP.SUMMARY_LEVEL].rowSummaryRatio;
} else {
return this.datalevels[MAPREP.THUMBNAIL_LEVEL].rowSummaryRatio;
}
};

//Return the summary row ratio
// Return the summary column ratio.
HeatMap.prototype.getSummaryColRatio = function () {
if (this.datalevels[MAPREP.SUMMARY_LEVEL] !== null) {
if (this.datalevels[MAPREP.SUMMARY_LEVEL]) {
return this.datalevels[MAPREP.SUMMARY_LEVEL].colSummaryRatio;
} else {
return this.datalevels[MAPREP.THUMBNAIL_LEVEL].col_summaryRatio;
return this.datalevels[MAPREP.THUMBNAIL_LEVEL].colSummaryRatio;
}
};
//Return the total number of detail rows

// Return the total number of detail rows.
HeatMap.prototype.getTotalRows = function () {
return this.datalevels[MAPREP.DETAIL_LEVEL].totalRows;
};

//Return the total number of detail rows
// Return the total number of detail columns.
HeatMap.prototype.getTotalCols = function () {
return this.datalevels[MAPREP.DETAIL_LEVEL].totalColumns;
};

//Return the total number of rows/columns on the specified axis.
// Return the total number of rows/columns on the specified axis.
HeatMap.prototype.getTotalElementsForAxis = function (axis) {
const level = this.datalevels[MAPREP.DETAIL_LEVEL];
return MAPREP.isRow(axis) ? level.totalRows : level.totalColumns;
};

//Return the number of rows or columns for the given level
// Return the number of rows or columns for the given level
HeatMap.prototype.getNumAxisElements = function (axis, level) {
const l = this.datalevels[level];
return MAPREP.isRow(axis) ? l.totalRows : l.totalColumns;
};

//Return the number of rows for a given level
// Return the number of rows for a given level.
HeatMap.prototype.getNumRows = function (level) {
return this.datalevels[level].totalRows;
};

//Return the number of columns for a given level
// Return the number of columns for a given level.
HeatMap.prototype.getNumColumns = function (level) {
return this.datalevels[level].totalColumns;
};

//Return the row summary ratio for a given level
// Return the row summary ratio for a given level.
HeatMap.prototype.getRowSummaryRatio = function (level) {
return this.datalevels[level].rowSummaryRatio;
};

//Return the column summary ratio for a given level
// Return the column summary ratio for a given level.
HeatMap.prototype.getColSummaryRatio = function (level) {
return this.datalevels[level].colSummaryRatio;
};
Expand Down
2 changes: 1 addition & 1 deletion NGCHM/WebContent/javascript/HeatMapCommands.js
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@
output.write();
for (const heatMap of MMGR.getAllHeatMaps()) {
const info = heatMap.getMapInformation();
const flag = heatMap == currentHeatmap ? "(*) " : "";
const flag = heatMap === currentHeatmap ? "(*) " : "";
output.write (`${flag}${info.name}`);
}
output.unindent();
Expand Down
Loading