feat: first-class synthetic columns (#391) - #406
Conversation
…esence (#391) Adds `syntheticColumns` config for columns whose values are computed from other columns at runtime (e.g. Total = Price × Qty). Synthetic columns appear in toJSON(), getValue(), findRow/findRows, and countRows filters, but throw clear errors from getCell() and smartFill() since they have no DOM cell. New API surface: - `syntheticColumns` config option on useTable() - `row.getValue(col)` — universal accessor for real, override, and synthetic columns - `SyntheticColumnDef` type exported from index Implementation: - Synthetics stay out of the header map (no sentinel indices) - splitFilters() extended with a third bucket for synthetic filters - matchesSyntheticFilters() post-evaluates candidates via compute() - No-chaining enforcement prevents one synthetic from reading another - Collision validation at init rejects names that match real headers Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
🤖 Bot HQ🔗 Issue Link🔗 Closes #391 🔍 AI Review
📊 This PR: $0.0537 · Repo today: $0.05 / $1.00 · Month: $0.05 / $10.00 Review History (1 round)
This comment is managed by the bot — do not edit directly. |
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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 |
|
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (1)
src/engine/rowFinder.ts (1)
93-107: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftExtract a shared override/synthetic post-filter matcher.
src/engine/rowFinder.tsandsrc/useTable.tseach implement their own copy of the override+synthetic filter matching logic. The duplication already caused the two copies to diverge in how often they instantiate aSmartRowper row.
src/engine/rowFinder.ts#L93-L107: keepmatchesSyntheticFilters/matchesOverrideFiltershere as the canonical implementation, or move them into a shared exported helper.src/useTable.ts#L356-L390: replace the inline override+synthetic matching loop incountPostFilterMatcheswith the shared helper fromrowFinder.ts, instead of re-implementing the same logic with a differentSmartRowinstantiation pattern.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/engine/rowFinder.ts` around lines 93 - 107, The override and synthetic post-filter matching logic is duplicated and creates SmartRow instances inconsistently. Keep or expose matchesSyntheticFilters and matchesOverrideFilters in src/engine/rowFinder.ts as the shared canonical implementation, then update countPostFilterMatches in src/useTable.ts to call those helpers instead of its inline matching loop; preserve the existing filter behavior while ensuring each row follows the shared SmartRow instantiation pattern.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/engine/tableMapper.ts`:
- Around line 71-84: Move the synthetic-column collision validation in the
table-mapping success path before assigning the map to this._headerMap. Keep the
collision check based on the newly built entries/map, and only commit
this._headerMap after validation succeeds so failed mapping leaves no cached
state for getMap().
In `@src/smartRow.ts`:
- Around line 577-582: Update the synthetic-column computation in toJSON’s
atomic path so def.compute receives a row bound to the frozen snapshot or
materialized clone, with getValue/getCell resolving from the already-extracted
result rather than live DOM. Preserve existing synthetic column filtering and
output conversion, and ensure atomic toJSON maintains zero inter-column stagger
when compute reads other columns.
- Around line 444-461: Replace the shared _inSyntheticCompute boolean in
smart.getValue with per-row serialization of synthetic column computations,
ensuring concurrent sibling reads queue and execute without false nesting errors
while genuine nested reads still throw the existing error. Update the
syntheticDef handling and cleanup flow accordingly, and add coverage for
Promise.all on independent synthetic columns if tests are available.
- Around line 463-486: Update getValue to use the same cell-navigation and
beforeCellRead flow as getCell and toJSON, including reuse of the already-read
value from the current non-atomic toJSON result when available. Ensure real and
override columns are navigated before reading, while preserving synthetic-column
compute behavior and existing column-not-found handling.
In `@src/typeContext.ts`:
- Around line 307-313: Do not edit the declarations in src/typeContext.ts
directly, including getValue, SyntheticColumnDef, and syntheticColumns. Update
the corresponding source declarations in src/types.ts, then regenerate
typeContext.ts using the project’s established generation process so all
mirrored sections remain synchronized.
In `@src/types.ts`:
- Around line 699-706: Update mergeTableConfig to merge syntheticColumns using
the same two-sided merge path as columnOverrides, so preset synthetic
definitions are retained when absent from the override while override keys
replace matching preset keys. Ensure the resulting configuration preserves
top-level override semantics without dropping preset synthetic columns.
---
Nitpick comments:
In `@src/engine/rowFinder.ts`:
- Around line 93-107: The override and synthetic post-filter matching logic is
duplicated and creates SmartRow instances inconsistently. Keep or expose
matchesSyntheticFilters and matchesOverrideFilters in src/engine/rowFinder.ts as
the shared canonical implementation, then update countPostFilterMatches in
src/useTable.ts to call those helpers instead of its inline matching loop;
preserve the existing filter behavior while ensuring each row follows the shared
SmartRow instantiation pattern.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: a19290dc-89ac-4e98-9bdf-a3b385d1d8ca
📒 Files selected for processing (11)
docs/.vitepress/smartrow-signatures.jsondocs/.vitepress/tableconfig-signatures.jsonsrc/engine/rowFinder.tssrc/engine/tableMapper.tssrc/index.tssrc/smartRow.tssrc/typeContext.tssrc/types.tssrc/useTable.tstests/synthetic-columns.spec.tstests/unit/syntheticColumns.test.ts
- Move collision check before _headerMap assignment in tableMapper - Replace shared _inSyntheticCompute boolean with per-compute guarded proxy row - Atomic toJSON compute reads from snapshot result via proxy row - Deep-merge syntheticColumns in mergeTableConfig - Make matchesOverrideFilters/matchesSyntheticFilters public on RowFinder - Refactor countPostFilterMatches to delegate to RowFinder methods Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Apply the same snapshot-bound proxy row pattern to the non-atomic toJSON path, so synthetic compute() reuses already-extracted column values instead of re-reading from potentially evicted DOM cells on horizontally virtualized tables. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
AI ReviewPR #406 adds first-class synthetic columns feature with computed values derived from real/override columns. The implementation is well-structured with 321 unit tests passing, comprehensive E2E coverage, and proper TypeScript strictness. All acceptance criteria from #391 are met with no regressions detected.
|
Closes #391
Summary
Adds synthetic columns — computed columns with no DOM presence. Values are derived from other columns at runtime (e.g.
Total = Price × Qty).syntheticColumnsconfig option onuseTable()row.getValue(col)universal accessor — works for real, override, and synthetic columnstoJSON()includes synthetic columns (respectscolumnsoption)findRow()/findRows()/countRows()filter by synthetic values via per-row evaluationgetCell()andsmartFill()throw clear errors for synthetic columns (no DOM cell)compute()cannot read another syntheticBail-out point: The branch forks cleanly from
d80637f— if this feature proves to be a bad idea, revert the merge commit.Files changed
src/types.tsSyntheticColumnDeftype,syntheticColumnson config,getValueon SmartRowsrc/smartRow.tsgetValue()impl,toJSON()synthetic append,getCell/smartFillguardssrc/engine/rowFinder.tssplitFilters3-way split,matchesSyntheticFilterspost-filtersrc/engine/tableMapper.tssrc/useTable.tscountRowssynthetic support,getRowguard,getHeadersincludes syntheticssrc/index.tsTest plan
pnpm exec tsc --noEmit— passespnpm run test:unit— 321/321 pass (8 new synthetic column tests)override-filter.spec.ts) — 5/5 pass (no regression)pnpm run build— passes🤖 Generated with Claude Code
Summary by CodeRabbit
SmartRow.getValue()for retrieving resolved column values.