Skip to content

refactor(service-analytics): derive the auto-bridge's engine view from the declared contracts - #12777

Merged
os-litant merged 3 commits into
mainfrom
claude/issue-11833-analytics-dataenginelike
Aug 27, 2026
Merged

refactor(service-analytics): derive the auto-bridge's engine view from the declared contracts#12777
os-litant merged 3 commits into
mainfrom
claude/issue-11833-analytics-dataenginelike

Conversation

@os-litant

@os-litantos-litant commented Aug 27, 2026

Copy link
Copy Markdown
Collaborator

Part of #11833

Replaces the consumer-local structural DataEngineLike in packages/services/service-analytics/src/plugin.ts with the declared IDataEngine / IObjectQLEngine members — the #4251 B3 sweep pattern, and the second and last of the two sites #11833 names. The first landed as PR #12011 (MERGED).

Everything below is split into MEASURED (a command was run and its own output is quoted) and INFERRED (reasoning from what was read). Every claim names a file and a line.

Note on spelling. This body was re-read after publishing and GitHub's sanitizer had eaten the TypeScript generic arguments (Pick … lost everything between the angle brackets). Where a generic must be shown below, square brackets stand in for the angle brackets; the real spelling is in the diff.


Why this says Part of rather than a closing keyword

An earlier revision of this body carried a closing keyword, on the strength of the card's 2026-08-27 unlock note — "#12010 does NOT ride on this card closing" — and of the endorsed rule that when a dispatch names N sites and only M land, the closing keyword is what changes, not the scope. Both named sites do land here, so that read was available.

The PM has since measured the sub-issue structure and reversed that guidance: #11833 carries has_children: true with sub_issues_summary: {total: 1, completed: 0}. #12010 is a real GitHub sub-issue, so a closing keyword on this PR would auto-retire a parent showing 0 of 1 children complete — as a merge side effect rather than anyone's decision. The endorsed rule is untouched; what changed is that the reason to hold the keyword here is the sub-issue edge, not the scope.

⭐ The card's question is fully answered by this PR. The PM will act on that deliberately after the merge, with the reason recorded on the card.

⚠️ Deliberately, no sentence anywhere in this body places a GitHub closing keyword next to an issue number — the reference parser matches close / fixes / resolves plus a number regardless of the surrounding sentence, so even a line explaining that this PR does not retire the card would retire it. Prose about the card uses other verbs on purpose.


MEASURED — the five members line up

Re-read on origin/main at 87d3f9a0a, not inherited from the dispatch table:

local memberdeclared equivalentverdict
execute?IDataEngine.execute?contracts/data-engine.ts:250compatible, derived
aggregateIDataEngine.aggregatecontracts/data-engine.ts:230derived; one real mismatch, below
getObject?IObjectQLEngine.getObjectcontracts/objectql-engine.ts:207 (and :90 on the registry view), returning ServiceObject or undefinedcompatible, derived
resolveEffectiveDatasource?IDataEngine.resolveEffectiveDatasource?contracts/data-engine.ts:321compatible, derived
getDriverForObject?IDataEngine.getDriverForObject?contracts/data-engine.ts:342compatible, derived

No member is a fork, so no contract moves and nothing is cast past. The replacement (square brackets standing in for angle brackets, per the note above):

type DataEngineLike =
Pick[IDataEngine, 'aggregate']
& Partial[Pick[IDataEngine, 'execute' | 'resolveEffectiveDatasource' | 'getDriverForObject']]
& Partial[Pick[IObjectQLEngine, 'getObject']];

getObject was the one that needed #12248 most. MEASURED: substituting the contract's ServiceObject return produced no diagnostic at any of its call sites — plugin.ts:398 (obj?.fields?.[relationshipName], reading .type / .reference), the DimensionLabelDeps.getObjectFields wiring at :415 (whose declared return is a record of FieldMetaLite, dimension-labels.ts:39), pickDisplayField at :418, isExternalObject at :566 and getObjectFieldNames at :592. The 08-25 report predicted this substitution would "replace real typing with casts at ~10 call sites"; that prediction was made against the old unknown return and no longer holds — fork 3's repair is what removed the cost.

getDriverForObject keeps its call-site narrowing, as the contract asks it to.data-engine.ts:342's own docblock says consumers "keep narrowing the RETURN at the call site (a Pick over IDataDriver admits the full contract value); what this member ends is each of them re-inventing the MEMBER." So TemporalDriverSurface survives — no longer as a re-declared member, but as the annotation on the two locals that read it (plugin.ts:508, :530).

MEASURED — every member stays optional, and the profile is preserved exactly

aggregate stays required, exactly as the hand-written type had it: it is what the typeof svc.aggregate === 'function' probe at plugin.ts:214 uses to decide whether a registered 'data' service qualifies at all. Every other member stays optional. getObject is required on IObjectQLEngine, so the Partial wrapper around it is load-bearing rather than decoration — without it, a 'data' service that is not ObjectQL stops satisfying this view. Nothing became required; a degraded boot does exactly what it did before.

MEASURED — the aggregate enum, the one careful spot

Substituting the contract member with no other change produced exactly one new diagnostic, and it is the predicted one:

src/plugin.ts(259,11): error TS2322: ...
Types of property 'function' are incompatible.
Type 'string' is not assignable to type '"min" | "max" | "count" | "sum" | "avg" | "count_distinct"'.

That is the correct signal, and it is reported rather than smothered. What it says: the deleted structural type declared aggregations[].function as string, the contract declares the six-value AggregationFunction (data/query.zod.ts:149, reached through AggregationNodeSchema at :262), and nothing compiled the two against each other.

First-hand verification of the #12209 refusal, not inherited.ObjectQLStrategy.resolveMeasureAggregation (strategies/objectql-strategy.ts:1262) now refuses a custom-SQL measure at :1296, keyed on EXPRESSION_METRIC_TYPES, with invalidMemberError (INVALID_FIELD / 400). Read on this branch. Its docblock is also explicit that it deliberately does not key on "method is not one of the six", because an enum-invalid metric type "is OUR bug — the undeclared-500 tier".

How the gap is closed. Not by widening function back to string (that is what hid it), and not by a cast. The bridge parses the incoming method with the spec's own enum:

const parsed = AggregationFunction.safeParse(method);
if (!parsed.success) throw new Error(/* names the aggregation, the method, and the six */);
return parsed.data;

One vocabulary, no local literal list to drift, and AggregationFunction's error map already carries the retired array_agg / string_agg prescriptions. AggregationFunction is already a runtime import in this package (dataset-compiler.ts:4), so this adds no dependency. The refusal is a bare Error in the undeclared-500 tier, matching the tiering dataset-refusal.ts's module header assigns to internal-invariant and host-drift arrivals — enveloping it as a 400 would blame the caller for a cube they did not write. Reviewed and accepted by the PM against objectql-strategy.ts:1288-1294, which assigns an enum-invalid method to exactly that tier: this implements the prescribed tiering rather than inventing one, and no caller-visible accept/reject behaviour moves.

INFERRED: no authored analytics can trigger it. The one reachable producer of a non-aggregate method is refused earlier by #12209; resolveMeasureAggregation otherwise returns either a surviving metric type or one of a six-element alias list (objectql-strategy.ts:1318).

MEASURED — ablation at the compiler, with a control leg

Predictions were written to a file before either leg ran. Shared note, stated because it changes how the numbers read: this package's tsc program carries 10 pre-existing errors, all in src/__tests__/ (it has no typecheck script; it is covered through check:type-check-coverage's DEBT ledger, recorded at 10). So tsc exits 2 in both legs and the exit code is not the verdict — the verdict is whether a diagnostic lands on src/plugin.ts.

  • LEG 1 — MUTATION, under this PR's derived type: replace the parse with the unchecked forward function: a.method.
    • PREDICTED: 11 errors — the 10 baseline plus one TS2322 on src/plugin.ts reading Type 'string' is not assignable to type '"min" | "max" | …'.
    • OBSERVED: tsc exit = 2, 12 errors. The predicted TS2322 appeared verbatim, at src/plugin.ts(289,11), with the predicted elaboration. Honest correction: the count was 12, not 11 — the mutation also orphaned the helper, producing a second diagnostic src/plugin.ts(80,10): error TS6133: 'parseEngineAggregateFunction' is declared but its value is never read. That is an artifact of the mutation, not a second finding, and it is reported rather than rounded away.
  • LEG 2 — CONTROL, the same unchecked forward under origin/main's original structural type (main's plugin.ts already forwards a.method verbatim), written by redirecting git show origin/main:PATH into the file — the form that does not stage.
    • PREDICTED: exactly the 10 baseline errors, zero on src/plugin.ts — green.
    • OBSERVED: tsc exit = 2, 10 errors, plugin.ts diagnostics: (none). GREEN.

That pair is the point: the unchecked forward typechecks clean on main today and does not after this PR. The #4251 drift, demonstrated live at this seam.

Discipline, each leg: mutation confirmed on disk by grepping the injected marker and the displaced anchor (leg 1: injected 1, displaced 0; leg 2: original interface 1, derived type 0, unchecked forward 1) and by comparing git hash-object against the HEAD blob — never a bare git diff --stat. trap 'restore' EXIT INT TERM on an absolute path from git rev-parse --show-toplevel. Restore with git checkout HEAD -- ABSPATH, then disk == index == HEAD proved three ways (git diff --quiet, git diff --cached --quiet, empty git status --porcelain) plus a blob-hash equality check, with an empty hash treated as failure. After both legs the tree was back at blob 01ef2ea1c.

MEASURED — verification

All at final commit 3e041bcc6 (branch head; merge base 87d3f9a0a). Dependency closure built first, so every type judgement reads a freshly built .d.ts rather than a stale one: the upstream-closure build for @objectstack/service-analyticsos-verify-lock: VERDICT command-exit 0 · held the lock 157s.

⚠️The dispatch's pnpm --filter @objectstack/service-analytics typecheck is the pnpm zero-match trap here, and was NOT run as a result. This package declares no typecheck script (packages/services/service-analytics/package.json:16build and test only), so that command matches nothing, runs nothing and exits 0. Substituted with the invocation check:type-check-coverage itself uses for this ledger entry:

  • npx tsc --noEmit -p tsconfig.json in the package → EXIT=2, 10 errors, byte-identical to the pre-change baseline, zero on src/plugin.ts. The 10 are the ledger's recorded count and all sit in src/__tests__/.
  • pnpm --filter @objectstack/service-analytics testTest Files 83 passed (83) · Tests 1805 passed (1805) (baseline 82 / 1803; this PR adds one file with two cases) · VERDICT command-exit 0
  • pnpm --filter @objectstack/service-analytics buildDTS ⚡️ Build success in 4015ms, check-dts-emitted: 1/1 declared declaration file(s) present · VERDICT command-exit 0
  • pnpm lint (full repo, eslint . --no-inline-config, not narrowed) → VERDICT command-exit 0

Gate union re-derived live on the actual changed set, node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack, stderr confirming the tree of objectstack-ai/objectstack at 3e041bcc6, the --repo assertion holding, and 3 paths vs merge base under three-dot semantics. ⭐ It named no contract family. All named families ran; each quotes its own verdict line:

