Uh oh!
There was an error while loading. Please reload this page.
fix(service-analytics): wire the typecheck script and clear the 10 errors the unwired gate hid - #13188
Merged
Merged
Conversation
Contributor
📓 Docs Drift CheckNothing in this diff resolved to a documentable surface (no symbol, route or SDK anchor derived from 0 changed package(s)), so this run has no opinion about the docs. What this run could not see
Coarse fallback — 0 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): |
os-elon
marked this pull request as ready for review
August 29, 2026 10:16
os-elon
enabled auto-merge
August 29, 2026 10:16
Contributor
Uh oh!
There was an error while loading. Please reload this page.
This was referenced Aug 29, 2026
Merged
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 freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Fixes#12939
packages/services/service-analytics/package.jsondeclared onlybuildandtest. Roottypecheckisturbo run typecheck, which no-ops a package that has no such script and reports success;buildis tsup (esbuild — its DTS pass processes declarations only) andtestis vitest (esbuild transform). Neither type-checks.This wires the one-line script, clears all 10 standing errors, and graduates the package out of the
check:type-check-coverageDEBT ledger.Authored in Claude Code session
session_012WkdHQwHr2KQmaX7P1BHzi(https://claude.ai/code/session_012WkdHQwHr2KQmaX7P1BHzi) — recorded here in prose because the signature footer's session URL does not survive a body edit.The star reading: the new gate is real, not a phantom
The cheap way to make a new
typecheckscript green is to exclude the tests from the program. That was not done, and the evidence is direct —tsc --noEmit --listFilesin the package, counted against what is on disk:*.test.tsfiles on disk undersrc/*.test.tsfiles listed in the tsc programerror TSlinestsconfig.jsonis byte-identical —includeissrc,excludeisnode_modulesanddist. Noanywas added, no@ts-expect-error, no@ts-nocheck,strictuntouched.Ablation, to prove the gate can fail. With the fix committed, the TS6133 import was reintroduced into
src/__tests__/analytics-service.test.ts; the mutation was confirmed on disk by blob hash (b1e1af24tocf7b0dd4) before measuring.pnpm --filter @objectstack/service-analytics typecheckthen exited 2 withanalytics-service.test.ts(10,1): error TS6133. Restored withgit checkout HEAD --on an absolute path; the blob is back tob1e1af24andgit diff HEADis empty. So the gate reads the test files at runtime, not merely in--listFiles.Which precedent, and why this one
The two in-repo precedents for this class differ, and the choice between them is a measurement rather than a preference:
service-settings(theservice-settingshas notypecheckscript — turbo silently no-ops it, and ~5 test files carry pre-existing type errors behind the unwired gate #7925 instance, PR fix(service-settings): wire thetypecheckscript and clear the 14 errors the unexecuted gate hid #7959) — its tests were already inside the package program, so it took a plaintsc --noEmitand repaired every error.rest(thepackages/resthas NO tsc program that compiles any test file — 149 test files unchecked, and AGENTS.md names this exact shape as how a@ts-expect-errorbecomes a phantom check #12542 instance) —packages/rest/tsconfig.jsonexplicitly excludes**/*.test.tsand**/*.spec.ts, so it needed the siblingtsconfig.test.jsonplus a per-filetest-typecheck-debt.json.service-analyticshas no test exclusion and its tests sit insrc/__tests__/**inside anincludeofsrc, so it is the first shape. No third pattern was invented.The 10 errors — all 10 repaired, none deferred
Every one was a stale test, not a source defect. No non-test source file changed.
src/__tests__/measure-source-field-gate.test.ts.promise.catch(fn)does not drop the resolved branch from the type, soservice.query(...).catch((e) => e as Error)was the unionAnalyticsResult | Errorand everyerr.message/err.field/err.member/err.paramread was a property access againstAnalyticsResult. A localrefusalOf()helper narrows once throughthen, giving the fulfilled branch thenevertype. One deliberate behaviour note, confined to the test failure path: a query that is not refused now fails by name instead of surfacing later asexpect(undefined).toMatch(...).src/__tests__/objectql-timedimension-projection.test.ts. TheTABLEfixture was inferred as an array of{ id: number; due_date: string; priority: string }while theengine.aggregate()stand-in indexes it by a computedstringkey. Annotated as theRowalias the file already declares (a Record keyed bystringwithunknownvalues).src/__tests__/analytics-service.test.ts. An unusedAnalyticsDriverCapabilitiestype import. The capability literals in this file are inlinectxobjects already checked contextually at eachcanHandlecall site, so the import carried no coverage and is removed rather than artificially exercised.One correction to the card's premise
The card states that "no CI lane runs tsc over this package at all". That is not quite right, and the difference matters for how the repair is read.
lint.ymlrunspnpm check:type-check-debt(check-type-check-coverage.mjs --re-measure), which re-runstsc --noEmitper ledger entry — and this package had a DEBT entry (errors: 10). So tsc did run over it in CI; what it asserted was only that the count must not grow, never that it is zero, and the entry's own note records the count walking 3 to 7 to 10 unremarked. The honest statement is that no lane required the package to be clean, and thetypechecklane skipped it entirely. Everything else in the card re-measured exactly: scriptsbuildandtestonly, 10 errors, same three files, same error classes.Column numbers drifted slightly from the card for the TS7053 pair — measured at
(67,43)and(68,34), not(67,68).Ledger graduation
check:type-check-coveragebefore and after, from the gate's own verdict line:check-type-check-coverage: OK — 66/78 workspace packages type-checked (plus the root), 12 in the DEBT ledger (372 frozen raw errors)check:type-check-debt—surplus: none — every entry sits exactly at its measurement, so any new error is red.RECONCILED forbids carrying both a
typecheckscript and a DEBT entry, so the entry is deleted in the same commit as the script is added.Out of scope
#12940 is not addressed here and remains open. It is held behind this card and touches the
executeAggregateconfig-mirror annotations in the same package. Wiring the gate surfaced no errors on that surface — the package type-checks at 0 errors — so nothing was ledgered or deferred on its behalf. The gate now exists, so that card's narrowing will be type-checked when it lands.Verification
All at final head
6cd14ca62.pnpm --filter @objectstack/service-analytics typecheck— exit 0 (script name echoed, so not a zero-match no-op)pnpm --filter @objectstack/service-analytics exec vitest run --maxWorkers=2— 83 test files, 1805 tests, all passedpnpm lint(repo-wideeslint . --no-inline-config) — exit 0node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstackat edit time, from the real changed set, and re-derived after the changeset landed. All ran green, includingcheck:type-check-coverage,check:type-check-debt,check:cross-package-test-inputs,check:engine-double-contract,check:where-matcher,check:test-source-alias,check:type-source-resolution,check:dual-build-cjs-loads,check:published-filesand the changeset family.Two gates in the derived family returned NOT MEASURED, neither a pass nor a failure:
scripts/pm/check-half-states.mjs— exit 3, PREREQUISITE NOT MET: no usable GitHub credential in this container, so no issue was listed and no predicate ran. Its self-test half (pnpm check:pm-half-states, 1551 cases) passes. It is a PM board-hygiene gate pulled in by the changeset path, unrelated to this diff's content.scripts/check-test-completeness.mjs— exit 1 on a usage error; it requires a turbo test log argument that only CI produces. Not a failing measurement.Changeset
patchfor@objectstack/service-analytics, matching the #7925 precedent, which shipped one for the same shape of repair. The publishedpackage.jsongains a script, so this is not askip-changesetdiff.