fix(spec,drivers): the three drivers type-check — and most of their 292 errors were the types, not the tests (#4311) - #4407
Merged
Conversation
…d 165 of their 292 errors were the types, not the tests (#4311) `driver-sql` (241), `driver-sqlite-wasm` (27) and `driver-memory` (23) were filed as one playbook: author-tier literals handed to a parsed-tier parameter. That is what 127 of them were. The other 165 were the types being wrong, and only a tsc that had never run could have kept them invisible: - **`bypassTenantAudit` was never on `DriverOptionsSchema`** (118 errors). `SqlDriver.auditMissingTenant` reads it, its own warning text tells callers to pass it, `ObjectQLEngine` sets it for system-context calls, and both `service-settings` and `service-datasource` send it on global-scope writes. Declared now, with the limit stated: it silences a diagnostic, it never changes which rows a write touches. The driver read it — and `timezone`, `tenantId`, `tenantIds`, `preserveAudit`, all four long since declared — through `(options as any)`; those seven casts are gone, so the next undeclared option fails the build instead of hiding behind one. - **`findOne(object, id)` was on no contract** (41). An undeclared `typeof query === 'string'` branch in `SqlDriver`, used by nothing outside this package's own tests, and answered differently by the other two drivers: `MemoryDriver` spreads the string (`{0:'t',1:'1'}`), `MongoDriver` reads `query.where` (undefined → an arbitrary row). It also bypassed `find()`, so it skipped field selection, temporal coercion and the deterministic-order tie-breaker `IDataDriver.find` promises. Removed; the call sites spell the lookup as `{ object, where: { id } }`. - **`initObjects` did not declare the `tenancy` it consumes** (4) — each object flows into `computeAndRecordTenantField`, which reads `obj.tenancy` to pick the tenant column and set the sticky opt-out. `registerExternalObject` had it right all along. - **`AnalyticsQuery` has no author tier** (19+1). `timezone` is `.default('UTC')` and `public` is `.default(false)`, so the parsed types require both; the tests wrote author-tier literals. Added `AnalyticsQueryInput` beside `AnalyticsQuery` (the `QueryInput`/`QueryAST` pattern) and routed the literals through the schema, so the parse itself is the proof the default lands — `defineCube` does the same job for the cube. - **`InMemoryDriver.create` declared no return type** (3), so TS inferred the literal it builds and every other column of the created row vanished from the caller's view. The remaining 127 are the filed playbook: `object` added to the query literal (the driver reads the name from parameter 1 — the AST field is required and redundant there, filed separately), and `count`/`find` calls likewise. Six call sites the type checker could not see at all wrote `findOne('users', alice.id as any)`; the cast is what let an off-contract call survive a narrowing. They surfaced as runtime failures the moment the branch was removed, which is the point. All three packages now declare `typecheck` and leave the DEBT ledger: 60/77 packages covered, 17 ledgered. 1069 tests green across the three. Claude-Session: https://claude.ai/code/session_011m13wbaZziPveBdtQthrXd Co-authored-by: Claude <noreply@anthropic.com>
…were opting the driver call sites out of the check they had just been given (#4311) Onboarding a package is supposed to make its `typecheck` mean something. These three declared it while 111 driver-call arguments were cast to `any` — the gate went green over call sites the type checker was not allowed to read. Stripping the casts produced 66 fresh errors, every one real: - 65 were a missing `object` on the query literal, the same defect the first commit fixed 127 times in the sites that were visible. - 1 was a silent runtime bug. `sql-driver-aggregate-temporal-output.test.ts` asked for `orderBy: [['id', 'asc']]`; the driver reads `item.field`, a tuple has none, so `if (item.field)` was false and the sort never reached SQL. The helper is named `viaFind` and exists to read a column in order — it had been reading whatever order the rows came back in, in the one file whose subject is output ordering. The tuple form appears nowhere else in the repo. 43 of the casts were needed by nothing at all. Exactly 2 are load-bearing and stay: `sql-driver-filter-no-silent-drop` and `memory-filter-ast-vocabulary` both feed `where` shapes `isFilterAST()` refuses, deliberately, to prove the driver throws rather than dropping the condition — those now say `where as FilterCondition` next to a comment, so the one honest cast in the package is not camouflaged by a hundred idle ones. Verified: `turbo run typecheck` 122/122 (three new), `pnpm test` 132/132, eslint, the 12 lint.yml check gates, coverage ratchet self-test 18/18. Claude-Session: https://claude.ai/code/session_011m13wbaZziPveBdtQthrXd Co-authored-by: Claude <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
) Two conflicts, both in `driver-sql`, both where #4363's paged-read work landed on the same lines as this branch's contract fixes. `sql-driver.ts` — `findOne`. main refactored the shared read path into `findRows(object, query, options, singleRowLookup)`; this branch removed the undeclared bare-id branch above it. Kept both: the bare-id branch stays gone and the surviving call is main's `findRows(..., true)`, so a single-row lookup keeps the `singleRowLookup` ORDER BY decision #4363 introduced. The method's doc comment named `find()` as the path the removed branch skipped; it now names `findRows()` and what that path actually does. `sql-driver-pagination-conformance.test.ts` — took main's version whole (it adds the `PAGINATION_UNORDERED_CASES` suite) and re-applied this branch's two transforms to it: 18 `as any` casts on driver-call arguments dropped, and the 15 missing `object` keys they had been hiding added back. One of those 15 was a codemod misfire caught in review, not by tsc: `sqlOfFind` passes a whole query through, and the rewrite read its `query` parameter as an id (`where: { id: query }`) — type-clean, semantically nonsense, and it would have made every assertion in that block test the wrong statement. The helper now takes `Omit<QueryAST, 'object'>` and spreads it. Audited every generated `where: { id: … }` in the branch for the same shape; that was the only one. Verified on the merge result: `turbo run typecheck` 122/122, `pnpm test` 132/132, eslint, coverage ratchet 60/77 with self-test 18/18, release-notes gate. Adds the changeset this branch was missing. Claude-Session: https://claude.ai/code/session_011m13wbaZziPveBdtQthrXd Co-authored-by: Claude <noreply@anthropic.com>
github-actions
Bot
added
documentation
Contributor
📓 Docs Drift CheckThis PR changes 4 package(s): 111 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:
|
…udit` (#4311) `content/docs/references/data/driver.mdx` is generated from the Zod schema, so declaring a new `DriverOptions` key leaves it stale until `gen:docs` runs. `@objectstack/spec check:docs` caught it — the generated table is what a driver author reads to learn which options exist, and it had been complete for every key except the one this branch adds. `pnpm --filter @objectstack/spec gen:schema && gen:docs`; the diff is the one row. 252 generated files back in sync. Claude-Session: https://claude.ai/code/session_011m13wbaZziPveBdtQthrXd Co-authored-by: Claude <noreply@anthropic.com>
…4311) `check:api-surface` reports every public export against a committed snapshot, so a new type — additive, 0 breaking — still has to be acknowledged rather than appearing unannounced in a consumer's build. One line. Also ran the other fifteen `@objectstack/spec` gates against this branch rather than waiting to meet them one CI round at a time: docs, generated, skill-refs, skill-docs, exported-any, authorable-surface, spec-changes, upgrade-guide, liveness, empty-state, variant-docs, strictness-ledger, react-blocks, react-conformance, skill-examples — plus `check:i18n` / `check:i18n-coverage` and `check:generated --reconcile-only`, which are the remaining steps of the CI job that failed. All green; only this snapshot needed regenerating. The pattern behind both this and the previous commit: `pnpm build` does not run the generators, so a schema edit type-checks and tests clean locally while two committed artifacts derived from it sit stale. Worth knowing before the next spec change — the generators are `gen:*` in `packages/spec`, and their gates are the `check:*` beside them. Claude-Session: https://claude.ai/code/session_011m13wbaZziPveBdtQthrXd Co-authored-by: Claude <noreply@anthropic.com>
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.
Burns down the largest block of #4311's code-tier ledger:
driver-sql(241),driver-sqlite-wasm(27) anddriver-memory(23). All three now declaretypecheckand leave the DEBT ledger — 60/77 packages covered, 17 ledgered (was 57/20).#4311 filed these as one playbook: author-tier literals handed to a parsed-tier parameter. That is what 127 of the 292 were. 165 were the types being wrong, and only a
tscthat had never run could have kept them invisible.What the errors actually were
bypassTenantAuditwas never onDriverOptionsSchema— 118SqlDriver.auditMissingTenantreads it. Its own warning text tells callers to pass it ("set bypassTenantAudit:true to silence").ObjectQLEnginesets it for system-context calls (engine.ts:1069).service-settingsandservice-datasourcesend it on global-scope writes.packages/verifydocuments it. Only the schema had never heard of it — so every driver read went through(options as any)and every caller through an untyped options object, and nothing on either side type-checked.Declared now, with the limit stated in the schema: it silences a diagnostic, it never changes which rows a write touches.
The same
as anycoveredtimezone,tenantId,tenantIdsandpreserveAudit— all four long since declared. All seven casts are gone, so the next undeclared option fails the build instead of hiding behind one.findOne(object, id)was on no contract — 41An undeclared
typeof query === 'string' | 'number'branch inSqlDriver, used by nothing outside this package's own tests, and answered differently by the other two drivers:findOne('task', 't1')doesSqlDriverMemoryDriver{0:'t',1:'1'}MongoDriverquery.where→undefined→ an arbitrary rowIt also bypassed
find(), skipping field selection, temporal coercion, and the deterministic-order tie-breakerIDataDriver.findexplicitly promises. Removed; call sites spell the lookup as{ object, where: { id } }.initObjectsdid not declare thetenancyit consumes — 4Each object flows into
computeAndRecordTenantField, which readsobj.tenancyto pick the tenant column and set the sticky opt-out.registerExternalObjecthad it right all along.AnalyticsQueryhad no author tier — 20timezoneis.default('UTC')andCube.publicis.default(false), so the parsed types require both while authors write neither. AddedAnalyticsQueryInputbesideAnalyticsQuery(the existingQueryInput/QueryASTpattern) and routed the literals through the schema, so the parse itself proves the default lands.defineCube— already in spec for exactly this — does the same job for the cube.InMemoryDriver.createdeclared no return type — 3TS inferred the literal it builds, so every column of the created row except
id/created_at/updated_atvanished from the caller's view.The second commit: 111
as anycastsOnboarding a package is supposed to make its
typecheckmean something. These three declared it while 111 driver-call arguments were cast toany. Stripping them produced 66 fresh errors, every one real:object— the same defect, in the sites the checker had not been allowed to read.sql-driver-aggregate-temporal-output.test.tsasked fororderBy: [['id', 'asc']]. The driver readsitem.field; a tuple has none;if (item.field)was false and the sort never reached SQL. The helper is calledviaFindand exists to read a column in order — in the one file whose subject is output ordering. The tuple form appears nowhere else in the repo.43 casts were needed by nothing at all. Exactly 2 are load-bearing and stay:
sql-driver-filter-no-silent-dropandmemory-filter-ast-vocabularyboth feedwhereshapesisFilterAST()refuses, on purpose, to prove the driver throws rather than dropping the condition. Those now readwhere as FilterConditionnext to a comment explaining why — one honest cast is easier to see when a hundred idle ones aren't standing next to it.Six call sites the checker could not see
findOne('users', alice.id as any)— the cast is what let an off-contract call survive a contract narrowing. They surfaced as runtime failures the moment the branch was removed, which is the point of the whole issue.Filed, not fixed: #4406
QueryAST.objectis required at the driver boundary and read by no driver — parameter 1 always wins, and nothing rejects a call where the two disagree. Adding it to query literals was the single largest edit here (127 keys, all inert at the call site). ReshapingIDataDriveracross four drivers is a design change, not a rider on a type-debt burn-down.Verification
turbo run typecheck— 122/122 (three new targets)pnpm test— 132/132 tasks; 1069 tests green across the three driverspnpm lint, the 12lint.ymlcheck gates, coverage ratchet self-test 18/18packages/spec/authorable-surface.jsonregenerated by the build (one added key)Generated by Claude Code