Skip to content

Check every index read, everywhere: noUncheckedIndexedAccess in tsconfig.json - #417

Merged
adamjohnwright merged 2 commits into
mainfrom
fix/345-guard-libraries
Oct 12, 2026
Merged

adamjohnwright merged 2 commits into
mainfrom
fix/345-guard-libraries

Conversation

@adamjohnwright

@adamjohnwright adamjohnwright commented Oct 12, 2026 •

Copy link
Copy Markdown
Contributor

Closes #345. The last 84 findings, in the two workspace libraries, are handled. noUncheckedIndexedAccess is now in tsconfig.json, so every build enforces it: both libraries, reactome-table-wc, the app and the embed. The flag comes out of the app and embed configs (they inherit it). The temporary counted check (scripts/check-indexed-access.mjs, its baseline, spec, npm script, and preflight and CI steps) is removed.

Over #413–#417: 346 findings (77 website, 95 + 85 pathway browser, 84 libraries, 5 templates), with no non-null assertions added (no-non-null-assertion stayed at 232).

reactome-table: the GSA form's spreadsheet

table.store.ts indexes a 2-D grid throughout and had two unit tests. So before rewriting it:

  • table.store.session.spec.ts (new) drives the store through an editing session: generate, mapped import, typing (with the column maximum rising and falling), wrapping moves, range select, paste, delete, add and remove rows and columns by name and index, and an unmapped import. It snapshots the full state (cell values, selection marks, hidden cell, column maxima, start/stop, focus) after every step, plus every selector.
  • The snapshots were recorded against the old store, and the rewrite reproduces them exactly.
  • Proven sensitive: dropping a single trim() in paste fails two of its tests.

The rewrite keeps the store's behaviour on a missing row or cell. Reads that always threw on one still throw, through rowAt/cellAt/maxAt, which now name the coordinates. Reads across rows that are rectangular by construction (pushAll and addRow keep them so) go through atColumn, an explicit cast with that reason. Ranges.minMax uses Math.min/max in place of a two-element sort.

reactome-gsa-form

  • An unknown parameter type from the GSA service (one added since) used to throw on typeToParse[type](…), taking the form down. It's now read as text: parserFor, with a new spec.
  • An empty method list now gives no common parameters instead of a crash, and so do the covariances of a dataset with no annotations (?.slice). An empty annotation table still throws in initAnnotationColumns (now a RangeError saying the table has no rows), as it did before.
  • comparisonGroup2Values already handled a missing list; its signature now says so.
  • The rest are equivalent rewrites: split parts, transposed columns, and the pairing check, which still counts a missing value the way it always did.

Fixed after review

  • Deleting the selected row broke the table (already on main). Select a cell in the last row, delete that row (the bin button), and Cells.select threw reading the stale selection. That ended the store's state stream and left the GSA form's table unusable. A cell that's gone now has nothing to un-mark. The session spec gains that step: it fails without the fix, and the earlier snapshots are unchanged (additions only).
  • A missing GSA annotation value (a values list shorter than sample_ids) stays undefined (sent as null), as before. An earlier commit here had made it "".
  • Review probe: the reviewer ran 20 store edge cases on main and on this branch: ragged and empty pastes and imports, reversed ranges, out-of-range selects, deleting the last row or column while selected. The final state matched in every case; only the error class differs (RangeError for TypeError), at the same point.

Templates found by the library builds

  • table.component.html colSpan.
  • The statistical-design group lookup.

Lint

With the flag in the root config, type-aware lint also sees spec and e2e files under it. Three index reads there (e2e/reacfoam.spec.ts, chat-handoff.spec.ts) now type-check as errors, which lint counts as any. They're guarded. Lint is unchanged at its baseline.

Tests

  • Builds with the flag: build:libs, ng build reactome-table-wc, npm run build and build:embed all succeed.
  • Full e2e suite locally: 388 passed, plus the 6 tests that skip themselves. After the last change, the analysis, analysis-results and reacfoam specs were rerun: 35 passed.
  • Unit tests: 688 passed.
  • Gates: check:lint, check:dead and format:check unchanged at their baselines.
  • Render tests: ran live in the pre-push hook: ok.

🤖 Generated with Claude Code

adamjohnwright and others added 2 commits October 12, 2026 08:00
…fig.json

The workspace libraries' last 84 findings handled, so the flag moves from the
app's and the embed's own configs into tsconfig.json, and every build --
both libraries, the table web component, the app, the embed -- enforces it.
The counted check that held the libraries, its baseline and its CI and
preflight steps are removed.

reactome-table's store is the spreadsheet behind the GSA form's data entry
and had two unit tests. A session test now drives it through generate,
import, type, move, select, paste, delete, add and remove rows and columns,
and snapshots its state after every step; the snapshots were recorded
against the old code, and the rewrite reproduces them exactly. Reads that
always threw on a missing row or cell still do, through rowAt/cellAt/maxAt,
now saying where.

reactome-gsa-form: a parameter type the form does not know, one the GSA
service added since, is read as text rather than throwing (parserFor, with
a spec); an empty method list or an annotation table with no header row
gives an empty value rather than a crash.

With the flag in the root config, type-aware lint sees spec and e2e files
under it too: three index reads there are guarded.

Closes #345.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… stay null

From review:
- Deleting the row (or column) the selection was in left selectedCoords
  pointing at it. Cells.select read that row to un-mark it and threw,
  which ended the store's state stream and left the GSA form's table
  unusable -- reachable with the delete-row button. Already on main. A cell
  that is gone has nothing to un-mark now; the session spec gains the step
  (fails without the fix; earlier snapshots unchanged).
- A GSA annotation value missing from a short values list was turned into
  "" by this PR; it stays undefined (sent as null), as it always was.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@adamjohnwright
adamjohnwright merged commit 5dbca43 into main Oct 12, 2026
6 checks passed
@adamjohnwright
adamjohnwright deleted the fix/345-guard-libraries branch October 12, 2026 08:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

noUncheckedIndexedAccess: needs its 261 findings fixed, or a counted check

1 participant