familyverdict
check:changeset-gate-self-testsEXIT=0 — 118 + 212 + 116 assertions
check:cross-package-test-inputsEXIT=0 — "20 package(s) read outside themselves, all declared"
check:objectql-double-limitEXIT=0 — "287 double(s) graded … none new"
check:objectui-changesetEXIT=0 — "objectui-range --self-test: all checks passed"
check:page-declaration-shapeEXIT=0 — "34 page entries across 2267 sources … all reach the kernel"
check:pm-half-statesEXIT=0 — "1515 cases pass"
check:published-filesEXIT=0 — "69 publishable package(s) of 78"
check:slot-lookupEXIT=0 — "ratchet holds: 107 unswept site(s) in 25 file(s), none new"
check:test-source-aliasEXIT=0 — "72 packages with tests scanned"
check:type-source-resolutionEXIT=0 — "94 tsc program(s) across 77 packages scanned"
check-adr-0087-registration.mjsEXIT=0 — "adds no declared-breaking changeset"
check-changeset-no-major.mjsEXIT=0 — "introduces no major bump"
check-ci-filter-parity.mjsEXIT=0 — "all 109 declared cross-package glob(s) … covered"
check-comment-mask-adoption.mjsEXIT=0 — "20 private comment-stripper(s) … all 20 recorded"
check-cross-package-test-inputs.mjsEXIT=0 — "all declared, and turbo.json hashes every declared glob"
check-empty-changeset.mjsEXIT=0 — "No empty-frontmatter changeset introduced"
check-plugin-teardown-shape.mjsEXIT=0 — "64 Plugin implementation(s) across 4892 source(s)"
docs-audit/check-affected-docs.mjsEXIT=0
docs-audit/check-drift-comment.mjsEXIT=0 — "56 cases pass across 5 fixture diff(s)"
pm/release-rehearsal-clone.mjs --self-testEXIT=0 — "self-test passed"
check:query-options-erasure (convention)EXIT=0 — "ratchet holds: 67 unswept non-test site(s) in 17 file(s), none new"
check:engine-double-contract (convention)EXIT=0 — "691 pinned, 134 in the DEBT ledger, 3 exempt"
check:where-matcher (convention)EXIT=0 — "309 matcher(s) discovered, 309 answer the combinator battery correctly or refuse it loudly"
check:type-check-coverage (convention)EXIT=0 — "65/78 workspace packages type-checked … 13 in the DEBT ledger"
check:type-check-debt (convention, the ratchet)EXIT=0 — "31 ledger entr(ies) re-measured in 212.7s, 1687 raw tsc error(s) total, none above its recorded number" — run after the full turbo run build over the packages closure (70 successful, 70 total), as the gate requires
check:nul-bytesEXIT=0 — "scanned 7111 text file(s) … no raw ASCII control bytes"

⚠️One family produced NO reading and is reported as such, not as a pass: the bare node scripts/pm/check-half-states.mjs invocation exited 3, and says so itself — "trigger-file index gathered nothing, so this result says NOTHING about whether the board carries half-states. It is not a clean board and it is not a dirty one — it is no reading at all." Its pnpm check:pm-half-states self-test form is green (above); the board scan needs an index this container does not have. NOT MEASURED.

service-analytics's DEBT ledger entry stays at 10; nothing is lowered and nothing is raised.

Tests added

src/__tests__/aggregate-bridge-function-vocabulary.test.ts, driving the real plugin auto-bridge through the fakePluginContext harness this package already uses:

  1. positive control — a declared aggregate reaches the engine as function: 'sum'. Without it, case 2 could pass because nothing reaches the engine for reasons unrelated to the vocabulary.
  2. the refusal — a method outside the six never reaches the engine (calls is empty), the message names the offending method, the aggregation and the whole legal vocabulary, and the error carries nocode, pinning the deliberate undeclared-500 tiering rather than leaving it to chance. The drift is staged with a cube object that never met CubeSchema's parse, which is the arrival path the tiering is written for.

Changeset: shipped, and here is the argument both ways

PR #12011 shipped skip-changeset, and that precedent was not copied across unexamined.

  • For skip-changeset: four of the five members are a pure alias swap, the replaced type is file-private, and the emitted JS for those is unchanged.
  • For a changeset (chosen): the fifth member is not a pure alias swap. The aggregate narrowing adds a real runtime boundary — a new throw path that did not exist — and changes what a malformed host cube produces from null per bucket into a loud, attributed refusal. "Unreachable via any authored path today" is an argument about likelihood, not about whether behaviour moved; it moved.

Shipped as patch on @objectstack/service-analytics (.changeset/analytics-bridge-engine-aggregate-vocabulary.md). No label is needed as a result, so nothing was written to the PR's labels.

Fences

Zero packages/spec — MEASURED, the diff against the merge base is three files: the plugin, the new test, the changeset. Nothing under content/docs/releases/**, docs/adr/**, .claude/**, skills/**, AGENTS.md or CLAUDE.md. Draft; ready was not flipped and auto-merge was not armed.

Out-of-scope finding

Filed as #12776 (unassigned, not fixed here; triage owns its routing): StrategyContext.executeAggregate (packages/spec/src/contracts/analytics-service.ts:300) declares aggregations[].method as string while IDataEngine.aggregate declares the six-value enum — the same slot, two types, one layer up from this seam. It is the reason the bridge needs a runtime parse instead of a compile-time guarantee, and repairing it is an accept-set narrowing on a published contract, which is a spec-seat call rather than a consumer-side one.


Generated by Claude Code

claude[bot]and others added 3 commits August 27, 2026 18:16
…the declared contracts
Replaces the consumer-local structural `DataEngineLike` in
`service-analytics/src/plugin.ts` with the declared `IDataEngine` /
`IObjectQLEngine` members - the #4251 B3 sweep pattern, and the second
of the two sites named by #11833 (the first landed as PR #12011).
The `aggregate` narrowing surfaced the mismatch the structural type hid:
the local declaration typed `aggregations[].function` as `string` where
the contract declares the six-value `AggregationFunction`. Closed by
parsing with the spec enum itself at the forwarding site - not by
widening back to `string` (which hid it) and not by a cast.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0194kbQJxUvv2yvsGRtuXpP5
Positive control (a declared function reaches the engine as `function`)
plus the refusal (a method outside the engine's six never reaches the
engine, and answers in the bare-Error/undeclared-500 tier rather than a
400 that would blame the caller for host drift).
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0194kbQJxUvv2yvsGRtuXpP5
@github-actionsgithub-actionsBot added size/m documentation Improvements or additions to documentation tests tooling labels Aug 27, 2026
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/service-analytics, touching 12 documentable anchor(s).

3 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:

  • content/docs/kernel/contracts/metadata-service.mdx(via getObject (symbol), getObject (literal))
  • content/docs/plugins/packages.mdx(via AnalyticsServicePlugin (symbol))
  • content/docs/protocol/objectql/state-machine.mdx(via /object/:name/state/:field (route))

1 release-owned page(s) also name something this change touched. These are read-only:

  • content/docs/releases/v17.mdx(via getLegalNextStates (sdk), meta.getLegalNextStates (sdk))

content/docs/releases/ is RELEASE-OWNED (AGENTS.md "Documentation Guardrails"): release
notes are written centrally at release time, and a code PR that edits them is the exact PR
that guardrail exists to stop. They are still audited — read-only. If one of them is actually
wrong, file an issue or open a dedicated docs-only PR; do not edit it here.

What this run could not see
  • 3 name(s) were too generic to anchor anything (single lowercase words)
  • the SDK route bridge reached 47 of 219 client-bound route-ledger rows — the other 172 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 172: 14 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 56 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 102 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.

Coarse fallback — 8 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json 4d5b4f83254ee1b7f53197073cfc6eee3025418apackageMentionDocs.

Which tree this was computed on

This run read content/docs from 2e8b88007378fd122b671fee5847c1aa4d2b354f — the merge of head 3e041bcc699db79a08333ab2cbcc933e54472711 into base 4d5b4f83254ee1b7f53197073cfc6eee3025418a, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 2e8b88007378fd122b671fee5847c1aa4d2b354f && git checkout 2e8b88007378fd122b671fee5847c1aa4d2b354f
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 4d5b4f83254ee1b7f53197073cfc6eee3025418a 3e041bcc699db79a08333ab2cbcc933e54472711 && git checkout -B drift-repro 4d5b4f83254ee1b7f53197073cfc6eee3025418a && git merge --no-ff 3e041bcc699db79a08333ab2cbcc933e54472711
node scripts/docs-audit/affected-docs.mjs --json 4d5b4f83254ee1b7f53197073cfc6eee3025418a

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs 4d5b4f83254ee1b7f53197073cfc6eee3025418a → pass the list as
args.docs, on the commit named under Which tree this was computed on.

@os-litant
os-litant marked this pull request as ready for review August 27, 2026 19:10
@os-litant
os-litant enabled auto-merge August 27, 2026 19:10
@os-litant
os-litant added this pull request to the merge queueAug 27, 2026
Merged via the queue into main with commit c8be110Aug 27, 2026
42 of 43 checks passed
@os-litant
os-litant deleted the claude/issue-11833-analytics-dataenginelike branch August 27, 2026 19:31
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationsize/mteststooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@os-litant