Skip to content

docs(analytics): correct four measured-false claims in the CubeRegistry docblocks - #15995

Draft
os-warren wants to merge 2 commits into
mainfrom
claude/issue-15019-cube-registry-infer-dead-code
Draft

docs(analytics): correct four measured-false claims in the CubeRegistry docblocks#15995
os-warren wants to merge 2 commits into
mainfrom
claude/issue-15019-cube-registry-infer-dead-code

Conversation

@os-warren

Copy link
Copy Markdown
Collaborator

Part of #15019. Deliberately not a closing keyword: this PR ships only the half that needs no ruling. The card's actual question — delete the published method, or wire it up — is unresolved and goes to the maintainer, so the card must stay open.

Headline: the card's premise is refuted, by execution

The card says CubeRegistry.inferFromObject "has no runtime caller" and that its three built-in default labels "are unreachable". The first half reproduces exactly. The second half does not.

inferFromObject is on the published surface of @objectstack/service-analytics@17.3.0 (publishConfig.access: public, files: ["dist", ...]), and its output reaches the wire. Driven against the built package — dist/index.js, the artifact a consumer imports, not src, not the declared type:

exports has CubeRegistry: true        exports has AnalyticsService: true

ROUTE 1  new CubeRegistry().inferFromObject('tasks', [...])
  count      -> count / "Count"
  hours_sum  -> sum   / "Hours (Sum)"
  hours_avg  -> avg   / "Hours (Avg)"
  fee_sum    -> sum   / "Fee (Sum)"
  fee_avg    -> avg   / "Fee (Avg)"
  registered under its own name: true

ROUTE 2  AnalyticsService#cubeRegistry is a CubeRegistry: true
  svc.cubeRegistry.inferFromObject('tasks', [{name:'hours',type:'number',label:'Hours'}])
  await svc.getMeta('tasks')  ->  the CubeMeta payload GET /api/v1/analytics/meta serves:
    tasks.count      -> title="Count"        type=count
    tasks.hours_sum  -> title="Hours (Sum)"  type=sum
    tasks.hours_avg  -> title="Hours (Avg)"  type=avg

getMeta() maps every registered cube's measure.label onto CubeMeta.measures[].title (analytics-service.ts:1495-1503), and inferFromObject self-registers (this.register(cube)), so all three labels are one published call away from a real response. "No caller in this repository" and "unreachable" are different claims, and only the first one holds.

The census, and what its anchor would miss

Whole tracked tree, no path restriction (the card and both triage comments scanned -- packages apps examples):

git grep -n "inferFromObject"          -> exactly 2
  packages/services/service-analytics/src/cube-registry.ts:73                    definition
  packages/services/service-analytics/src/__tests__/analytics-service.test.ts:89 its own unit test

Controls, same command shape, same cwd — a zero-hit grep proves nothing without one:

control files
registerAll 8
inferCubeFromQuery 7
inferMeasure 7
getAll 144
compileDataset 31

What the identifier anchor would miss, each scanned separately rather than assumed:

  • computed member access on a registry receiver — git grep -E "(cubeRegistry|registry)\s*\[" returns 6 hits, all unrelated packages (cloud-connection, service-storage), none on a CubeRegistry. The scan is live and finds nothing here.
  • reflective enumeration — getOwnPropertyNames / getPrototypeOf / Reflect.get / prototype across packages/services/service-analytics: 3 hits, all Array.prototype.sort / Object.prototype.hasOwnProperty / a comment. No dispatch table.
  • a name assembled from fragments — a quote-anchored infer scan across packages apps examples: every hit is a different identifier (inferDriverTypeFromUrl, inferExpressionType, inferCubeFromQuery, inferMeasure) or prose.
  • out-of-repo consumers — the part no in-repo grep can answer. objectui at 24e027e: 0 hits for inferFromObject, 0 for CubeRegistry, with the control service-analytics firing on 12 files, so that scan is live and objectui is a measured non-consumer. cloud is not checked out on this box — NOT MEASURED. The npm consumer population is NOT MEASURABLE from here at all.

Every use of cubeRegistry in analytics-service.ts enumerated: registerAll (734), get (765, 935, 1492, 1563), size (818), register (1068, 1589, 1657), getAll (1493). inferFromObject appears in none of them.

The three labels are not one fact

  • label: 'Count' is minted unconditionally by the method.
  • FIELDLABEL (Sum) / FIELDLABEL (Avg) are minted only for number / currency / percent fields, and each embeds the field's own label.

They share one route (the method) but not one condition, and the composites are two per qualifying field, not one each. Separately: the string Count is emphatically not unreachable — analytics-service.ts:2089 and :2344 mint label: 'Count' on live paths (inferCubeFromQuery, inferMeasure). Removing site 3 would take nothing off the wire that is on it today.

