test(table): pin the row-write behavior the suite could not fail on - #6811
Conversation
Mutation testing across the branch found four changes that could be reverted with the whole suite green, plus two comments asserting something untrue. - Copilot row writes now assert the translated storage keys, not the literal keying flag. The shared table fixture uses legacy columns with no id, where name-to-id mapping is the identity, so the wrong keying was unobservable; columns whose id differs from name make it fail. Copilot is the one row-write surface whose keys come from a model, and under id keying the lax write path stores those keys verbatim and reports success. - The three provenance transport helpers get direct cover. The route tests only reach mapInput and present, so each could be replaced by a constant, and a constant envelope reader silently downgrades every executor write to untracked. - The domain provenance fixtures run against the real envelope guard instead of a stub that always accepted them; none of the old fixtures was a shape the guard admits. - The attribution allowlist keys on the client-id reader rather than the field name, so it no longer misses a surface that names the acting tab positionally -- one of the two real suppliers was already invisible to it. Also restores the selectedValues narrowing on the migrated row routes, so a stale sidecar entry for a dropped column no longer rides along in the envelope, matching what the unmigrated rows and query routes have always done. Adds a compile-time tie between the shared table fixture and TableDefinition. tsconfig excludes test files, so an assertion placed in one is never checked; a new required field was a production type error and a silent no-op across every route test. Corrects the getRowSummaryById TSDoc, which claimed the Copilot row tool never put executions on the wire. It did, and that narrowing is a deliberate wire change rather than a pure saving.
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
PR SummaryLow Risk Overview Provenance transport ( Production fix: Attribution audit detects actor suppliers via Reviewed by Cursor Bugbot for commit a03e471. Configure here. |
Greptile SummaryThe PR strengthens mutation-sensitive coverage around table row key translation, provenance transport, fixture contracts, and attribution auditing while narrowing loaded provenance to values still present in each row.
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains.
|
| Filename | Overview |
|---|---|
| apps/sim/lib/table/application/rows.ts | Narrows provenance sidecar loading to the values currently present in each row, consistent with exact-cell boundary filtering. |
| apps/sim/lib/table/events.attribution.test.ts | Replaces client-ID-reader discovery with checks for the two actual actor-attribution forms and covers both current supplying routes. |
| apps/sim/app/api/table/row-secret-provenance.test.ts | Adds direct coverage for the three provenance transport helpers and their rejection paths. |
| apps/sim/lib/copilot/tools/server/table/user-table.test.ts | Verifies insert and update operations translate model-supplied column names to stable storage IDs. |
| apps/sim/lib/table/application/row-secret-provenance.test.ts | Replaces an always-accepting bundle mock with valid fixtures exercised against the production envelope guard. |
| apps/sim/lib/table/fixture-contract.ts | Adds a type-only compile-time assertion that the shared table fixture remains assignable to the production table definition. |
Reviews (2): Last reviewed commit: "test(table): key the attribution audit o..." | Re-trigger Greptile
…ader The allowlist matched the `readClientId` call site. That reader is a general-purpose helper any surface may call for unrelated reasons, so an innocent caller elsewhere in the app would be classified as naming a tab, and aliasing or wrapping the reader would slip past it. Match the two forms that actually attribute a write instead -- setting `actorClientId`, or passing a second argument to the signal -- which hold however the id was obtained and say nothing about unrelated readers.
|
@cursor review |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit a03e471. Configure here.
Summary
Follow-up to #6808/#6809. A mutation-testing pass over that work found four changes that could be reverted with the whole suite green, plus two comments asserting something untrue. This pins them.
id, where name-to-id mapping is the identity, so the wrong keying was unobservable; columns whoseiddiffers fromnamemake it fail. Copilot is the one row-write surface whose column keys come from a model, and under id keying the lax write path stores those keys verbatim and reports success — a silent row corruption.mapInputandpresent, so each helper could be replaced by a constant, and a constant envelope reader silently downgrades every executor write from a stamped bundle to untracked.selectedValuesnarrowing on the migrated row routes, so a stale sidecar entry for a dropped column no longer rides along in the provenance envelope. Matches what the unmigratedrows/queryroutes have always done.TableDefinition.tsconfigexcludes test files, so an assertion placed in one is never checked — a new required field was a production type error and a silent no-op across every route test. The probe is type-only so the@sim/testingdevDependency never becomes a runtime import.getRowSummaryByIdTSDoc, which claimed the Copilot row tool never put executions on the wire. It did; that narrowing is a deliberate wire change rather than a pure saving.Type of Change
Testing
1,975 tests pass across
lib/table,app/api/table,lib/copilot/tools/server/table,app/api/v2/tablesandlib/api/server/routes.type-checkclean, all 29 audits pass.Every new test is mutation-verified — the behavior was reverted and the covering test confirmed to go red. The keying mutation reproduces the corruption exactly:
{ name: 'Grace' }stored where{ col_name: 'Grace' }was meant.Checklist