Skip to content

docs: name the reference dimension class instead of listing two of its four members - #18474

Merged
os-try-charles merged 1 commit into
mainfrom
claude/issue-17483-label-bearing-dimension-class
Sep 16, 2026
Merged

os-try-charles merged 1 commit into
mainfrom
claude/issue-17483-label-bearing-dimension-class

Conversation

@os-try-charles

Copy link
Copy Markdown
Collaborator

Part of #17483.

Two authored pages stated the label-resolving dimension class by naming two of its four
members. PR #17470 (card #16390) deleted service-analytics's private two-member
LOOKUP_TYPES subset and routed every site that classified a dimension through spec's
referenceTargetOf instead, so a user or tree dimension now yields a label exactly as a
lookup one already did. Both sentences were true when the card was filed and are false on
main today; the card's Blocked-by: fence is spent (#16390 is closed, PR #17470 merged as
113050e2c2).

The two sentences, before and after

All tree readings below were taken in this worktree at 8fe5cb8e5 (the merge base of this
branch) between 14:13Z and 14:39Z on 2026-09-16; the diff itself is commit 39528063a.
origin/main had moved on to 0fe38a92f (committed 14:08:52Z) by the last fetch at 14:39Z.

Site 1 — content/docs/data-modeling/analytics.mdx, line 211 (before), line 211 (after)

Before:

  • Dimensions — a select dimension returns its option label (not the stored
    value), a lookup / master_detail dimension returns the related record's display
    name
    (not the FK id), and a date dimension with a dateGranularity returns a human
    bucket label …

After:

  • Dimensions — a select dimension returns its option label (not the stored
    value), a reference dimension (any field whose stored value is another record's id,
    a user person axis included) returns the related record's display name (not the FK
    id), and a date dimension with a dateGranularity returns a human bucket label …

Site 2 — content/docs/ui/dashboards.mdx, line 146 (before), lines 146–152 (after)

Before:

  • A sortBy naming a select or lookup dimension orders by the display label
    the rows render (the option label / the related record's name), not the stored value or
    foreign-key id — and the label is resolved before limit applies, so a top-N by name
    truncates the right N. Sorting by a measure (the common case) involves no label lookup
    and is unaffected.

After:

  • A sortBy naming a select or reference dimension orders by the display label
    the rows render (the option label / the related record's name), not the stored value or
    foreign-key id — and the label is resolved before limit applies, so a top-N by name
    truncates the right N. Reference is the whole class of fields whose stored value is
    another record's id, so a user person axis sorts by name too. Sorting by a measure (the
    common case) involves no label lookup and is unaffected.

Why the class is named rather than the members listed

Triage's sharpest instruction was: add the missing two, but do not restate the class by
listing members if a name for it exists. A name does exist, in three places at once:

  • spec declares the set as REFERENCE_VALUE_TYPES (packages/spec/src/data/field-value.zod.ts:153);
  • the implementation's own header calls it "the reference class"
    (packages/services/service-analytics/src/dimension-labels.ts:11), and
    createOrderLabelResolver's doc comment says a dimension is label-bearing when its field
    "carries select options or belongs to the reference class and names a target";
  • content/docs/** already uses the same noun in prose — "a reference field"
    (content/docs/api/data-api.mdx:199), "the reference field types"
    (content/docs/data-modeling/queries.mdx:327, content/docs/protocol/objectql/query-syntax.mdx:739).

So both sentences now name the class. Appending user and tree to each list would have
produced a four-item prose copy of a four-item const — a second copy that goes stale the
next time the set moves, which is exactly the failure this card is. A named class with a
definition ("any field whose stored value is another record's id") stays true when a fifth
member lands.

Two deliberate choices inside that:

  • "reference" is bolded, not backticked. There is no reference field type; there is a
    reference property on a reference field. Code-formatting the word would have invented a
    fifth field type. Bold marks it as the class noun, matching how dashboards.mdx already
    bolds select and derived measure.
  • user is named as an example, not as a list member. The card's finding is directional:
    a reader asking "does a person axis render a name?" read the old enumeration and concluded
    no. A class name alone would leave that reader to classify user themselves, which is the
    very belief gap analytics: 数据集维度只解析 lookup 字段,user 字段维度返回原始用户 id —— 以人为轴的图表/报表标签是一串 id #16390 was filed to close. An open example ("a user person axis included")
    answers the question inside the paragraph and — unlike a closed enumeration — claims no
    completeness, so a future fifth member does not falsify it.

How REFERENCE_VALUE_TYPES' members were confirmed

By module import of the built package, not by grepping the declaration — a grep over that
file also matches four TSDoc comments that quote the same members, and a comment miscounted as
a row is a known way to get this wrong by one. Run from
/home/user/objectstack-issue-17483/packages/spec after pnpm --filter @objectstack/spec build:

import { REFERENCE_VALUE_TYPES, referenceTargetOf } from '@objectstack/spec/data';
MEMBERS=["lookup","master_detail","user","tree"]
SIZE=4
has(user)=true has(tree)=true
CONTROL has(select)=false has(text)=false
referenceTargetOf({type:user})=sys_user
referenceTargetOf({type:tree,reference:"dept"})=dept
CONTROL referenceTargetOf({type:select})=undefined

SIZE=4 is read from the live Set, so it counts members and cannot count a comment. The two
negative controls show the probe discriminates rather than answering true to everything.

Site 2's absence from drift output is structural

content/docs/ui/dashboards.mdx names neither master_detail nor any symbol in PR #17470's
diff — it states the rule by its inputs (sortBy, select, lookup, "display label"),
and shares no identifier with the emitter that implements it. An emitter-only diff
therefore cannot list it, on that run or on any run; the drift bot's own "What this run could
not see" section states that limit by name. This page was found by a hand sweep over output
phrasings, not by the tool. Its absence from any drift output is structural and is never
evidence that it is fine.
Both pages were re-derived by hand for this PR: neither contained
the string user or tree as a field type anywhere (0 hits in each) before this change.

Changeset: none, and the files[] readings behind that

Measured, not assumed — the criterion is whether anything published moves.

reading (all taken 14:35Z, tree 39528063a) value
paths this PR touches content/docs/data-modeling/analytics.mdx, content/docs/ui/dashboards.mdx
package directories containing a touched path 0content/docs/** is at the repo root, inside no package
non-private packages under packages/** and apps/** 70
of those, packages declaring no files[] at all 0 — every one declares it
files[] entries that escape their own package directory (begin with .. or /) 0
apps/docs (@objectstack/docs, the renderer of content/docs/**) private: true, files absent ⇒ never published
root package.json (@objectstack/spec-monorepo) private: true, files absent

files[] globs resolve relative to their own package directory, and no package declares an
escaping entry, so no published tarball can contain a repo-root content/docs/** path. The
one package that consumes these files is private. ⇒ nothing published moves; skip-changeset
applies. This was measured rather than taken from the docs-only reflex, which has been wrong
before on a package whose files[] ships its own CHANGELOG.md — that shape needs the touched
path to sit inside a package directory, and neither of these does.

Gates

Derived with node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack
over the two touched paths, from this worktree. 39 families derived, 39 run, 0 NOT MEASURED,
0 unrun
— reconciled by handing the exit codes back to the same tool
(--ran, exit 0, 14:38Z).

  • 38 exit 0.
  • Four of them first exited 3 (PREREQUISITE NOT MET) in an unbuilt fresh worktree
    (check:doc-formula-expressions, check:doc-security-posture, check:skill-examples,
    check:docs-transcript-drift). Exit 3 is neither a pass nor a finding; after
    turbo run build --filter=@objectstack/formula --filter=@objectstack/lint --filter=@objectstack/client-react
    (verify-lock VERDICT command-exit 0, 418 s) all four re-ran at exit 0.
  • One exit 1, not caused by this diff: check:cross-package-test-inputs reports
    packages/cli/test/init-created-files-summary.e2e.test.ts descending packages/spec/dist/
    with no declared glob reaching inside it. That gate is a scan of test sources and
    turbo.json; this diff contains 0 files with a .ts/.mts/.mjs/.js/.json extension, so
    its inputs are byte-identical to the merge base. Re-run after the builds above: same finding,
    same exit 1.

Repo-wide pnpm lint (eslint . --no-inline-config) was narrowed, and the narrowing is
measured
, not assumed:

  1. Population, read from eslint's own config: every files entry in eslint.config.mjs is
    **/*.{ts,tsx,mts,cts,js,jsx,mjs,cjs} or narrower. .mdx appears in none of them.
  2. File count, read from --format json: running eslint directly on both touched paths
    returns, for each, File ignored because no matching configuration was supplied.
    errorCount: 0, warningCount: 1, zero rules applied. This diff contributes 0 files to
    the linted population.
  3. Invariance for untouched files: type-aware linting is not enabled anywhere in this
    config — no parserOptions.project, no typed @typescript-eslint rules (stated at
    eslint.config.mjs:328) — so no .mdx edit can move the verdict on any file it does not
    contain.

Also run as an MDX parse check: pnpm exec fumadocs-mdx in apps/docs, exit 0 — both
pages still compile. CI's two path-scheduled jobs (Test Core shards and Build Docs) have no
local invocation and are declared to CI.

Risk

Prose only, in two bullets, on two pages. No packages/** file is touched; the spec fact the
prose now names was read out of the built package, not authored here. Rollback is a revert of
one commit.


Generated by Claude Code

…g two of its four members

`REFERENCE_VALUE_TYPES` is one class -- `lookup`, `master_detail`, `user`,
`tree` -- and all four store a related record's id and resolve to that
record's display name. Two authored pages enumerated only `lookup` /
`master_detail`, so a reader asking whether a person axis renders a name
read "no" off the enumeration.

Name the class in both sentences rather than growing the enumeration by two:
a prose list that mirrors a `const` is a second copy that drifts the next
time the set moves.

Claude-Session: https://claude.ai/code/session_017ef78bLdybu3AffehKkhfk
Co-authored-by: Claude <noreply@anthropic.com>

Copy link
Copy Markdown
Collaborator Author

PM review — ACCEPT

Reviewed against GitHub and against a detached worktree at origin/main, ⛔ not against the report's narrative.

⭐ The dispatch offered two options; the dev found a third and it is better than both

Triage's constraint was 「Add the missing two; ⛔ do not restate the class by listing members if a name for it exists」. The answer:

- a `lookup` / `master_detail` dimension returns the related
+ a **reference** dimension (any field whose stored value is
+ another record's id, a `user` person axis included) returns the related
- A `sortBy` naming a **select** or **lookup** dimension orders by the
+ A `sortBy` naming a **select** or **reference** dimension orders by the
+ **Reference** is the whole class of fields whose stored value is another
+ record's id, so a `user` person axis sorts by name too.

Two things at once, and the second is the one I would not have specified:

  1. The card's directional defect is answered inside the paragraph — a reader asking "does a person axis render a name?" now reads that it does.
  2. user appears as an EXAMPLE, not as a list member. ⇒ when REFERENCE_VALUE_TYPES gains a fifth member, this prose does not become false. A four-item list mirroring a const would have — and a prose copy of a const drifting is precisely why this card exists.

And the noun is not invented — I checked at source rather than taking it. packages/services/service-analytics/src/dimension-labels.ts:11 reads 「the reference class (REFERENCE_VALUE_TYPES: lookup, …)」, with :40 and :127 using the same noun. ⇒ the prose adopts the implementation's own vocabulary.

The spec fact, and whose reading each half is

leg instrument result
the dev's import from a built packages/spec MEMBERS=["lookup","master_detail","user","tree"] · SIZE=4 · has(user)=true · CONTROL has(select)=false
this seat's source read of the declaration, field-value.zod.ts:153-155 same four members

⭐ The dev explicitly noted that SIZE counts Set members, ⛔ not the four TSDoc comments in the same file that quote them — which is the #17141 lesson applied unprompted. ⛔ This seat could not reproduce the import leg (packages/spec/dist is absent here), so the import reading is theirs, recorded as theirs; two different instruments agree.

Carriers and fences

  • 2 files, +7 −4, docs only — ⛔ no packages/** byte.
  • check-clause2-carriers --pair 18474exit 0.
  • PR body first line Part of #17483.; a regex over the stored body finds zero closing keywords bound to a card number.
  • Commit trailer pair model-free, no card trailer.
  • skip-changeset, with the narrowing measured rather than asserted: the eslint population was read off eslint.config.mjs (every files entry is **/*.{ts,tsx,…}; .mdx in none), the file count off --format json (both touched paths return "File ignored because no matching configuration was supplied", 0 rules applied), and the invariance argued from there being no parserOptions.project anywhere. ⇒ an .mdx edit cannot move the verdict on an untouched file.
  • ⚠️ Site 2's absence from any drift output is structural — the card's own finding — and the PR body says so. ⛔ Never read as evidence it is fine.

The one red, and what I did with it — ⛔ NOT a new card

pnpm check:cross-package-test-inputs exit 1 on a diff of two .mdx files. ⛔ Not this PR's: that gate's inputs are byte-identical to the merge base, and the FAIL names packages/cli/test/init-created-files-summary.e2e.test.ts / packages/spec/dist/.

The dev marked it to file. I checked and it is #18348 — same FAIL line, same two paths, same mechanism. ⇒ ⛔ no duplicate card; recorded as that card's fourth independent reproduction (comment 5699392971).

⭐ The fourth adds a route the first three did not have: this dev reached it because four other gates first exited 3 (PREREQUISITE NOT MET) and forced a build it never intended. ⇒ the paying population is now measured as anyone whose worktree ends up with packages/spec/dist, for any reason. Three of the four reproductions were seats working on documentation.

⚠️ The dev's caveat — 「whether this is red on main in CI or only in a built local tree is UNVERIFIED by me」 — is already answered on that card's title and by its third reproduction; the answer is written back there so nobody re-derives it.

One place I overruled the dev, and the reason

Its noted, not filed: both repaired sentences still key the option arm on select, while the resolver keys it on the field having options. I verified at source — dimension-labels.ts:204, :210, :388 are all Array.isArray(meta.options) && meta.options.length > 0, ⛔ not one tests type === 'select'.

Its reasoning for not filing was careful: 「a docs list short by a member is an incompleteness, not a copy-it-and-it-fails error」. ⛔ I did not accept it, and the reason is this card's own precedent: #17483 was graded bug on exactly that shape — site 1 was a list short by two members, and the harm accepted was directional. Same sentence, same page, one class over. ⇒ filed as #18475, with the dev's counter-argument quoted in it so the grader weighs it first, and ⛔ with no fix prescribed.

Arming once the ready flip's re-run clears; ⛔ the arm is a separate act from the reading that clears it.

PM seat domain:devx · session session_017ef78bLdybu3AffehKkhfk · round 9 · reviewed head 39528063a8d04cc44a1bca5bc3165089209f3bd7 · 2026-09-16T15:12Z


Generated by Claude Code

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 skip-changeset PR has no user-facing published change; bypasses the changeset gate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants