Feature/dge in browser - #352
Open
STaylorWellcome wants to merge 15 commits into
Open
Conversation
added 7 commits
March 4, 2026 05:13
- Statistical engine: Welford online algorithm, Welch t-test, BH correction - Adaptive effect size: log2FC for log1p data, mean difference for z-scored - DGERunner orchestrator with Web Worker support and batch streaming - dgeIntegration.ts: wires DGERunner to live ChartManager/DataStore - DGEDialogReact: React UI with group/target/reference selection, progress - ChartManager: adds DNA icon for datasources with gene expression links - Validated against Scanpy PBMC3k reference (76 tests, all passing)
…aIsLog1p heuristic only when the metadata is inconclusive.
…atch size to 2000 - Add yield_byte_data() generator to mdvproject.py to stream HDF5 column data using chunked transfer encoding, eliminating ERR_CONTENT_LENGTH_MISMATCH and reducing peak server memory usage - Update /get_data route in server.py to use stream_with_context - Increase default batchSize from 500 to 2000 in DGEDimension.ts, dgeIntegration.ts, and DGEDialogReact.tsx for faster DGE runs on typical datasets
…f log1p-normalized (small non-negative, max ≤ 20), linear/raw counts (non-negative, max > 20), or z-scored (has negatives).
Contributor
📝 WalkthroughWalkthroughAdds a browser-first Differential Gene Expression (DGE) pipeline: pure-statistics core, batched worker processing, DataStore integration and column writes, React/MUI dialog UI, backend streaming support, fixture generation, and extensive unit/integration tests validating PBMC3k references. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant UI as DGEDialogReact
participant CM as ChartManager
participant Integration as dgeIntegration
participant Runner as DGERunner
participant Worker as Web Worker
participant Stats as dgeStats
participant GenesDS as Genes DataStore
User->>UI: choose group, target, reference
UI->>CM: verify availability
UI->>Integration: runDGEOnDataStore(config)
Integration->>Integration: validate datasources & links
Integration->>Integration: load group column, normalize groups
Integration->>Runner: run(config, buffers, loadColumns...)
loop per batch
Runner->>Worker: post DGEBatchInput
Worker->>Stats: computeGeneStats (per gene)
Stats-->>Worker: GeneResult[]
Worker-->>Runner: DGEBatchOutput
Runner->>Integration: onProgress(done,total)
Integration->>UI: update progress
end
Runner->>Stats: benjaminiHochberg(p-values)
Stats-->>Runner: adjusted p-values
Runner-->>Integration: DGERunResult
Integration->>GenesDS: write result columns
Integration-->>UI: DGEIntegrationResult
UI->>CM: optionally add volcano chart
sequenceDiagram
participant Browser as Client
participant Server as Flask
participant HDF5 as HDF5 File
Browser->>Server: GET /data (columns, data_source)
Server->>Server: stream_with_context(yield_byte_data(...))
loop for each column
Server->>HDF5: read column bytes
HDF5-->>Server: bytes
Server-->>Browser: yield chunk
end
Note over Browser,Server: streaming response (no Content-Length)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 15
🧹 Nitpick comments (9)
src/datastore/qcmWorker.ts (1)
65-96: Consider extracting shared logic to reduce duplication.The
processQCMfunction duplicates the computation logic fromonmessage. Consider extracting the core algorithm into a shared helper function.♻️ Optional refactor to reduce duplication
+function runQCMComputation( + assignments: Int32Array, + labels: Uint8Array, + nLabels: number, + nTiles: number, + nPermutations: number, + onProgress?: (done: number, total: number) => void, +): Omit<QCMBatchOutput, "type"> { + const observed = computeCoOccurrence(assignments, labels, nLabels, nTiles); + const sums = new Float64Array(nLabels * nLabels); + const sumsSq = new Float64Array(nLabels * nLabels); + const countsHigher = new Uint32Array(nLabels * nLabels); + + const shuffledLabels = new Uint8Array(labels); + + for (let p = 0; p < nPermutations; p++) { + shuffle(shuffledLabels); + const permObserved = computeCoOccurrence(assignments, shuffledLabels, nLabels, nTiles); + + for (let i = 0; i < nLabels * nLabels; i++) { + const val = permObserved[i]; + sums[i] += val; + sumsSq[i] += val * val; + if (val >= observed[i]) { + countsHigher[i]++; + } + } + onProgress?.(p, nPermutations); + } + + return { observed, sums, sumsSq, countsHigher }; +} + self.onmessage = (e: MessageEvent<QCMBatchInput>) => { const { assignments, labels, nLabels, nTiles, nPermutations } = e.data; - - const observed = computeCoOccurrence(assignments, labels, nLabels, nTiles); - // ... rest of duplicated logic + const result = runQCMComputation( + assignments, labels, nLabels, nTiles, nPermutations, + (done, total) => { + if (done % 10 === 0) { + self.postMessage({ type: "progress", done, total }); + } + } + ); + self.postMessage({ type: "qcm_result", ...result }); }; export function processQCM(input: QCMBatchInput): QCMBatchOutput { const { assignments, labels, nLabels, nTiles, nPermutations } = input; - // ... duplicated logic - return { type: "qcm_result", observed, sums, sumsSq, countsHigher }; + return { type: "qcm_result", ...runQCMComputation(assignments, labels, nLabels, nTiles, nPermutations) }; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/datastore/qcmWorker.ts` around lines 65 - 96, processQCM duplicates the permutation loop and accumulation logic that also appears in the worker's onmessage handler; extract the shared core algorithm into a helper (e.g., computePermutationStats or accumulatePermutationResults) that takes assignments, labels (or shuffledLabels), nLabels, nTiles, nPermutations and returns {observed, sums, sumsSq, countsHigher}, then call that helper from both processQCM and onmessage; reuse existing computeCoOccurrence and shuffle inside the new helper and keep function signatures (processQCM, onmessage) intact so callers only replace the duplicated block with a call to the new helper.src/tests/dge-pbmc3k.spec.ts (1)
78-88: MissingdataTypeargument incomputeGeneStatscall.The
computeGeneStatsfunction accepts an optional 7th argument fordataType. While it may have a default value, for clarity and consistency with other test files, consider explicitly passing"log1p"since the PBMC3k fixture uses log1p-normalized data.♻️ Suggested improvement for clarity
return computeGeneStats( gene, values, groupAssignments, filterArray, targetGroupIdx, -1, // "rest" + "log1p", );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/tests/dge-pbmc3k.spec.ts` around lines 78 - 88, The call to computeGeneStats inside the genes.map callback omits the optional 7th dataType argument; update the invocation in the genes.map block so it passes the explicit dataType string "log1p" as the seventh parameter (after targetGroupIdx and the "-1" rest indicator) to match the PBMC3k fixture; this affects the computeGeneStats call that uses gene, new Float32Array(expression[gene]), groupAssignments, filterArray, targetGroupIdx, -1, and should become the same but with "log1p" appended as the final argument.src/datastore/DGEDimension.ts (2)
100-100: Minor formatting: extra indentation.Line 100 has inconsistent indentation (extra tab/spaces).
🔧 Fix indentation
- const allBatchResults: GeneResult[] = []; + const allBatchResults: GeneResult[] = [];🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/datastore/DGEDimension.ts` at line 100, In DGEDimension, the declaration of the local variable allBatchResults (const allBatchResults: GeneResult[] = [];) has extra indentation; fix the formatting by aligning the line with the surrounding block scope indentation (remove the extra tab/spaces) so the declaration matches the file's indentation style and surrounding statements.
150-162: Consider gating first-batch diagnostic logging.This logging is helpful for debugging but verbose for production use.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/datastore/DGEDimension.ts` around lines 150 - 162, The diagnostic console logging inside DGEDimension (the block that runs when b === 0 and iterates batchResult.results to print top5, sigCount, etc.) is noisy; gate it behind a configurable debug flag or logger level (e.g., check an env var like DEBUG_DGE or use an existing processLogger.isDebugEnabled()) so these messages only emit in debug mode; update the condition from if (b === 0) to if (b === 0 && debugEnabled) (or wrap the console.log calls in a debug-only branch) and replace console.log with the project logger where available to respect log levels.src/datastore/dgeIntegration.ts (1)
102-118: Consider gating diagnostic logging behind a debug flag.This extensive logging is helpful for development but adds noise in production. Consider using a debug flag or removing before release.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/datastore/dgeIntegration.ts` around lines 102 - 118, The diagnostic console.log blocks (using groupValues, targetIdx, refIdx, groupCounts, filterArr, filteredCount and the various "[DGE diag]" messages) should be gated behind a debug flag or logger level; update the code around these logs to check a config or logger.isDebug/isVerbose before emitting them (or replace console.log calls with processLogger.debug/processLogger.trace) so diagnostics only print when debug diagnostics are enabled.src/datastore/QCMStats.ts (1)
43-66: Document the filter convention.The filter array interpretation (
filter[i] !== 0means filtered out) is opposite to some common conventions. Consider adding a brief doc comment for clarity.📝 Documentation suggestion
/** * Assign each cell to a quadrat index. Returns -1 if cell is filtered or outside bbox. + * `@param` filter Uint8Array where 0 = visible/included, non-zero = filtered out */🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/datastore/QCMStats.ts` around lines 43 - 66, The function assignToQuadrats uses the filter Uint8Array with the convention that filter[i] !== 0 means the point is filtered out, which is non-obvious; add a short doc comment above the assignToQuadrats declaration that clearly states the filter convention (e.g., "filter[i] !== 0 => exclude point, 0 => include point"), and optionally document expected value range/type of filter entries and that indices with excluded points will remain -1 in the returned Int32Array; reference the assignToQuadrats function and the filter parameter in the comment so future readers understand the intent.src/charts/dialogs/DGEDialogReact.tsx (3)
128-130: Empty catch block silently swallows errors.While the volcano plot is marked as optional, completely silencing errors makes debugging difficult. Consider logging the error at a debug/warn level.
🔧 Suggested improvement
} catch { - // Volcano plot is optional; don't fail the whole operation + // Volcano plot is optional; don't fail the whole operation + console.debug("[DGE] Volcano chart creation failed (non-fatal)"); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/charts/dialogs/DGEDialogReact.tsx` around lines 128 - 130, The empty catch in the try/catch around the volcano-plot generation silently swallows errors; update that catch to log the caught error (e.g., console.warn or your app logger) with context like "Volcano plot generation failed" while still allowing the operation to continue; locate the try/catch around the volcano plot in DGEDialogReact (the block that currently has "Volcano plot is optional; don't fail the whole operation") and replace the empty catch with a call that logs the error object and a short message.
76-78: Consider removing verbose debug logging for production.These
console.logstatements are useful during development but should be removed or gated behind a debug flag before merging.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/charts/dialogs/DGEDialogReact.tsx` around lines 76 - 78, The three console.log calls in DGEDialogReact (the "=== DGE RUN START ===", "groupColumn... target... reference..." and "dataStore... size..." lines) are verbose debug output; remove them or guard them behind a debug flag before merging — e.g., replace with a conditional that checks a debug prop or an env flag (process.env.NODE_ENV !== 'production' or a DEBUG_DGE boolean) or route them to a proper logger at debug level (logger.debug) inside the DGEDialogReact component or the function where they appear so production builds do not emit these logs.
103-118: Remove or gate diagnostic logging.This debug logging block iterates through DGE columns and logs detailed statistics. Consider removing this for production or wrapping it in a debug flag.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/charts/dialogs/DGEDialogReact.tsx` around lines 103 - 118, The diagnostic console logging in the DGE column verification block (references: cm.dsIndex, pair.genesDsName, gDS, column names "dge_effect_size" and "dge_neg_log10_pval_adj") should be removed or gated behind a debug flag; update the code to either delete these console.error/console.log calls or wrap them with a runtime check (e.g., a DEBUG constant or NODE_ENV check) or replace them with a debug-level logger (logger.debug) so they only run when debugging is enabled.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@python/mdvtools/server.py`:
- Around line 202-206: Pre-validate the stream inputs before creating the
Response: resolve and validate data["columns"] and data["data_source"] (e.g.,
call the same resolution/lookup logic that yield_byte_data uses or iterate
columns and assert they exist via the project's column/subgroup lookup methods)
inside a try/except so any missing column/subgroup or bad data_source raises a
handled error and returns an appropriate error response; only once validation
succeeds construct Response(stream_with_context(project.yield_byte_data(...)),
...). Ensure you reference and reuse the resolution routines used by
project.yield_byte_data to avoid duplicating inconsistent logic.
In `@python/mdvtools/tests/generate_dge_reference.py`:
- Around line 28-29: The call to pandas.DataFrame.sort_values in
generate_dge_reference.py is using a positional column name which Pyright flags;
update the call on the DataFrame gdf (result[result["group"] ==
group_name].copy()) to use the keyword argument by (i.e., change
gdf.sort_values("pvals") to gdf.sort_values(by="pvals")) so the overload matches
and the CI error is resolved.
In `@src/charts/dialogs/DGEDialogReact.tsx`:
- Around line 81-91: Replace direct global access to window.mdv?.chartManager in
DGEDialogReact with the useChartManager hook: import useChartManager from
src/react/hooks.ts, call const cm = useChartManager() where
window.mdv?.chartManager was used, keep the existing null check (if (!cm) throw
new Error("ChartManager not available")), then proceed to call
findDGECapableDatasources(cm) and runDGEOnDataStore(cm, ... ) as before; also
remove the debug console.log statements that print "DGE-capable pairs:" and any
other console.log lines in this block.
In `@src/datastore/dgeIntegration.ts`:
- Around line 59-71: The conversion from a Uint16Array group column to a
Uint8Array (in the block that references nCells, groupSAB, groupArr, and
groupCol.datatype === "text16") can silently truncate values >255; add
validation and a safe fallback: when groupCol.datatype === "text16" iterate the
src = new Uint16Array(groupCol.buffer) and check if any src[i] > 255, and if so
either (a) allocate a Uint16Array SharedArrayBuffer and use that throughout
downstream DGE worker code (propagate use of a 16-bit group buffer), or (b)
throw a descriptive error/report (e.g., "group column contains values >255;
cannot convert to Uint8Array") so the caller can remap/group IDs; ensure no
silent truncation occurs and update any downstream consumers to expect the
chosen buffer type.
In `@src/datastore/dgeStats.ts`:
- Around line 196-202: The test call to log2FoldChange is missing the required
third boolean parameter; update the call where mdvLog2FC is computed (currently
passing statsTarget.mean, statsRef.mean) to include the isLog1p flag (true or
false based on whether input means are log1p-transformed). Locate the call to
log2FoldChange(statsTarget.mean, statsRef.mean) and change it to pass the
appropriate boolean (e.g., isLog1p or a literal true/false) so the signature
log2FoldChange(meanTarget, meanReference, isLog1p) is satisfied.
In `@src/datastore/QCMDimension.ts`:
- Around line 1-2: Remove the stray placeholder export and comment—delete the
"// test lowercase" line and the "export const X = 1;" declaration from
QCMDimension.ts so you don't expose a meaningless public symbol; if the value
was intended for internal tests, either move it into a test file or convert it
to a non-exported local constant used only where needed (search for references
to X before removal).
In `@src/tests/dge-pbmc3k.spec.ts`:
- Line 259: The call to log2FoldChange is missing the required third parameter
isLog1p; update the invocation where mdvLog2FC is assigned so it calls
log2FoldChange(statsTarget.mean, statsRef.mean, true) because PBMC3k uses
log1p-normalized values—locate the mdvLog2FC assignment and add the boolean true
as the third argument to log2FoldChange.
In `@src/tests/dge-worker.spec.ts`:
- Around line 243-253: The test is passing the deprecated flag dataIsLog1p to
processBatch; update the call to use the new dataType parameter by replacing
dataIsLog1p: true with dataType: "log1p" so processBatch receives the expected
enum/string type (locate the call to processBatch in the test and change the
argument name and value accordingly).
- Around line 128-138: The test is using the old flag dataIsLog1p; update the
call to processBatch to use the new dataType parameter instead by replacing
dataIsLog1p: true with dataType: "log1p" so processBatch receives the expected
enum/string identifier (locate the processBatch invocation in dge-worker.spec.ts
and update the argument key/value accordingly).
- Around line 201-211: The test invokes processBatch with the deprecated boolean
flag dataIsLog1p; update the call in this spec so it passes the new dataType
field instead (replace dataIsLog1p: true with dataType: "log1p") in the object
passed to processBatch (located in the test near the block using
geneBuffers/geneNames/batchIndex); ensure the key name and string value match
the runtime API so the test aligns with the updated processBatch signature.
- Around line 99-109: The call to processBatch is using the deprecated boolean
parameter dataIsLog1p; update the invocation to pass the unified dataType string
instead — replace dataIsLog1p: true with dataType: "log1p" in the object passed
to processBatch (the run_batch payload) so processBatch receives dataType and
not dataIsLog1p.
- Around line 171-181: The test invokes processBatch with the deprecated flag
dataIsLog1p; update the call to use dataType: "log1p" instead (remove
dataIsLog1p and add dataType: "log1p") so the processBatch signature is
satisfied. Search for usages in this spec (the processBatch invocation around
the variable result1) and replace any other occurrences of dataIsLog1p with
dataType: "log1p" to keep the tests consistent with the new API.
- Around line 274-284: The call to processBatch uses the deprecated boolean flag
dataIsLog1p; update the argument object to use dataType: "log1p" instead of
dataIsLog1p to match the new API. Locate the processBatch invocation in the test
(the block passing type: "run_batch", filterBuffer, groupBuffer, targetGroup,
referenceGroup, geneBuffers, geneNames, batchIndex) and remove dataIsLog1p: true
and add dataType: "log1p" so the test reflects the current signature.
- Around line 157-167: The test call to processBatch uses the deprecated boolean
flag dataIsLog1p; replace it with the new dataType option by changing the
argument from dataIsLog1p: true to dataType: "log1p" in the processBatch
invocation (look for the processBatch call that creates result0 and update the
object literal accordingly), ensuring other tests use the same dataType pattern
if present.
- Around line 61-71: The test calls to processBatch are using the wrong property
name; DGEBatchInput expects dataType: "log1p" | "linear" | "zscored" not
dataIsLog1p: boolean. Fix each processBatch invocation (and any other test
fixtures creating a DGEBatchInput) by replacing dataIsLog1p: true/false with
dataType: "log1p" (or the appropriate string value) so the object shape matches
DGEBatchInput; update all other test cases that currently use dataIsLog1p to the
new dataType property.
---
Nitpick comments:
In `@src/charts/dialogs/DGEDialogReact.tsx`:
- Around line 128-130: The empty catch in the try/catch around the volcano-plot
generation silently swallows errors; update that catch to log the caught error
(e.g., console.warn or your app logger) with context like "Volcano plot
generation failed" while still allowing the operation to continue; locate the
try/catch around the volcano plot in DGEDialogReact (the block that currently
has "Volcano plot is optional; don't fail the whole operation") and replace the
empty catch with a call that logs the error object and a short message.
- Around line 76-78: The three console.log calls in DGEDialogReact (the "=== DGE
RUN START ===", "groupColumn... target... reference..." and "dataStore...
size..." lines) are verbose debug output; remove them or guard them behind a
debug flag before merging — e.g., replace with a conditional that checks a debug
prop or an env flag (process.env.NODE_ENV !== 'production' or a DEBUG_DGE
boolean) or route them to a proper logger at debug level (logger.debug) inside
the DGEDialogReact component or the function where they appear so production
builds do not emit these logs.
- Around line 103-118: The diagnostic console logging in the DGE column
verification block (references: cm.dsIndex, pair.genesDsName, gDS, column names
"dge_effect_size" and "dge_neg_log10_pval_adj") should be removed or gated
behind a debug flag; update the code to either delete these
console.error/console.log calls or wrap them with a runtime check (e.g., a DEBUG
constant or NODE_ENV check) or replace them with a debug-level logger
(logger.debug) so they only run when debugging is enabled.
In `@src/datastore/DGEDimension.ts`:
- Line 100: In DGEDimension, the declaration of the local variable
allBatchResults (const allBatchResults: GeneResult[] = [];) has extra
indentation; fix the formatting by aligning the line with the surrounding block
scope indentation (remove the extra tab/spaces) so the declaration matches the
file's indentation style and surrounding statements.
- Around line 150-162: The diagnostic console logging inside DGEDimension (the
block that runs when b === 0 and iterates batchResult.results to print top5,
sigCount, etc.) is noisy; gate it behind a configurable debug flag or logger
level (e.g., check an env var like DEBUG_DGE or use an existing
processLogger.isDebugEnabled()) so these messages only emit in debug mode;
update the condition from if (b === 0) to if (b === 0 && debugEnabled) (or wrap
the console.log calls in a debug-only branch) and replace console.log with the
project logger where available to respect log levels.
In `@src/datastore/dgeIntegration.ts`:
- Around line 102-118: The diagnostic console.log blocks (using groupValues,
targetIdx, refIdx, groupCounts, filterArr, filteredCount and the various "[DGE
diag]" messages) should be gated behind a debug flag or logger level; update the
code around these logs to check a config or logger.isDebug/isVerbose before
emitting them (or replace console.log calls with
processLogger.debug/processLogger.trace) so diagnostics only print when debug
diagnostics are enabled.
In `@src/datastore/QCMStats.ts`:
- Around line 43-66: The function assignToQuadrats uses the filter Uint8Array
with the convention that filter[i] !== 0 means the point is filtered out, which
is non-obvious; add a short doc comment above the assignToQuadrats declaration
that clearly states the filter convention (e.g., "filter[i] !== 0 => exclude
point, 0 => include point"), and optionally document expected value range/type
of filter entries and that indices with excluded points will remain -1 in the
returned Int32Array; reference the assignToQuadrats function and the filter
parameter in the comment so future readers understand the intent.
In `@src/datastore/qcmWorker.ts`:
- Around line 65-96: processQCM duplicates the permutation loop and accumulation
logic that also appears in the worker's onmessage handler; extract the shared
core algorithm into a helper (e.g., computePermutationStats or
accumulatePermutationResults) that takes assignments, labels (or
shuffledLabels), nLabels, nTiles, nPermutations and returns {observed, sums,
sumsSq, countsHigher}, then call that helper from both processQCM and onmessage;
reuse existing computeCoOccurrence and shuffle inside the new helper and keep
function signatures (processQCM, onmessage) intact so callers only replace the
duplicated block with a call to the new helper.
In `@src/tests/dge-pbmc3k.spec.ts`:
- Around line 78-88: The call to computeGeneStats inside the genes.map callback
omits the optional 7th dataType argument; update the invocation in the genes.map
block so it passes the explicit dataType string "log1p" as the seventh parameter
(after targetGroupIdx and the "-1" rest indicator) to match the PBMC3k fixture;
this affects the computeGeneStats call that uses gene, new
Float32Array(expression[gene]), groupAssignments, filterArray, targetGroupIdx,
-1, and should become the same but with "log1p" appended as the final argument.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 134f992e-b2cb-4eba-992c-f67857dff2f3
📒 Files selected for processing (23)
DGE_README.mdpython/mdvtools/mdvproject.pypython/mdvtools/server.pypython/mdvtools/tests/generate_dge_reference.pysrc/charts/ChartManager.jssrc/charts/dialogs/DGEDialogReact.tsxsrc/datastore/DGEDimension.tssrc/datastore/QCMDimension.tssrc/datastore/QCMStats.tssrc/datastore/dgeIntegration.tssrc/datastore/dgeStats.tssrc/datastore/dgeWorker.tssrc/datastore/qcmWorker.tssrc/lib/constants.tssrc/tests/dge-integration.spec.tssrc/tests/dge-pbmc3k.spec.tssrc/tests/dge-stats.spec.tssrc/tests/dge-worker.spec.tssrc/tests/fixtures/dge_reference_ttest.jsonsrc/tests/fixtures/dge_reference_ttest_small.jsonsrc/tests/fixtures/dge_reference_wilcoxon.jsonsrc/tests/fixtures/dge_reference_wilcoxon_small.jsonsrc/tests/fixtures/dge_validation_data.json
Comment on lines
+202
to
+206
| # Use streaming to avoid large memory allocations and Content-Length mismatches | ||
| return Response( | ||
| stream_with_context(project.yield_byte_data(data["columns"], data["data_source"])), | ||
| mimetype="application/octet-stream" | ||
| ) |
Contributor
There was a problem hiding this comment.
Validate the stream inputs before returning the response.
After the first chunk is yielded, exceptions from project.yield_byte_data(...) won't be caught by this try/except, so a missing column/subgroup can become a truncated 200 OK octet-stream instead of a clean error. Please pre-resolve/validate the requested handles before constructing the Response.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@python/mdvtools/server.py` around lines 202 - 206, Pre-validate the stream
inputs before creating the Response: resolve and validate data["columns"] and
data["data_source"] (e.g., call the same resolution/lookup logic that
yield_byte_data uses or iterate columns and assert they exist via the project's
column/subgroup lookup methods) inside a try/except so any missing
column/subgroup or bad data_source raises a handled error and returns an
appropriate error response; only once validation succeeds construct
Response(stream_with_context(project.yield_byte_data(...)), ...). Ensure you
reference and reuse the resolution routines used by project.yield_byte_data to
avoid duplicating inconsistent logic.
Comment on lines
+28
to
+29
| gdf = result[result["group"] == group_name].copy() | ||
| gdf = gdf.sort_values("pvals") |
Contributor
There was a problem hiding this comment.
Pipeline failure: sort_values requires by= keyword argument.
Pyright cannot match the overload because pandas' sort_values expects the by keyword for column names. This is causing the CI failure.
🐛 Fix for the sort_values call
gdf = result[result["group"] == group_name].copy()
- gdf = gdf.sort_values("pvals")
+ gdf = gdf.sort_values(by="pvals")📝 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
| gdf = result[result["group"] == group_name].copy() | |
| gdf = gdf.sort_values("pvals") | |
| gdf = result[result["group"] == group_name].copy() | |
| gdf = gdf.sort_values(by="pvals") |
🧰 Tools
🪛 GitHub Actions: Python CI
[error] 29-29: pyright: No overloads for 'sort_values' match the provided arguments. Possibly an incorrect call signature (e.g., mismatched parameter types or missing keyword).
🪛 GitHub Check: pyright (3.12, 1.1.402)
[failure] 29-29:
No overloads for "sort_values" match the provided arguments
Argument types: (Literal['pvals']) (reportCallIssue)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@python/mdvtools/tests/generate_dge_reference.py` around lines 28 - 29, The
call to pandas.DataFrame.sort_values in generate_dge_reference.py is using a
positional column name which Pyright flags; update the call on the DataFrame gdf
(result[result["group"] == group_name].copy()) to use the keyword argument by
(i.e., change gdf.sort_values("pvals") to gdf.sort_values(by="pvals")) so the
overload matches and the CI error is resolved.
Comment on lines
+59
to
+71
| // Normalize group assignments into a Uint8Array SharedArrayBuffer. | ||
| // Text columns use Uint8Array, text16 columns use Uint16Array — we need | ||
| // a consistent Uint8Array for the DGE worker regardless of source type. | ||
| const nCells = cellsDs.size as number; | ||
| const groupSAB = new SharedArrayBuffer(nCells); | ||
| const groupArr = new Uint8Array(groupSAB); | ||
| if (groupCol.datatype === "text16") { | ||
| const src = new Uint16Array(groupCol.buffer); | ||
| for (let i = 0; i < nCells; i++) groupArr[i] = src[i]; | ||
| } else { | ||
| const src = new Uint8Array(groupCol.buffer); | ||
| groupArr.set(src); | ||
| } |
Contributor
There was a problem hiding this comment.
Risk of data loss when converting text16 columns to Uint8Array.
If the group column has more than 256 unique values (text16 supports up to 65536), the conversion src[i] → groupArr[i] will silently truncate values, causing incorrect group assignments.
🛡️ Suggested validation
const groupSAB = new SharedArrayBuffer(nCells);
const groupArr = new Uint8Array(groupSAB);
if (groupCol.datatype === "text16") {
+ if (groupCol.values.length > 255) {
+ throw new Error(`Group column "${config.groupColumn}" has ${groupCol.values.length} values, exceeding 255 limit for DGE`);
+ }
const src = new Uint16Array(groupCol.buffer);
for (let i = 0; i < nCells; i++) groupArr[i] = src[i];🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/datastore/dgeIntegration.ts` around lines 59 - 71, The conversion from a
Uint16Array group column to a Uint8Array (in the block that references nCells,
groupSAB, groupArr, and groupCol.datatype === "text16") can silently truncate
values >255; add validation and a safe fallback: when groupCol.datatype ===
"text16" iterate the src = new Uint16Array(groupCol.buffer) and check if any
src[i] > 255, and if so either (a) allocate a Uint16Array SharedArrayBuffer and
use that throughout downstream DGE worker code (propagate use of a 16-bit group
buffer), or (b) throw a descriptive error/report (e.g., "group column contains
values >255; cannot convert to Uint8Array") so the caller can remap/group IDs;
ensure no silent truncation occurs and update any downstream consumers to expect
the chosen buffer type.
Comment on lines
+196
to
+202
| export function log2FoldChange(meanTarget: number, meanReference: number, isLog1p: boolean): number { | ||
| const valT = isLog1p ? Math.expm1(meanTarget) : meanTarget; | ||
| const valR = isLog1p ? Math.expm1(meanReference) : meanReference; | ||
| const numFC = Math.max(0, valT) + PSEUDOCOUNT; | ||
| const denFC = Math.max(0, valR) + PSEUDOCOUNT; | ||
| return Math.log2(numFC / denFC); | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
rg -n "log2FoldChange\(" --type ts -A 1 -B 1Repository: Taylor-CCB-Group/MDV
Length of output: 2685
Fix missing argument at line 259 in src/tests/dge-pbmc3k.spec.ts.
The call to log2FoldChange is missing the required isLog1p boolean argument. Add the third argument to match the function signature defined at line 196 in dgeStats.ts.
Call site with missing argument
src/tests/dge-pbmc3k.spec.ts:259
const mdvLog2FC = log2FoldChange(statsTarget.mean, statsRef.mean);
Should include the isLog1p argument:
const mdvLog2FC = log2FoldChange(statsTarget.mean, statsRef.mean, isLog1pFlag);
(Replace isLog1pFlag with the appropriate boolean value based on the data context.)
🧰 Tools
🪛 GitHub Actions: TypeScript/JS CI
[error] 196-196: An argument for 'isLog1p' was not provided.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/datastore/dgeStats.ts` around lines 196 - 202, The test call to
log2FoldChange is missing the required third boolean parameter; update the call
where mdvLog2FC is computed (currently passing statsTarget.mean, statsRef.mean)
to include the isLog1p flag (true or false based on whether input means are
log1p-transformed). Locate the call to log2FoldChange(statsTarget.mean,
statsRef.mean) and change it to pass the appropriate boolean (e.g., isLog1p or a
literal true/false) so the signature log2FoldChange(meanTarget, meanReference,
isLog1p) is satisfied.
Comment on lines
+157
to
+167
| const result0 = processBatch({ | ||
| type: "run_batch", | ||
| filterBuffer, | ||
| groupBuffer, | ||
| targetGroup: 0, | ||
| referenceGroup: -1, | ||
| geneBuffers: [createSharedBuffer(Float32Array, gene1Expr)], | ||
| geneNames: ["Gene1"], | ||
| batchIndex: 0, | ||
| dataIsLog1p: true, | ||
| }); |
Contributor
There was a problem hiding this comment.
Same fix needed: replace dataIsLog1p: true with dataType: "log1p".
🐛 Proposed fix
const result0 = processBatch({
type: "run_batch",
filterBuffer,
groupBuffer,
targetGroup: 0,
referenceGroup: -1,
geneBuffers: [createSharedBuffer(Float32Array, gene1Expr)],
geneNames: ["Gene1"],
batchIndex: 0,
- dataIsLog1p: true,
+ dataType: "log1p",
});📝 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
| const result0 = processBatch({ | |
| type: "run_batch", | |
| filterBuffer, | |
| groupBuffer, | |
| targetGroup: 0, | |
| referenceGroup: -1, | |
| geneBuffers: [createSharedBuffer(Float32Array, gene1Expr)], | |
| geneNames: ["Gene1"], | |
| batchIndex: 0, | |
| dataIsLog1p: true, | |
| }); | |
| const result0 = processBatch({ | |
| type: "run_batch", | |
| filterBuffer, | |
| groupBuffer, | |
| targetGroup: 0, | |
| referenceGroup: -1, | |
| geneBuffers: [createSharedBuffer(Float32Array, gene1Expr)], | |
| geneNames: ["Gene1"], | |
| batchIndex: 0, | |
| dataType: "log1p", | |
| }); |
🧰 Tools
🪛 GitHub Actions: TypeScript/JS CI
[error] 166-166: TS2353: Object literal may only specify known properties, and 'dataIsLog1p' does not exist in type 'DGEBatchInput'.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/tests/dge-worker.spec.ts` around lines 157 - 167, The test call to
processBatch uses the deprecated boolean flag dataIsLog1p; replace it with the
new dataType option by changing the argument from dataIsLog1p: true to dataType:
"log1p" in the processBatch invocation (look for the processBatch call that
creates result0 and update the object literal accordingly), ensuring other tests
use the same dataType pattern if present.
Comment on lines
+171
to
+181
| const result1 = processBatch({ | ||
| type: "run_batch", | ||
| filterBuffer, | ||
| groupBuffer, | ||
| targetGroup: 0, | ||
| referenceGroup: -1, | ||
| geneBuffers: [createSharedBuffer(Float32Array, gene2Expr)], | ||
| geneNames: ["Gene2"], | ||
| batchIndex: 1, | ||
| dataIsLog1p: true, | ||
| }); |
Contributor
There was a problem hiding this comment.
Same fix needed: replace dataIsLog1p: true with dataType: "log1p".
🐛 Proposed fix
const result1 = processBatch({
type: "run_batch",
filterBuffer,
groupBuffer,
targetGroup: 0,
referenceGroup: -1,
geneBuffers: [createSharedBuffer(Float32Array, gene2Expr)],
geneNames: ["Gene2"],
batchIndex: 1,
- dataIsLog1p: true,
+ dataType: "log1p",
});📝 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
| const result1 = processBatch({ | |
| type: "run_batch", | |
| filterBuffer, | |
| groupBuffer, | |
| targetGroup: 0, | |
| referenceGroup: -1, | |
| geneBuffers: [createSharedBuffer(Float32Array, gene2Expr)], | |
| geneNames: ["Gene2"], | |
| batchIndex: 1, | |
| dataIsLog1p: true, | |
| }); | |
| const result1 = processBatch({ | |
| type: "run_batch", | |
| filterBuffer, | |
| groupBuffer, | |
| targetGroup: 0, | |
| referenceGroup: -1, | |
| geneBuffers: [createSharedBuffer(Float32Array, gene2Expr)], | |
| geneNames: ["Gene2"], | |
| batchIndex: 1, | |
| dataType: "log1p", | |
| }); |
🧰 Tools
🪛 GitHub Actions: TypeScript/JS CI
[error] 180-180: TS2353: Object literal may only specify known properties, and 'dataIsLog1p' does not exist in type 'DGEBatchInput'.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/tests/dge-worker.spec.ts` around lines 171 - 181, The test invokes
processBatch with the deprecated flag dataIsLog1p; update the call to use
dataType: "log1p" instead (remove dataIsLog1p and add dataType: "log1p") so the
processBatch signature is satisfied. Search for usages in this spec (the
processBatch invocation around the variable result1) and replace any other
occurrences of dataIsLog1p with dataType: "log1p" to keep the tests consistent
with the new API.
Comment on lines
+201
to
+211
| const result = processBatch({ | ||
| type: "run_batch", | ||
| filterBuffer, | ||
| groupBuffer, | ||
| targetGroup: 0, | ||
| referenceGroup: -1, | ||
| geneBuffers: [createSharedBuffer(Float32Array, new Array(nCells).fill(0))], | ||
| geneNames: ["ZeroGene"], | ||
| batchIndex: 0, | ||
| dataIsLog1p: true, | ||
| }); |
Contributor
There was a problem hiding this comment.
Same fix needed: replace dataIsLog1p: true with dataType: "log1p".
🐛 Proposed fix
const result = processBatch({
type: "run_batch",
filterBuffer,
groupBuffer,
targetGroup: 0,
referenceGroup: -1,
geneBuffers: [createSharedBuffer(Float32Array, new Array(nCells).fill(0))],
geneNames: ["ZeroGene"],
batchIndex: 0,
- dataIsLog1p: true,
+ dataType: "log1p",
});📝 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
| const result = processBatch({ | |
| type: "run_batch", | |
| filterBuffer, | |
| groupBuffer, | |
| targetGroup: 0, | |
| referenceGroup: -1, | |
| geneBuffers: [createSharedBuffer(Float32Array, new Array(nCells).fill(0))], | |
| geneNames: ["ZeroGene"], | |
| batchIndex: 0, | |
| dataIsLog1p: true, | |
| }); | |
| const result = processBatch({ | |
| type: "run_batch", | |
| filterBuffer, | |
| groupBuffer, | |
| targetGroup: 0, | |
| referenceGroup: -1, | |
| geneBuffers: [createSharedBuffer(Float32Array, new Array(nCells).fill(0))], | |
| geneNames: ["ZeroGene"], | |
| batchIndex: 0, | |
| dataType: "log1p", | |
| }); |
🧰 Tools
🪛 GitHub Actions: TypeScript/JS CI
[error] 210-210: TS2353: Object literal may only specify known properties, and 'dataIsLog1p' does not exist in type 'DGEBatchInput'.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/tests/dge-worker.spec.ts` around lines 201 - 211, The test invokes
processBatch with the deprecated boolean flag dataIsLog1p; update the call in
this spec so it passes the new dataType field instead (replace dataIsLog1p: true
with dataType: "log1p") in the object passed to processBatch (located in the
test near the block using geneBuffers/geneNames/batchIndex); ensure the key name
and string value match the runtime API so the test aligns with the updated
processBatch signature.
Comment on lines
+243
to
+253
| const result = processBatch({ | ||
| type: "run_batch", | ||
| filterBuffer, | ||
| groupBuffer, | ||
| targetGroup: 0, | ||
| referenceGroup: -1, | ||
| geneBuffers, | ||
| geneNames, | ||
| batchIndex: 0, | ||
| dataIsLog1p: true, | ||
| }); |
Contributor
There was a problem hiding this comment.
Same fix needed: replace dataIsLog1p: true with dataType: "log1p".
🐛 Proposed fix
const result = processBatch({
type: "run_batch",
filterBuffer,
groupBuffer,
targetGroup: 0,
referenceGroup: -1,
geneBuffers,
geneNames,
batchIndex: 0,
- dataIsLog1p: true,
+ dataType: "log1p",
});📝 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
| const result = processBatch({ | |
| type: "run_batch", | |
| filterBuffer, | |
| groupBuffer, | |
| targetGroup: 0, | |
| referenceGroup: -1, | |
| geneBuffers, | |
| geneNames, | |
| batchIndex: 0, | |
| dataIsLog1p: true, | |
| }); | |
| const result = processBatch({ | |
| type: "run_batch", | |
| filterBuffer, | |
| groupBuffer, | |
| targetGroup: 0, | |
| referenceGroup: -1, | |
| geneBuffers, | |
| geneNames, | |
| batchIndex: 0, | |
| dataType: "log1p", | |
| }); |
🧰 Tools
🪛 GitHub Actions: TypeScript/JS CI
[error] 252-252: TS2353: Object literal may only specify known properties, and 'dataIsLog1p' does not exist in type 'DGEBatchInput'.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/tests/dge-worker.spec.ts` around lines 243 - 253, The test is passing the
deprecated flag dataIsLog1p to processBatch; update the call to use the new
dataType parameter by replacing dataIsLog1p: true with dataType: "log1p" so
processBatch receives the expected enum/string type (locate the call to
processBatch in the test and change the argument name and value accordingly).
Comment on lines
+274
to
+284
| const result = processBatch({ | ||
| type: "run_batch", | ||
| filterBuffer, | ||
| groupBuffer, | ||
| targetGroup: 0, | ||
| referenceGroup: -1, | ||
| geneBuffers, | ||
| geneNames: ["NaNGene"], | ||
| batchIndex: 0, | ||
| dataIsLog1p: true, | ||
| }); |
Contributor
There was a problem hiding this comment.
Same fix needed: replace dataIsLog1p: true with dataType: "log1p".
🐛 Proposed fix
const result = processBatch({
type: "run_batch",
filterBuffer,
groupBuffer,
targetGroup: 0,
referenceGroup: -1,
geneBuffers,
geneNames: ["NaNGene"],
batchIndex: 0,
- dataIsLog1p: true,
+ dataType: "log1p",
});📝 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
| const result = processBatch({ | |
| type: "run_batch", | |
| filterBuffer, | |
| groupBuffer, | |
| targetGroup: 0, | |
| referenceGroup: -1, | |
| geneBuffers, | |
| geneNames: ["NaNGene"], | |
| batchIndex: 0, | |
| dataIsLog1p: true, | |
| }); | |
| const result = processBatch({ | |
| type: "run_batch", | |
| filterBuffer, | |
| groupBuffer, | |
| targetGroup: 0, | |
| referenceGroup: -1, | |
| geneBuffers, | |
| geneNames: ["NaNGene"], | |
| batchIndex: 0, | |
| dataType: "log1p", | |
| }); |
🧰 Tools
🪛 GitHub Actions: TypeScript/JS CI
[error] 283-283: TS2353: Object literal may only specify known properties, and 'dataIsLog1p' does not exist in type 'DGEBatchInput'.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/tests/dge-worker.spec.ts` around lines 274 - 284, The call to
processBatch uses the deprecated boolean flag dataIsLog1p; update the argument
object to use dataType: "log1p" instead of dataIsLog1p to match the new API.
Locate the processBatch invocation in the test (the block passing type:
"run_batch", filterBuffer, groupBuffer, targetGroup, referenceGroup,
geneBuffers, geneNames, batchIndex) and remove dataIsLog1p: true and add
dataType: "log1p" so the test reflects the current signature.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
src/charts/dialogs/DGEDialogReact.tsx (1)
54-61:⚠️ Potential issue | 🔴 CriticalUnify the
ChartManagercontract before calling the DGE helpers.TS CI is already failing at Line 57 and Line 108 because
window.mdv?.chartManageris not assignable to theChartManagertype expected byfindDGECapableDatasources(...)/runDGEOnDataStore(...). Please normalize this dependency to one shared type at the boundary instead of reading the global directly in both places; otherwise this file does not compile.Based on learnings, in the MDV codebase, avoid direct access to
window.mdv.chartManagerproperties. Instead, use or create hooks likeuseChartManager()anduseTheme()to access these values. This allows for stable contracts and easier refactoring in the future, as the globalmdvobject is considered a temporary hack.Also applies to: 101-108
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/charts/dialogs/DGEDialogReact.tsx` around lines 54 - 61, Replace direct reads of window.mdv?.chartManager with a unified ChartManager boundary by using or creating a hook (e.g., useChartManager) and consume that hook in this file; specifically, call useChartManager() and pass the returned ChartManager instance into findDGECapableDatasources(...) and runDGEOnDataStore(...), remove direct window.mdv accesses, and update relevant hooks' dependency arrays (e.g., the useMemo that builds genesDatasourceOptions and any other effect that calls runDGEOnDataStore) to include the ChartManager reference so types align with the ChartManager expected by those helpers.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/charts/dialogs/DGEDialogReact.tsx`:
- Around line 90-154: The async handleRun can continue after the dialog
unmounts, causing setResult/setError and cm.addChart to run on a gone component;
add an AbortController (or similar cancel token) that you create when starting
runDGEOnDataStore and store in a ref, pass its signal into runDGEOnDataStore so
the long job can be aborted, and on dialog unmount/close call abort; after each
await in handleRun check the abort flag or mounted-ref before calling setResult,
setError, or cm.addChart to avoid post-unmount side-effects (reference:
handleRun, runDGEOnDataStore, setResult, setError, cm.addChart). Ensure the
cleanup also handles ignored optional cm.addChart errors and treat abort errors
specially so they don’t set error state.
- Around line 138-148: The empty catch after calling cm.addChart(...) hides
volcano-plot failures; modify the try/catch around the cm.addChart(...) call in
DGEDialogReact so the catch takes an error variable (catch (err)) and surfaces a
non-fatal warning (e.g., console.warn or the app's toast/notification API)
including the error and context (e.g., id `dge_volcano_${getRandomString()}`,
selectedGenesDsName, safeTargetGroup/safeReferenceGroup) instead of swallowing
it so partial success is visible and diagnosable.
---
Duplicate comments:
In `@src/charts/dialogs/DGEDialogReact.tsx`:
- Around line 54-61: Replace direct reads of window.mdv?.chartManager with a
unified ChartManager boundary by using or creating a hook (e.g.,
useChartManager) and consume that hook in this file; specifically, call
useChartManager() and pass the returned ChartManager instance into
findDGECapableDatasources(...) and runDGEOnDataStore(...), remove direct
window.mdv accesses, and update relevant hooks' dependency arrays (e.g., the
useMemo that builds genesDatasourceOptions and any other effect that calls
runDGEOnDataStore) to include the ChartManager reference so types align with the
ChartManager expected by those helpers.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 0a994f93-2bdb-46ac-bfd0-cfa25e826a9f
📒 Files selected for processing (3)
src/charts/dialogs/DGEDialogReact.tsxsrc/datastore/dgeIntegration.tssrc/tests/dge-integration.spec.ts
✅ Files skipped from review due to trivial changes (1)
- src/tests/dge-integration.spec.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/datastore/dgeIntegration.ts
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/charts/dialogs/DGEDialogReact.tsx`:
- Around line 162-176: The InputLabel text ("DGE Datasource") and the Select
label prop ("Genes Datasource") are inconsistent; update them to the same string
(pick one consistent label such as "DGE Datasource" or "Genes Datasource") where
InputLabel and the Select component for selectedGenesDsName (and its onChange
setSelectedGenesDsName) are defined, ensuring the label prop matches the
InputLabel so genesDatasourceOptions rendering remains unchanged for
accessibility and clarity.
- Around line 121-136: Remove the temporary diagnostic logging block that
inspects DGE columns (the code that reads
cm.dsIndex[selectedGenesDsName]?.dataStore into gDS and then iterates over
["dge_effect_size","dge_neg_log10_pval_adj"] logging via
console.log/console.error), or replace it with conditional/structured logging
tied to the app logging facility or a debug flag (e.g., use the project's logger
at debug level or guard with an isDebug/isDev check) so production builds no
longer emit these console statements; update references to gDS, c, d, and the
diagnostic messages accordingly and ensure no leftover console.* calls remain.
- Around line 96-98: Remove the three debug console.log statements in
DGEDialogReact.tsx (the lines that print "=== DGE RUN START ===", the
groupColumn/target/reference values, and the dataStore name/size) so no console
logging remains in the production branch; locate them inside the DGE run handler
in DGEDialogReact and delete those console.log calls (or replace with a proper
debug/logger call if persistent diagnostic logging is required).
- Around line 54-61: The TypeScript mismatch arises because the local
ChartManager interface in dgeIntegration.ts declares loadColumnSetAsync as
returning Promise<void> while the actual ChartManager returns Promise<unknown>,
and this component also improperly reads window.mdv.chartManager; to fix: update
the ChartManager type in dgeIntegration.ts so loadColumnSetAsync returns
Promise<unknown> (or a generic/unknown-compatible type) and in
DGEDialogReact.tsx replace direct window.mdv?.chartManager access with the
useChartManager() hook (call useChartManager() at component level, pass the
returned cm into the useMemo dependency array) so genesDatasourceOptions uses
findDGECapableDatasources(cm) with the correct typed ChartManager and
dataStore.name dependency.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: bc54e502-5084-429b-bc41-776ee18e97d0
📒 Files selected for processing (1)
src/charts/dialogs/DGEDialogReact.tsx
Comment on lines
+96
to
+98
| console.log("=== DGE RUN START ==="); | ||
| console.log("groupColumn:", groupColumn, "target:", safeTargetGroup, "reference:", safeReferenceGroup); | ||
| console.log("dataStore:", dataStore.name, "size:", dataStore.size); |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Remove debug console.log statements before merging.
These debug logging statements should be removed for production code.
🧹 Remove debug logging
- console.log("=== DGE RUN START ===");
- console.log("groupColumn:", groupColumn, "target:", safeTargetGroup, "reference:", safeReferenceGroup);
- console.log("dataStore:", dataStore.name, "size:", dataStore.size);📝 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
| console.log("=== DGE RUN START ==="); | |
| console.log("groupColumn:", groupColumn, "target:", safeTargetGroup, "reference:", safeReferenceGroup); | |
| console.log("dataStore:", dataStore.name, "size:", dataStore.size); | |
| const dgeResult = await runDGEOnDataStore( | |
| cm, | |
| dataStore.name, | |
| selectedGenesDsName, | |
| groupColumn, | |
| safeTargetGroup, | |
| safeReferenceGroup, | |
| (done, total) => setProgress({ done, total }), | |
| ); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/charts/dialogs/DGEDialogReact.tsx` around lines 96 - 98, Remove the three
debug console.log statements in DGEDialogReact.tsx (the lines that print "===
DGE RUN START ===", the groupColumn/target/reference values, and the dataStore
name/size) so no console logging remains in the production branch; locate them
inside the DGE run handler in DGEDialogReact and delete those console.log calls
(or replace with a proper debug/logger call if persistent diagnostic logging is
required).
Comment on lines
+162
to
+176
| <FormControl fullWidth size="small"> | ||
| <InputLabel>DGE Datasource</InputLabel> | ||
| <Select | ||
| value={selectedGenesDsName} | ||
| label="Genes Datasource" | ||
| onChange={(e) => setSelectedGenesDsName(e.target.value)} | ||
| disabled={running || genesDatasourceOptions.length <= 1} | ||
| > | ||
| {genesDatasourceOptions.map((name: string) => ( | ||
| <MenuItem key={name} value={name}> | ||
| {name} | ||
| </MenuItem> | ||
| ))} | ||
| </Select> | ||
| </FormControl> |
Contributor
There was a problem hiding this comment.
Fix inconsistent label text.
The InputLabel shows "DGE Datasource" while the Select's label prop shows "Genes Datasource". These should match for accessibility and clarity.
🔧 Fix label consistency
<FormControl fullWidth size="small">
- <InputLabel>DGE Datasource</InputLabel>
+ <InputLabel>Genes Datasource</InputLabel>
<Select
value={selectedGenesDsName}
label="Genes Datasource"📝 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
| <FormControl fullWidth size="small"> | |
| <InputLabel>DGE Datasource</InputLabel> | |
| <Select | |
| value={selectedGenesDsName} | |
| label="Genes Datasource" | |
| onChange={(e) => setSelectedGenesDsName(e.target.value)} | |
| disabled={running || genesDatasourceOptions.length <= 1} | |
| > | |
| {genesDatasourceOptions.map((name: string) => ( | |
| <MenuItem key={name} value={name}> | |
| {name} | |
| </MenuItem> | |
| ))} | |
| </Select> | |
| </FormControl> | |
| <FormControl fullWidth size="small"> | |
| <InputLabel>Genes Datasource</InputLabel> | |
| <Select | |
| value={selectedGenesDsName} | |
| label="Genes Datasource" | |
| onChange={(e) => setSelectedGenesDsName(e.target.value)} | |
| disabled={running || genesDatasourceOptions.length <= 1} | |
| > | |
| {genesDatasourceOptions.map((name: string) => ( | |
| <MenuItem key={name} value={name}> | |
| {name} | |
| </MenuItem> | |
| ))} | |
| </Select> | |
| </FormControl> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/charts/dialogs/DGEDialogReact.tsx` around lines 162 - 176, The InputLabel
text ("DGE Datasource") and the Select label prop ("Genes Datasource") are
inconsistent; update them to the same string (pick one consistent label such as
"DGE Datasource" or "Genes Datasource") where InputLabel and the Select
component for selectedGenesDsName (and its onChange setSelectedGenesDsName) are
defined, ensuring the label prop matches the InputLabel so
genesDatasourceOptions rendering remains unchanged for accessibility and
clarity.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (4)
src/charts/dialogs/DGEDialogReact.tsx (3)
214-218:⚠️ Potential issue | 🟡 MinorFix inconsistent label text.
The
InputLabelshows "DGE Datasource" while theSelect'slabelprop shows "Genes Datasource". These should match for accessibility and clarity.🔧 Fix label consistency
<FormControl fullWidth size="small"> - <InputLabel>DGE Datasource</InputLabel> + <InputLabel>Genes Datasource</InputLabel> <Select value={selectedGenesDsName} label="Genes Datasource"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/charts/dialogs/DGEDialogReact.tsx` around lines 214 - 218, The label text is inconsistent: InputLabel currently displays "DGE Datasource" while the Select's label prop is "Genes Datasource"; update one to match the other for accessibility. Locate the FormControl block that contains InputLabel and Select (refer to InputLabel, Select and the value prop selectedGenesDsName) and make the label strings identical (e.g., change the Select label to "DGE Datasource" or change InputLabel to "Genes Datasource") so both rendered label and Select.props.label are the same.
125-127: 🛠️ Refactor suggestion | 🟠 MajorRemove debug console.log statements before merging.
These diagnostic statements should be removed for production code.
🧹 Remove debug logging
- console.log("=== DGE RUN START ==="); - console.log("groupColumn:", groupColumn, "target:", safeTargetGroup, "reference:", safeReferenceGroup); - console.log("dataStore:", dataStore.name, "size:", dataStore.size);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/charts/dialogs/DGEDialogReact.tsx` around lines 125 - 127, Remove the three debug console.log statements found inside the DGEDialogReact component (the lines logging "=== DGE RUN START ===", "groupColumn/target/reference" referencing groupColumn, safeTargetGroup, safeReferenceGroup, and "dataStore" referencing dataStore.name and dataStore.size); either delete them entirely or replace them with calls to the project’s structured logger at a debug level if persistent logging is required (use the existing logging utility in the codebase), ensuring no raw console.log remains in production code.
155-170: 🛠️ Refactor suggestion | 🟠 MajorRemove diagnostic logging block before merging.
This verification block with console.log/console.error statements is debugging code. Either remove it entirely or convert to structured logging if runtime diagnostics are needed.
🧹 Remove diagnostic block
- // Verify DGE columns exist on genes DS before charting - const gDS = cm.dsIndex[selectedGenesDsName]?.dataStore; - if (gDS) { - for (const f of ["dge_effect_size", "dge_neg_log10_pval_adj"]) { - const c = gDS.columnIndex[f]; - if (!c) { console.error("[DGE] column missing:", f); continue; } - const d = c.data; - if (!d) { console.error("[DGE] column has no data:", f); continue; } - let valid = 0, nanCount = 0, min = Infinity, max = -Infinity; - for (let i = 0; i < d.length; i++) { - if (Number.isNaN(d[i])) { nanCount++; } else { valid++; if (d[i] < min) min = d[i]; if (d[i] > max) max = d[i]; } - } - console.log(`[DGE] genes DS col "${f}": length=${d.length}, valid=${valid}, NaN=${nanCount}, min=${min}, max=${max}, minMax=`, c.minMax); - } - console.log("[DGE] genes DS size:", gDS.size, "columnsWithData:", gDS.columnsWithData.filter((x: string) => x.startsWith("dge_"))); - }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/charts/dialogs/DGEDialogReact.tsx` around lines 155 - 170, Remove the temporary diagnostic logging block that inspects DGE columns in DGEDialogReact: delete the code that retrieves gDS from cm.dsIndex[selectedGenesDsName] and the subsequent loop over ["dge_effect_size","dge_neg_log10_pval_adj"] that uses gDS.columnIndex, c.data, and the console.log/console.error calls (including the final console.log showing gDS.size and columnsWithData). If you need persistent diagnostics instead, replace those console.* calls with structured logging via the project's logger (e.g., processLogger.debug/info) and keep only minimal, non-verbose checks inside the same gDS existence guard.src/datastore/dgeIntegration.ts (1)
72-84:⚠️ Potential issue | 🟠 MajorRisk of silent data corruption when converting
text16toUint8Array.When
groupCol.datatype === "text16", values from aUint16Arrayare assigned directly to aUint8Array. If the group column has more than 256 unique values, values ≥256 will be silently truncated (e.g., index 300 becomes 44), causing incorrect group assignments and corrupted DGE results.🛡️ Add validation to prevent silent truncation
const groupSAB = new SharedArrayBuffer(nCells); const groupArr = new Uint8Array(groupSAB); if (groupCol.datatype === "text16") { + if (groupCol.values.length > 255) { + throw new Error( + `Group column "${config.groupColumn}" has ${groupCol.values.length} unique values, ` + + `exceeding the 255 limit supported for DGE analysis` + ); + } const src = new Uint16Array(groupCol.buffer); for (let i = 0; i < nCells; i++) groupArr[i] = src[i];🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/datastore/dgeIntegration.ts` around lines 72 - 84, The conversion from Uint16Array to Uint8Array can silently truncate values >=256; in the block handling groupCol.datatype === "text16" (variables: groupCol, src, groupArr) validate the source values before assignment by scanning src to find the maximum value (or any value >=256) and if any are >=256 throw/return a clear error (including the offending max value and context like nCells and column name) instead of writing truncated bytes; alternatively, if acceptable, implement an explicit mapping strategy (e.g., re-indexing/group remapping) and document it, but do not perform the blind assignment that causes silent corruption.
🧹 Nitpick comments (3)
src/datastore/DGEDimension.ts (2)
162-174: Consider conditionalizing first-batch diagnostics.The diagnostic logging for the first batch is useful during development but will be noisy in production. Consider gating behind a debug flag or environment variable.
🔧 Conditionalize diagnostic logging
+ const DEBUG_DGE = false; // or import from config if (b === 0) { + if (DEBUG_DGE) { const top5 = [...batchResult.results] .filter(r => !Number.isNaN(r.pval)) .sort((a, b) => a.pval - b.pval) .slice(0, 5); console.log(`[DGE diag] Batch 0 results...`); // ... rest of logging + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/datastore/DGEDimension.ts` around lines 162 - 174, The first-batch diagnostic console logging in DGEDimension (the if (b === 0) block that inspects batchResult.results and prints top5, sigCount, etc.) should be gated behind a debug flag or logger-level check so it doesn't run in production; update the code to check a boolean (e.g., process.env.DEBUG_DGE or a config/logging.isDebug() flag) before executing that block (or convert the console.log calls to use a debug-level logger method) and ensure the condition is used around the entire diagnostic block to keep behavior identical when debugging is enabled.
204-220: Worker error handler could lose context.The
onerrorhandler rejects with only the error message, losing potentially useful debugging information like the filename and line number. Consider preserving more context.🔧 Preserve error context
w.onerror = (e) => { - reject(new Error(`DGE Worker error: ${e.message}`)); + reject(new Error(`DGE Worker error: ${e.message} (${e.filename}:${e.lineno})`)); };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/datastore/DGEDimension.ts` around lines 204 - 220, The runInWorker method's worker.onerror handler currently only rejects with e.message, which loses file/line/stack context; update the onerror callback in runInWorker (the handler attached to this.worker / variable w) to preserve full error context by including e.filename, e.lineno, e.colno and e.error?.stack (or by rejecting with the original Error/event object) when calling reject so callers receive detailed debugging info from the Worker.src/datastore/dgeIntegration.ts (1)
131-136: Consider conditionalizing diagnostic logging.Multiple
console.logstatements throughout the function provide useful debugging information but will be noisy in production. Consider gating behind a debug flag or removing before final release.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/datastore/dgeIntegration.ts` around lines 131 - 136, The diagnostic console.log calls (referencing config.groupColumn, groupCol.datatype, groupValues, targetIdx, refIdx, groupCounts, nCells, filteredCount, geneFields) are noisy in production—wrap them in a conditional debug check or switch them to a proper logger level. Modify the function containing these logs to check a debug flag (e.g., a parameter debug or an env var like DEBUG_DGE) or use an existing logger instance, and only execute the console outputs when the flag is true; replace direct console.log calls with logger.debug(...) if a logger is available so runtime verbosity can be controlled without removing diagnostics.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/datastore/DGEDimension.ts`:
- Line 109: In DGEDimension.ts adjust the indentation of the declaration for the
allBatchResults variable so it matches surrounding code style; locate the const
allBatchResults: GeneResult[] = []; statement inside the DGEDimension
class/function and remove the extra tab so its indentation aligns with adjacent
statements (fix formatting only, no logic changes).
---
Duplicate comments:
In `@src/charts/dialogs/DGEDialogReact.tsx`:
- Around line 214-218: The label text is inconsistent: InputLabel currently
displays "DGE Datasource" while the Select's label prop is "Genes Datasource";
update one to match the other for accessibility. Locate the FormControl block
that contains InputLabel and Select (refer to InputLabel, Select and the value
prop selectedGenesDsName) and make the label strings identical (e.g., change the
Select label to "DGE Datasource" or change InputLabel to "Genes Datasource") so
both rendered label and Select.props.label are the same.
- Around line 125-127: Remove the three debug console.log statements found
inside the DGEDialogReact component (the lines logging "=== DGE RUN START ===",
"groupColumn/target/reference" referencing groupColumn, safeTargetGroup,
safeReferenceGroup, and "dataStore" referencing dataStore.name and
dataStore.size); either delete them entirely or replace them with calls to the
project’s structured logger at a debug level if persistent logging is required
(use the existing logging utility in the codebase), ensuring no raw console.log
remains in production code.
- Around line 155-170: Remove the temporary diagnostic logging block that
inspects DGE columns in DGEDialogReact: delete the code that retrieves gDS from
cm.dsIndex[selectedGenesDsName] and the subsequent loop over
["dge_effect_size","dge_neg_log10_pval_adj"] that uses gDS.columnIndex, c.data,
and the console.log/console.error calls (including the final console.log showing
gDS.size and columnsWithData). If you need persistent diagnostics instead,
replace those console.* calls with structured logging via the project's logger
(e.g., processLogger.debug/info) and keep only minimal, non-verbose checks
inside the same gDS existence guard.
In `@src/datastore/dgeIntegration.ts`:
- Around line 72-84: The conversion from Uint16Array to Uint8Array can silently
truncate values >=256; in the block handling groupCol.datatype === "text16"
(variables: groupCol, src, groupArr) validate the source values before
assignment by scanning src to find the maximum value (or any value >=256) and if
any are >=256 throw/return a clear error (including the offending max value and
context like nCells and column name) instead of writing truncated bytes;
alternatively, if acceptable, implement an explicit mapping strategy (e.g.,
re-indexing/group remapping) and document it, but do not perform the blind
assignment that causes silent corruption.
---
Nitpick comments:
In `@src/datastore/DGEDimension.ts`:
- Around line 162-174: The first-batch diagnostic console logging in
DGEDimension (the if (b === 0) block that inspects batchResult.results and
prints top5, sigCount, etc.) should be gated behind a debug flag or logger-level
check so it doesn't run in production; update the code to check a boolean (e.g.,
process.env.DEBUG_DGE or a config/logging.isDebug() flag) before executing that
block (or convert the console.log calls to use a debug-level logger method) and
ensure the condition is used around the entire diagnostic block to keep behavior
identical when debugging is enabled.
- Around line 204-220: The runInWorker method's worker.onerror handler currently
only rejects with e.message, which loses file/line/stack context; update the
onerror callback in runInWorker (the handler attached to this.worker / variable
w) to preserve full error context by including e.filename, e.lineno, e.colno and
e.error?.stack (or by rejecting with the original Error/event object) when
calling reject so callers receive detailed debugging info from the Worker.
In `@src/datastore/dgeIntegration.ts`:
- Around line 131-136: The diagnostic console.log calls (referencing
config.groupColumn, groupCol.datatype, groupValues, targetIdx, refIdx,
groupCounts, nCells, filteredCount, geneFields) are noisy in production—wrap
them in a conditional debug check or switch them to a proper logger level.
Modify the function containing these logs to check a debug flag (e.g., a
parameter debug or an env var like DEBUG_DGE) or use an existing logger
instance, and only execute the console outputs when the flag is true; replace
direct console.log calls with logger.debug(...) if a logger is available so
runtime verbosity can be controlled without removing diagnostics.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: c2c48aba-20db-4f34-abc5-4bfd64a10130
📒 Files selected for processing (3)
src/charts/dialogs/DGEDialogReact.tsxsrc/datastore/DGEDimension.tssrc/datastore/dgeIntegration.ts
| } | ||
| } | ||
|
|
||
| const allBatchResults: GeneResult[] = []; |
Contributor
There was a problem hiding this comment.
Fix indentation.
Line 109 has an extra tab of indentation that breaks consistency with the surrounding code.
🔧 Fix indentation
- const allBatchResults: GeneResult[] = [];
+ const allBatchResults: GeneResult[] = [];📝 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
| const allBatchResults: GeneResult[] = []; | |
| const allBatchResults: GeneResult[] = []; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/datastore/DGEDimension.ts` at line 109, In DGEDimension.ts adjust the
indentation of the declaration for the allBatchResults variable so it matches
surrounding code style; locate the const allBatchResults: GeneResult[] = [];
statement inside the DGEDimension class/function and remove the extra tab so its
indentation aligns with adjacent statements (fix formatting only, no logic
changes).
…is estimated to be too large
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
src/charts/dialogs/DGEDialogReact.tsx (1)
273-277:⚠️ Potential issue | 🟡 MinorUse one label for this datasource field.
The visible label says
DGE Datasource, but the control label saysGenes Datasource. Pick one string and use it in both places so the form reads consistently.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/charts/dialogs/DGEDialogReact.tsx` around lines 273 - 277, The InputLabel and Select label are inconsistent: InputLabel text is "DGE Datasource" while Select label prop is "Genes Datasource"; update one so both use the same string (for example change the Select label prop to "DGE Datasource") in the FormControl block that contains InputLabel, Select and the value prop selectedGenesDsName to keep the visible label consistent.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/charts/dialogs/DGEDialogReact.tsx`:
- Around line 67-69: The current safeTargetGroup/safeReferenceGroup logic can
still produce an invalid comparison (reference equals target or "rest" when
grouping has a single value); update the validation so safeReferenceGroup is
only set when it is different from safeTargetGroup and represents a non-empty
cohort: i.e., if referenceGroup === "rest" ensure selectedColumnValues contains
at least two distinct values (so "rest" is non-empty) otherwise clear the
reference; if referenceGroup is a concrete value ensure selectedColumnValues
includes it and it !== safeTargetGroup, otherwise clear it. Apply the same check
where you construct the final comparison payload and before calling
runDGEOnDataStore so you never invoke runDGEOnDataStore with an empty or
identical reference/target pair (adjust the logic around safeTargetGroup,
safeReferenceGroup and the code path that calls runDGEOnDataStore).
In `@src/dataloaders/DataLoaders.ts`:
- Around line 134-135: The current check sets shouldSerialize based only on row
count (size); update the logic next to _mdvGetDataInFlight so shouldSerialize is
based on an estimated payload byte size instead: compute estimatedBytes =
columnsCount × size × bytesPerValue (derive columnsCount and dtype byte width
from the request/metadata or reuse the same estimator used by the DGE memory
warning), then set shouldSerialize = estimatedBytes >= SERIALIZE_THRESHOLD_BYTES
(use the existing threshold constant or introduce one). Replace the existing
shouldSerialize = size >= 500_000 with this estimator and ensure
dtype→byte-width mapping (e.g., float32 → 4) is applied or the shared estimator
function is called.
- Around line 162-174: The in-flight counter (_mdvGetDataInFlight) and
serialization gate (releaseSerialize) are being released before
processArrayBuffer() completes and are not released if decompressData() throws;
wrap the entire sequence from response.arrayBuffer() through optional
decompressData() and processArrayBuffer() in a single try/finally so that you
always decrement _mdvGetDataInFlight and call releaseSerialize() in the finally
block, ensuring processArrayBuffer() runs inside the protected section and
resources are released regardless of exceptions.
---
Duplicate comments:
In `@src/charts/dialogs/DGEDialogReact.tsx`:
- Around line 273-277: The InputLabel and Select label are inconsistent:
InputLabel text is "DGE Datasource" while Select label prop is "Genes
Datasource"; update one so both use the same string (for example change the
Select label prop to "DGE Datasource") in the FormControl block that contains
InputLabel, Select and the value prop selectedGenesDsName to keep the visible
label consistent.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: c96e338f-28ea-4c55-8646-46a513ba6f61
📒 Files selected for processing (4)
python/mdvtools/server_utils.pysrc/charts/dialogs/DGEDialogReact.tsxsrc/dataloaders/DataLoaders.tsvite.config.mts
Comment on lines
+67
to
+69
| const safeTargetGroup = selectedColumnValues.includes(targetGroup) ? targetGroup : ""; | ||
| const safeReferenceGroup = referenceGroup === "rest" || selectedColumnValues.includes(referenceGroup) | ||
| ? referenceGroup : "rest"; |
Contributor
There was a problem hiding this comment.
Require a valid reference cohort before enabling DGE.
A user can still launch an invalid comparison here: pick reference B, then switch the target to B. Single-valued grouping columns also still run against an empty "rest" cohort. Both cases hand runDGEOnDataStore a comparison with no real reference population.
🩹 Minimal fix
+ const hasReferenceChoices = selectedColumnValues.length > 1;
- const safeReferenceGroup = referenceGroup === "rest" || selectedColumnValues.includes(referenceGroup)
- ? referenceGroup : "rest";
+ const safeReferenceGroup =
+ referenceGroup === "rest"
+ ? "rest"
+ : hasReferenceChoices &&
+ referenceGroup !== safeTargetGroup &&
+ selectedColumnValues.includes(referenceGroup)
+ ? referenceGroup
+ : "rest";
...
- disabled={running || !groupColumn || !safeTargetGroup || !selectedGenesDsName}
+ disabled={
+ running ||
+ !groupColumn ||
+ !safeTargetGroup ||
+ !selectedGenesDsName ||
+ selectedColumnValues.length < 2
+ }Also applies to: 78-80, 337-340
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/charts/dialogs/DGEDialogReact.tsx` around lines 67 - 69, The current
safeTargetGroup/safeReferenceGroup logic can still produce an invalid comparison
(reference equals target or "rest" when grouping has a single value); update the
validation so safeReferenceGroup is only set when it is different from
safeTargetGroup and represents a non-empty cohort: i.e., if referenceGroup ===
"rest" ensure selectedColumnValues contains at least two distinct values (so
"rest" is non-empty) otherwise clear the reference; if referenceGroup is a
concrete value ensure selectedColumnValues includes it and it !==
safeTargetGroup, otherwise clear it. Apply the same check where you construct
the final comparison payload and before calling runDGEOnDataStore so you never
invoke runDGEOnDataStore with an empty or identical reference/target pair
(adjust the logic around safeTargetGroup, safeReferenceGroup and the code path
that calls runDGEOnDataStore).
Comment on lines
+134
to
+135
| _mdvGetDataInFlight += 1; | ||
| const shouldSerialize = size >= 500_000; |
Contributor
There was a problem hiding this comment.
Base serialization on estimated bytes, not just row count.
size >= 500_000 misses the heaviest DGE requests in this PR. A 100k-row request for 2,000 float32 columns is ~800 MB, but shouldSerialize stays false because size is only 100,000. Key this off columns × size × byte width (or reuse the same estimator as the DGE memory warning) so the safeguard matches real payload size.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/dataloaders/DataLoaders.ts` around lines 134 - 135, The current check
sets shouldSerialize based only on row count (size); update the logic next to
_mdvGetDataInFlight so shouldSerialize is based on an estimated payload byte
size instead: compute estimatedBytes = columnsCount × size × bytesPerValue
(derive columnsCount and dtype byte width from the request/metadata or reuse the
same estimator used by the DGE memory warning), then set shouldSerialize =
estimatedBytes >= SERIALIZE_THRESHOLD_BYTES (use the existing threshold constant
or introduce one). Replace the existing shouldSerialize = size >= 500_000 with
this estimator and ensure dtype→byte-width mapping (e.g., float32 → 4) is
applied or the shared estimator function is called.
Comment on lines
+162
to
174
| let data: ArrayBufferLike; | ||
| try { | ||
| data = await response.arrayBuffer(); | ||
| } catch (e: any) { | ||
| _mdvGetDataInFlight -= 1; | ||
| if (releaseSerialize) releaseSerialize(); | ||
| throw e; | ||
| } | ||
|
|
||
| data = decompress ? await decompressData(new Uint8Array(data)) : data; | ||
| _mdvGetDataInFlight -= 1; | ||
| if (releaseSerialize) releaseSerialize(); | ||
| return processArrayBuffer(data, columns, size); |
Contributor
There was a problem hiding this comment.
Hold the serialization gate until processArrayBuffer() finishes.
If decompressData() throws here, the in-flight counter and gate never release. Even on success, Lines 172-173 unblock the next large request before processArrayBuffer() finishes copying into SharedArrayBuffers, so peak memory still overlaps. Wrap fetch → arrayBuffer() → decompress → processArrayBuffer() in one try/finally and release in finally.
Suggested fix
- let response: Response;
- try {
- response = await fetch(fetchUrl, {
- method: "POST",
- body: JSON.stringify({ columns: columns, data_source: dataSource }),
- headers: {
- "Content-Type": "application/json",
- },
- });
- } catch (e: any) {
- _mdvGetDataInFlight -= 1;
- if (releaseSerialize) releaseSerialize();
- throw e;
- }
-
- let data: ArrayBufferLike;
- try {
- data = await response.arrayBuffer();
- } catch (e: any) {
- _mdvGetDataInFlight -= 1;
- if (releaseSerialize) releaseSerialize();
- throw e;
- }
-
- data = decompress ? await decompressData(new Uint8Array(data)) : data;
- _mdvGetDataInFlight -= 1;
- if (releaseSerialize) releaseSerialize();
- return processArrayBuffer(data, columns, size);
+ try {
+ const response = await fetch(fetchUrl, {
+ method: "POST",
+ body: JSON.stringify({ columns, data_source: dataSource }),
+ headers: {
+ "Content-Type": "application/json",
+ },
+ });
+ let data: ArrayBufferLike = await response.arrayBuffer();
+ data = decompress ? await decompressData(new Uint8Array(data)) : data;
+ return processArrayBuffer(data, columns, size);
+ } finally {
+ _mdvGetDataInFlight -= 1;
+ releaseSerialize();
+ }🧰 Tools
🪛 GitHub Check: tsc
[failure] 173-173:
This expression is not callable.
[failure] 167-167:
This expression is not callable.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/dataloaders/DataLoaders.ts` around lines 162 - 174, The in-flight counter
(_mdvGetDataInFlight) and serialization gate (releaseSerialize) are being
released before processArrayBuffer() completes and are not released if
decompressData() throws; wrap the entire sequence from response.arrayBuffer()
through optional decompressData() and processArrayBuffer() in a single
try/finally so that you always decrement _mdvGetDataInFlight and call
releaseSerialize() in the finally block, ensuring processArrayBuffer() runs
inside the protected section and resources are released regardless of
exceptions.
added 2 commits
March 26, 2026 08:10
…n minMax/quantiles when overwriting column data (setColumnData), with an opt-out for callers that intentionally preserve stats.
❌ Deploy Preview for mdv-dev failed.
|
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (4)
src/dataloaders/DataLoaders.ts (2)
134-145:⚠️ Potential issue | 🟠 MajorSerialization is still gated by row count instead of payload size.
A 100k-cell request for a wide batch can still be hundreds of MB, but this stays parallel because
sizeis below500_000. Please baseshouldSerializeon estimated bytes (columns × rows × bytesPerValue) so the throttle matches the actual memory risk.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/dataloaders/DataLoaders.ts` around lines 134 - 145, The current throttle computes shouldSerialize using the existing size variable, which is row-count based and can miss wide payloads; update the logic in the block that sets shouldSerialize (near _mdvGetDataInFlight, _mdvGetDataSerialize, releaseSerialize) to estimate payload bytes as columns * rows * bytesPerValue (choose an appropriate bytesPerValue, e.g., 8 for 64-bit values or derive from the column dtype if available) and compare that estimatedBytes against the 500_000 threshold instead of using size; keep the rest of the serialize gating (_mdvGetDataSerialize promise, await prev, releaseSerialize) unchanged.
137-174:⚠️ Potential issue | 🔴 CriticalKeep the gate held until
processArrayBuffer()finishes, and release it infinally.
decompressData()can still throw without releasing the counter/gate, and on the success path you unblock the next large request beforeprocessArrayBuffer()has copied intoSharedArrayBuffers. That defeats the peak-memory guard and is also where the currentreleaseSerialize()type errors are coming from.💡 Suggested fix
- let releaseSerialize: (() => void) | null = null; + let releaseSerialize: (() => void) | undefined; if (shouldSerialize) { const prev = _mdvGetDataSerialize; _mdvGetDataSerialize = new Promise<void>((resolve) => { - releaseSerialize = resolve; + releaseSerialize = () => resolve(); }); await prev; } const fetchUrl = url; - - let response: Response; - try { - response = await fetch(fetchUrl, { - method: "POST", - body: JSON.stringify({ columns: columns, data_source: dataSource }), - headers: { - "Content-Type": "application/json", - }, - }); - } catch (e: any) { - _mdvGetDataInFlight -= 1; - if (releaseSerialize) releaseSerialize(); - throw e; - } - - let data: ArrayBufferLike; - try { - data = await response.arrayBuffer(); - } catch (e: any) { - _mdvGetDataInFlight -= 1; - if (releaseSerialize) releaseSerialize(); - throw e; - } - - data = decompress ? await decompressData(new Uint8Array(data)) : data; - _mdvGetDataInFlight -= 1; - if (releaseSerialize) releaseSerialize(); - return processArrayBuffer(data, columns, size); + try { + const response = await fetch(fetchUrl, { + method: "POST", + body: JSON.stringify({ columns, data_source: dataSource }), + headers: { + "Content-Type": "application/json", + }, + }); + + let data: ArrayBufferLike = await response.arrayBuffer(); + data = decompress ? await decompressData(new Uint8Array(data)) : data; + return processArrayBuffer(data, columns, size); + } finally { + _mdvGetDataInFlight -= 1; + releaseSerialize?.(); + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/dataloaders/DataLoaders.ts` around lines 137 - 174, The gate and in-flight counter need to be held until processArrayBuffer() completes; wrap the decompressData() and processArrayBuffer() calls in a try/finally so that _mdvGetDataInFlight is decremented and releaseSerialize() is called only in the finally block (after await processArrayBuffer(...)), and rethrow any caught errors; specifically modify the block using decompress, decompressData, and processArrayBuffer so any exception from decompressData or processArrayBuffer still goes through the finally that calls _mdvGetDataInFlight -= 1 and if (releaseSerialize) releaseSerialize().src/charts/dialogs/DGEDialogReact.tsx (2)
274-277:⚠️ Potential issue | 🟡 MinorMake the datasource label text consistent.
The
InputLabelsays"DGE Datasource"but theSelectlabel says"Genes Datasource". They should match for accessibility and clarity.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/charts/dialogs/DGEDialogReact.tsx` around lines 274 - 277, The InputLabel text and the Select label are inconsistent in DGEDialogReact (InputLabel currently "DGE Datasource" while Select label is "Genes Datasource"); update one so both match (pick a single phrasing, e.g., "DGE Datasource") and ensure the InputLabel component and the Select prop label="..." use the exact same string to preserve accessibility and clarity (locate the InputLabel and the Select using selectedGenesDsName in DGEDialogReact and make their label text identical).
67-69:⚠️ Potential issue | 🟠 MajorThis still allows invalid target/reference comparisons.
safeReferenceGroupcan still resolve to the same concrete group as the target, and"rest"is still considered valid when the grouping column only has one value. That can sendrunDGEOnDataStore()an empty or self-comparison.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/charts/dialogs/DGEDialogReact.tsx` around lines 67 - 69, safeReferenceGroup and safeTargetGroup can still end up equal or allow "rest" when there's only one group; update the sanitization before calling runDGEOnDataStore() to (1) derive the actual available groups from selectedColumnValues, (2) only accept "rest" as safeReferenceGroup if availableGroups.length > 1, (3) ensure safeReferenceGroup !== safeTargetGroup (if they would be equal set safeReferenceGroup to ""), and (4) ensure safeTargetGroup is only set if targetGroup exists in availableGroups (otherwise ""), so runDGEOnDataStore() never receives an empty or self-comparison; adjust the logic around safeTargetGroup, safeReferenceGroup, selectedColumnValues, targetGroup, referenceGroup, and the call site for runDGEOnDataStore accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/charts/dialogs/DGEDialogReact.tsx`:
- Line 31: The component is masking a type mismatch by using "as unknown as
DGEChartManager" on useChartManager(); fix by giving the hook a proper typed
signature or adding a typed wrapper hook and removing the double-cast: update
useChartManager() to return DGEChartManager (or DGEChartManager | undefined)
with correct typing for window.mdv.chartManager, or add a new
useDGEChartManager() that returns DGEChartManager and use that in DGEDialogReact
so you can remove the unsafe "as unknown as" cast.
---
Duplicate comments:
In `@src/charts/dialogs/DGEDialogReact.tsx`:
- Around line 274-277: The InputLabel text and the Select label are inconsistent
in DGEDialogReact (InputLabel currently "DGE Datasource" while Select label is
"Genes Datasource"); update one so both match (pick a single phrasing, e.g.,
"DGE Datasource") and ensure the InputLabel component and the Select prop
label="..." use the exact same string to preserve accessibility and clarity
(locate the InputLabel and the Select using selectedGenesDsName in
DGEDialogReact and make their label text identical).
- Around line 67-69: safeReferenceGroup and safeTargetGroup can still end up
equal or allow "rest" when there's only one group; update the sanitization
before calling runDGEOnDataStore() to (1) derive the actual available groups
from selectedColumnValues, (2) only accept "rest" as safeReferenceGroup if
availableGroups.length > 1, (3) ensure safeReferenceGroup !== safeTargetGroup
(if they would be equal set safeReferenceGroup to ""), and (4) ensure
safeTargetGroup is only set if targetGroup exists in availableGroups (otherwise
""), so runDGEOnDataStore() never receives an empty or self-comparison; adjust
the logic around safeTargetGroup, safeReferenceGroup, selectedColumnValues,
targetGroup, referenceGroup, and the call site for runDGEOnDataStore
accordingly.
In `@src/dataloaders/DataLoaders.ts`:
- Around line 134-145: The current throttle computes shouldSerialize using the
existing size variable, which is row-count based and can miss wide payloads;
update the logic in the block that sets shouldSerialize (near
_mdvGetDataInFlight, _mdvGetDataSerialize, releaseSerialize) to estimate payload
bytes as columns * rows * bytesPerValue (choose an appropriate bytesPerValue,
e.g., 8 for 64-bit values or derive from the column dtype if available) and
compare that estimatedBytes against the 500_000 threshold instead of using size;
keep the rest of the serialize gating (_mdvGetDataSerialize promise, await prev,
releaseSerialize) unchanged.
- Around line 137-174: The gate and in-flight counter need to be held until
processArrayBuffer() completes; wrap the decompressData() and
processArrayBuffer() calls in a try/finally so that _mdvGetDataInFlight is
decremented and releaseSerialize() is called only in the finally block (after
await processArrayBuffer(...)), and rethrow any caught errors; specifically
modify the block using decompress, decompressData, and processArrayBuffer so any
exception from decompressData or processArrayBuffer still goes through the
finally that calls _mdvGetDataInFlight -= 1 and if (releaseSerialize)
releaseSerialize().
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 9f596107-55b0-419f-9c07-052b96ce16c7
📒 Files selected for processing (8)
DGE_README.mdpython/mdvtools/server_utils.pysrc/charts/ChartManager.jssrc/charts/dialogs/DGEDialogReact.tsxsrc/dataloaders/DataLoaders.tssrc/datastore/DataStore.jssrc/tests/datastore-setColumnData.spec.tsvite.config.mts
✅ Files skipped from review due to trivial changes (1)
- DGE_README.md
🚧 Files skipped from review as they are similar to previous changes (2)
- python/mdvtools/server_utils.py
- vite.config.mts
| } | ||
|
|
||
| function DGEDialogContent({ dataStore, onClose }: DGEDialogContentProps) { | ||
| const chartManager = useChartManager() as unknown as DGEChartManager; |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, find the file and inspect the relevant lines
fd -t f "DGEDialogReact.tsx" --exec cat -n {} \; 2>/dev/null | head -50Repository: Taylor-CCB-Group/MDV
Length of output: 2299
🏁 Script executed:
# Also check if the file exists and its location
find . -name "DGEDialogReact.tsx" -type fRepository: Taylor-CCB-Group/MDV
Length of output: 103
🏁 Script executed:
# Get context around line 31
if [ -f "src/charts/dialogs/DGEDialogReact.tsx" ]; then
echo "=== DGEDialogReact.tsx lines 1-60 ===";
sed -n '1,60p' src/charts/dialogs/DGEDialogReact.tsx | cat -n;
fiRepository: Taylor-CCB-Group/MDV
Length of output: 2608
🏁 Script executed:
# Search for useChartManager hook definition
rg "export.*useChartManager|function useChartManager|const useChartManager" -A 3Repository: Taylor-CCB-Group/MDV
Length of output: 248
🏁 Script executed:
# Find DGEChartManager definition
rg "interface DGEChartManager|class DGEChartManager|type DGEChartManager" -A 5Repository: Taylor-CCB-Group/MDV
Length of output: 584
Avoid masking the ChartManager type mismatch with as unknown as.
The double cast bypasses TypeScript's type checking and masks the fact that useChartManager() lacks a proper return type annotation. The hook currently returns window.mdv.chartManager with no explicit type, forcing this component to use an escape hatch to satisfy the DGEChartManager type requirement. Either update useChartManager() to have an explicit return type, create a typed variant (e.g., useDGEChartManager()), or ensure the global manager type is properly defined. This violates the guideline: Avoid as casts where possible in TypeScript.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/charts/dialogs/DGEDialogReact.tsx` at line 31, The component is masking a
type mismatch by using "as unknown as DGEChartManager" on useChartManager(); fix
by giving the hook a proper typed signature or adding a typed wrapper hook and
removing the double-cast: update useChartManager() to return DGEChartManager (or
DGEChartManager | undefined) with correct typing for window.mdv.chartManager, or
add a new useDGEChartManager() that returns DGEChartManager and use that in
DGEDialogReact so you can remove the unsafe "as unknown as" cast.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
#Summary: Browser-Based Differential Gene Expression (DGE)
This PR introduces Tier 1 Differential Gene Expression (DGE) directly in the browser. It enables high-performance, real-time statistical analysis of large single-cell datasets (100k+ cells, 20k+ genes) without requiring a high-memory backend for calculation.
Core Features
High-Performance Statistical Engine:
Welch's t-test: Implemented for comparing gene expression between groups (default method in tools like Scanpy).
Welford’s Online Algorithm: Computes mean and variance in a single pass over the data, significantly reducing memory pressure and allowing the engine to handle huge datasets without multiple passes.
Benjamini-Hochberg Correction: Built-in FDR (False Discovery Rate) adjustment for multiple testing correction.
Adaptive Effect Size Detection:
Automatically probes gene expression data to detect normalization (Log1p-normalized, Linear/Raw, or Z-scored).
Calculates appropriate metrics: Log2 Fold Change (Log2FC) with optional back-transformation (expm1) or Mean Difference.
Fixes previous issues where sparse raw data was misclassified as log1p, causing inflated fold changes.
Scalable Data Pipeline:
HTTP Streaming: The Flask backend now uses chunked transfer encoding to stream individual gene columns from HDF5, preventing backend OOM (Out-of-Memory) errors.
Batching & Workers: Processes genes in batches (default: 2000) using Web Workers and SharedArrayBuffer to keep the UI responsive while performing heavy calculations.
UI & Integration:
New DGEDialogReact component for defining groups (target vs. reference).
Automatic generation of Volcano Plots and result columns (dge_effect_size, dge_neg_log10_pval_adj) for visualization.
Technical Changes
src/datastore/dgeStats.ts: Pure TypeScript core for statistical algorithms.
src/datastore/DGEDimension.ts & dgeWorker.ts: Orchestration of the calculation pipeline and worker management.
src/datastore/dgeIntegration.ts: Integration with MDV's DataStore, handling data discovery and result writing.
python/mdvtools/server.py: Implementation of yield_byte_data for column streaming.
src/lib/constants.ts: Centralized DEFAULT_DGE_BATCH_SIZE for easy tuning.
Test Plan
Unit Tests: Verified statistical correctness using vitest (see src/tests/dge-stats.spec.ts, dge-worker.spec.ts).
Integration Tests: Validated the full pipeline against reference datasets generated by Python scripts (see src/tests/dge-integration.spec.ts).
Data Detection Logic: Added specific tests for the multi-gene probe loop to ensure raw counts are correctly identified (see src/tests/dge-integration.spec.ts).
Manual Verification: Tested with large datasets (e.g., PBMC 3k, B-cell datasets) to ensure volcano plots and effect sizes match expected distributions.
Documentation
Detailed technical documentation and troubleshooting steps have been added to DGE_README.md.
Summary by CodeRabbit
New Features
Documentation
Infrastructure
Tests