What #14492's ruling site 3 actually says

The maintainer ruling of 2026-09-02 on #14492 lists cube-registry.ts:80 as one of five server-side built-in default sites to populate with builtinAggregate — i.e. the ruling was written on the assumption this code is live. The later disposition (PR #15017, merged) left the three Cube-metric sites alone on the ground that they never reach AnalyticsResult.fields[], and the epic PM's comment closing that card says in as many words that #15019 "stays with the maintainer".

What this PR changes: four measured-false documentation claims

Comment-only. Every changed line in the diff is a comment line (git diff -U0 filtered to non-comment lines is empty).

Class docblock. It said cubes reach the registry "from two sources: manifest definitions, and object schema inference". Neither half held. Two live sources were missing — a compiled dataset's Cube registered under the dataset's name by queryDataset (analytics-service.ts:1068, ADR-0021), and the ad-hoc Cube ensureCube / inferCubeFromQuery mints from the members a query references (:1589, gated by assertInferableCube, #3867). And "object schema inference" is inferFromObject, which nothing here calls. Note that inferCubeFromQuery does not deliver it either: it infers from the QUERY's referenced members, never from the object's field schema — the object name is only checked for existence.

Method docblock. Three of its five "heuristic rules" were false. Answered by the run above, not by reading:

documented measured
number fields to sum, avg, min, max measures sum and avg only. hours_min / hours_max absent
boolean fields to a count measure (count where true) boolean becomes a dimension and nothing else; no measure minted for active
all non-computed fields to dimensions all 5 fields became dimensions; the fields parameter carries no flag to exclude on
a default count measure is always added true — count, labelled Count
date / datetime to time dimensions, standard granularities true — type=time, ["day","week","month","quarter","year"]

The method docblock now also records what the method is: published, uncalled in-repo, still callable by consumers, and wire-reaching through getMeta().

A patch changeset, not skip-changeset. I assumed this was comment-only-therefore-invisible and measured it false. A/B on the emitted bytes, same tree, only this file's edit reverted and restored:

POST  cjs=58976482... esm=488dcebc... dts=e0c4bf53...
PRE   cjs=077b7d5d... esm=830b9471... dts=ddd6e6bd...
emitted CJS  : DIFFERS      emitted ESM : DIFFERS      emitted d.ts : DIFFERS (control fired)

The method docblock survives into the emitted bundle (dist/index.js:66); the class docblock does not. So this edit does change published bytes in all three artifacts, and a patch is the honest disposition. Mutation proven on disk by marker counts (Three sources write to it 0, Cubes can be registered from two sources 1 under mutation); restore proven by blob equality against the HEAD blob d7ab4efe... plus an empty git diff HEAD on the path, under a trap on EXIT INT TERM with absolute paths.

The open question, for the maintainer

Triage authorized option 1 (delete) as covered by "the enforce-or-remove precedent". Measured, that precedent does not reach this case: ADR-0049 is titled "Spec must not declare security properties the runtime does not enforce" and its trichotomy is about spec properties naming an access-control boundary; ADR-0078 extends it to metadata instances. Neither governs a TypeScript method on a published service class. And ADR-0059 concentrates the breadth gate on packages/spec/api-surface.json — there is no surface ledger for @objectstack/service-analytics, confirmed: a recursive grep for inferFromObject under packages/spec/api-surface/ returns nothing. A removal here would be invisible to every gate in the repo except the changeset's own major declaration.

Options, with their measured costs — no ranking on axes, because this dispatch carried no decision frame and I am not inventing one:

  1. Delete the method, its unit test, and the method docblock. Nothing in this repo breaks (measured: 92 test files / 2002 tests pass, and the only caller is the test being deleted). It is a declared-breaking removal from a published v17 package: major changeset plus an ADR-0087 disposition marker. Cost that is not visible from here: any npm consumer calling it breaks with no deprecation window, and no gate would have caught the removal.
  2. Wire it as a real cube source. New capability — needs a caller, getMeta() exposure, and a decision on localizing composite defaults that embed a field label. Triage already routed this to a separate card at manual floor. It would also make cube-registry.ts:80 live again, reopening analytics 契约:AnalyticsResult.fields[] 增加可选 builtinAggregate 鉴别字段,服务端内置聚合默认 label("Count" 等 5 处硬编码)不再以英文字面量出线(objectui#7258 裁 B 的生产者半边) #14492's conclusion for that site.
  3. Keep it, documented (this PR), plus optionally pin the three labels. The existing unit test asserts the cube's shape — cube.measures.count defined, hours_sum / hours_avg defined, four dimension types — and asserts none of the three labels. So the labels the card is about are unpinned published behaviour today, whichever way the ruling goes. Cost: pins make option 1 marginally more expensive later.

Verification

Head for every run below: 02fdc7b05 (the final commit on this branch).

  • Gate family derived by node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack with no paths passed — 48 commands from the actual change set (2 paths, committed). The first derivation warned STALE TREE; origin/main was merged in (clean, no merge=os-regen path among the 10 files it brought) and the derivation re-run at 02fdc7b05 with no stale warning.
  • 48/48 exit 0. Each captured as a single redirected command with the exit code read immediately after, never through a pipe.
  • pnpm check:dual-build-cjs-loads first answered exit 3 PREREQUISITE NOT MET (unbuilt worktree). Prerequisite built (pnpm build --concurrency=2, 72/72 tasks) and re-run: exit 0, verdict "103 published require entry point(s) across 66 package(s) load; 619 emitted CommonJS file(s) parse". check:dts-closure and check:nul-bytes re-run on the built tree: exit 0 (164/164 declaration files; 7710 text files scanned, no control bytes).
  • pnpm --filter @objectstack/service-analytics run typecheck exit 0; vitest run --maxWorkers=292 test files, 2002 tests, all passed.
  • node scripts/check-adr-0087-registration.mjs --base origin/main exit 0 and --self-test exit 0. Not breaking, so no ADR-0087 marker is owed; the gate agrees.

Heavy runs went through scripts/pm/os-verify-lock.sh; verdict lines, not bare $?.

🤖 Generated with Claude Code

https://claude.ai/code/session_01XpTx2tbq3pZRYAdoGt6E6Y


Generated by Claude Code

…ry docblocks

The class docblock advertised "two sources: manifest definitions, and object
schema inference". Neither half held. Two live sources were missing — a
compiled dataset's Cube, registered under the dataset's name by queryDataset
(ADR-0021), and the ad-hoc Cube ensureCube/inferCubeFromQuery mints from the
members a query references. And object schema inference is inferFromObject,
which no path in this repository calls: its only in-tree caller is a unit test.

inferFromObject's own "heuristic rules" list was wrong in three of its five
bullets. Driving the BUILT package through its published entry:

  - number/currency/percent fields mint one sum and one avg measure each. The
    documented min and max measures do not exist.
  - boolean fields become a boolean dimension and nothing else. The documented
    "count measure (count where true)" is not minted.
  - every field becomes a dimension; the documented "all non-computed fields"
    implies an exclusion the code does not have, on a parameter carrying no
    flag one could exclude on.

The two accurate bullets (a default count measure; date/datetime fields
becoming time dimensions granulated day/week/month/quarter/year) are kept and
stated in the form the run produced.

Comment-only: every changed line in the diff is a comment line. The corrected
text is what ships in dist/index.d.ts, where consumers read it, so this carries
a patch changeset rather than a skip-changeset claim.

This commit settles nothing about whether the published method is removed or
wired up as a real cube source. That measurement and its options go to the
maintainer.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XpTx2tbq3pZRYAdoGt6E6Y
@github-actions github-actions Bot added the size/s label Sep 5, 2026
@github-actions

github-actions Bot commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

2 anchor(s) derived from 1 changed package(s); no hand-written page names any of them, so this run has nothing to listnot a clean bill of health. This check sees only pages that NAME a derived anchor: one that documents this change in prose, or enumerates it in an authoring dialect, names none and stays invisible to it on every run.

What this run could not see
  • 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 — 9 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 f7db8f4fd268a86a08c62ae4894cf7417720f8c9packageMentionDocs.

Which tree this was computed on

This run read content/docs from 967d4caab6e68b34cb6fe5ecc6c488b4d29c31ad — the merge of head 02fdc7b056c97cb28128dbb7e6e0b664f51d630e into base f7db8f4fd268a86a08c62ae4894cf7417720f8c9, 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 967d4caab6e68b34cb6fe5ecc6c488b4d29c31ad && git checkout 967d4caab6e68b34cb6fe5ecc6c488b4d29c31ad
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin f7db8f4fd268a86a08c62ae4894cf7417720f8c9 02fdc7b056c97cb28128dbb7e6e0b664f51d630e && git checkout -B drift-repro f7db8f4fd268a86a08c62ae4894cf7417720f8c9 && git merge --no-ff 02fdc7b056c97cb28128dbb7e6e0b664f51d630e

node scripts/docs-audit/affected-docs.mjs --json f7db8f4fd268a86a08c62ae4894cf7417720f8c9

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation size/s tooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants