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 0 — content/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 18474 ⇒ exit 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

@os-try-charles
os-try-charles marked this pull request as ready for review September 16, 2026 15:12
@os-try-charles
os-try-charles added this pull request to the merge queue Sep 16, 2026
Merged via the queue into main with commit 9c1897e Sep 16, 2026
38 checks passed
@os-try-charles
os-try-charles deleted the claude/issue-17483-label-bearing-dimension-class branch September 16, 2026 15:25
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