Skip to content

feat(spec): warn when a grid view rowColor declares no colors map (view/row-color-without-colors) - #15101

Merged
zhuangjianguo merged 1 commit into
mainfrom
claude/issue-14111-row-color-without-colors
Sep 3, 2026
Merged

feat(spec): warn when a grid view rowColor declares no colors map (view/row-color-without-colors)#15101
zhuangjianguo merged 1 commit into
mainfrom
claude/issue-14111-row-color-without-colors

Conversation

@claude

@claude claude Bot commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Fixes #14111

Clause ②: no — path limb fires (packages/spec/src/**), content limb no: no accept-set or surface change.

What was wrong

RowColorConfigSchema (packages/spec/src/ui/view.zod.ts) requires field and leaves colors optional, but the only renderer that reads the block needs both. objectui plugin-grid's useRowColor.ts opens with:

if (!config?.field || !config.colors) return undefined;

So rowColor: { field: 'status' } parses, publishes and colours no row, while every authoring surface reports success. Every key involved is a declared, live one — rowColor itself is live in the liveness ledger — so neither the unknown-key rejection nor the ledger can see it. That is the ADR-0078 silent half, in the family validateFunctionalCompleteness already gates (a summary with no summaryOperations, a select with no options, a calendar with no calendar block).

Triage ruling 5502967476 (2026-09-02): route A only. Route B (a renderer fallback to the bound select's options[].color) is out of scope, is not filed, and nothing here touches objectui.

What changed

1. packages/spec/src/kernel/functional-completeness.ts

  • New exported rule id VIEW_ROW_COLOR_WITHOUT_COLORS = 'view/row-color-without-colors', beside the family's others and in the pinned FUNCTIONAL_COMPLETENESS_RULES list.
  • checkViewCompleteness emits a warning at path rowColor.colors when a grid list view's rowColor binds a non-empty field and declares no usable colors map. The finding follows the family's shape exactly (rule / severity / path / message / fix), the message carries the runtime line that makes it true, and the fix is machine-pastable and echoes the authored field back.
  • The module doc's rule-to-skip-site list gains its entry, as ADR-0078 §6 and the file's ADR anchor require.
  • The existing rules are untouched; the new emit is appended after the tree block, so a view with several findings reports layout, then tree, then rowColor.

2. packages/spec/src/kernel/functional-completeness.test.ts — a new describe block beside the family's other view pins: the positive fixture (rowColor: { field: 'status' } ⇒ exactly one warning, asserting rule, severity, path, the first sentence of message and the cited runtime line), the negative fixture (field + colors ⇒ silent), the colors: {} fixture, the non-grid NON-rule pin, and the junk/absent-field controls. The rule-id enumeration pin and the "every finding carries a fix" pin are extended.

3. packages/spec/src/ui/view.zod.ts — the RowColorConfigSchema.field describe rewritten so it no longer reads as if a colour is derived without a map. colors stays optional; no other member moves.

4. Generated artifactscheck:generated --fix regenerated the three it proved stale (api-surface/, export-origins/, content/docs/references/ui/view.mdx); a second run reports "All 15 generated artifacts are up to date."

5. .changeset/row-color-without-colors.md@objectstack/spec patch. A new author-time warning plus describe text; no accept-set movement.

Two decisions worth reviewing, both measured

How colors: {} is treated — flagged, same as absent

The dispatch asked this to be measured against the renderer rather than assumed, and the two spellings reach the same dead end by different routes:

  • colors absent — the resolver's own guard (!config.colors) returns before it reads a record. Nothing is coloured.
  • colors: {} — an empty object is truthy, so it passes that guard; the lookup one line down (hasOwnProperty.call(config.colors, value)) then matches no value, if (!color) return undefined, and nothing is coloured either.

So mirroring the guard expression alone would have blessed {}; the rule mirrors the outcome the guard produces, which is identical. An empty map is also not an "I meant it" marker the way an action's locations: [] is — turning row colouring off has its own spellings (omit the block, or leave the userActions.rowColor toolbar toggle off) — so {} is the same dead shape spelled out. Same reasoning the family already applies to triggers: [] in checkWebhookCompleteness. Any other colors shape (a string, an array) is left alone: the schema refuses it at parse, and this module is not a second parser.

Why the rule fires on grid only

rowColor is declared on every list view, but objectui's ListView adapter forwards it in exactly one branch of its per-type props switch — case 'grid'. kanban / gallery / calendar / timeline / gantt / map / tree / chart each build their own props and never carry the key; page mounts a published page through a different renderer. On a kanban board a rowColor block is inert too, but for a different reason, and this rule's prescription ("declare a colors map") would not fix it — warning there would be a false prescription, the exact failure the module doc, ADR-0078 §6 and this file's ADR anchor exist to prevent. That non-grid inertness is recorded in the module, not enforced, the same disposition gallery already has in VIEW_BINDING_BLOCKS. It costs no reach the predicate had: checkViewCompleteness already returns early on a view with no string type, so a view relying on the schema's type default was never judged by any rule in this family.

Path spelling: the finding sits at rowColor.colors, not bare rowColorCompletenessFinding.path is documented as "path of the omitted config", and when a block is present and a sub-key is missing the family already spells it that way (tree.parentField, map.locationField). Through the lint walk it reads views[0].list.rowColor.colors, the same granularity the sibling validate-list-view-field-refs uses for rowColor.field.

Verification record — all at head 67c64885

Run in a dedicated worktree off origin/main 6665c5c9. Every exit code captured by redirecting first, never through a pipe; each gate quoted by its own verdict line.

Check Result
pnpm --filter @objectstack/spec build lock VERDICT command-exit 0; 34/34 declared declaration file(s) present
pnpm --filter @objectstack/spec exec vitest run --maxWorkers=2 Test Files 466 passed (466) · Tests 12434 passed (12434)
pnpm --filter @objectstack/lint exec vitest run --maxWorkers=2 (downstream consumer of the predicate) Test Files 93 passed (93) · Tests 2873 passed + 5 skipped, 2878 total
pnpm --filter @objectstack/spec typecheck exit 0; check:test-typecheck: OK — @objectstack/spec's test layer compiles under packages/spec/tsconfig.test.json
test-layer coverage is real, not assumed tsc -p tsconfig.test.json --listFiles lists both edited files (functional-completeness.ts, functional-completeness.test.ts) — measured, so "typecheck clean" actually covers them
pnpm --filter @objectstack/spec check:generated ✓ All 15 generated artifacts are up to date. (second run, after --fix)
check:authorable-surface, check:docs, check:api-surface, check:export-origins, check:liveness, check:exported-any, check:dual-source-exports, check:entry-nameability, check:browser-reachable-entries, check:skill-examples, check:empty-state, check:variant-docs, check:yaml-examples, check:llms-txt, check:skill-refs, check:strictness-ledger all exit 0
node scripts/check-system-context-census.mjs OK — 106 elevation read sites in 20 packages across 45 files, all anchored; 140 anchors resolve, 27 declared non-read. No anchor rotted, so no --fix
pnpm lint (eslint . --no-inline-config, repo-wide, not narrowed) exit 0, no output
pnpm check:nul-bytes plus a manual control-byte sweep over the edited files exit 0 / no match
node scripts/pm/dispatch-gates.mjs --commands union at this head 77 commands derived; 73 exit 0

The 4 that are NOT MEASURED, listed as such rather than as green, each by its own verdict text — all four refuse because this worktree has only the spec / lint / client closures built, none because of anything in this diff:

  • node scripts/check-dev-prereqs.mjsThe workspace is not built — 1 unmet precondition, not a list of problems.
  • node scripts/check-test-completeness.mjs — exit 3, PREREQUISITE NOT MET — this gate grades a saved turbo run test log, and no log was named (its own text: record as NOT MEASURED).
  • pnpm check:type-check-debt — exit 3, PREREQUISITE NOT MET … ⛔ This is NOT a pass and NOT a finding: nothing was measured.
  • pnpm check:dual-build-cjs-loads — exit 3, PREREQUISITE NOT MET — this gate reads built output, and some package has no dist/. … ⛔ This is NOT a pass: nothing was measured.

Reverse verification — one leg, from the committed state

The fix was committed first, so the restore leg has a real reference. The rule was then unhooked at its call site (if (ROW_COLOR_VIEW_TYPES.has(type) becomes if (false && ROW_COLOR_VIEW_TYPES.has(type)), under a trap … EXIT INT TERM with absolute paths.

Rebuild: none needed, and that is a measured claim, not an omission — the test imports ./functional-completeness by relative source path, so no dist sits between the mutation and the assertion. The mutation script fails loudly on a zero-hit anchor rather than proceeding.

Mutation confirmed on disk, anchored on the exact text being changed, both directions:

BEFORE: anchor=1 injected=0
AFTER:  anchor=0 injected=1   (blob 7a06160d… vs HEAD blob a8201fee…)

Observed direction — red, as predicted, 3 failures out of 47:

× flags `rowColor: { field }` with no `colors` as a WARNING
    AssertionError: expected [] to have a length of 1 but got +0
× flags `colors: {}` the same — an empty map passes the guard and then matches nothing
    AssertionError: expected [] to have a length of 1 but got +0
× every emitted finding carries a fix — the prescription IS the payload
    AssertionError: expected [ … ] to have a length of 9 but got 8
Test Files 1 failed (1) · Tests 3 failed | 44 passed (47)

The negative fixture stayed green throughout, as it must: with the rule unhooked, silence is silence — it is a control on the positive pin, not a second discriminator.

Restore proven by state, not by exit code: git hash-object on the restored file returns a8201feeb13ba978dc1545fe639d94b1c78c9497, byte-equal to the HEAD blob (a non-empty hash, checked; an empty one would have been read as failure), and both git diff HEAD and git status --porcelain came back empty. packages/spec was then rebuilt and the file re-run: Test Files 1 passed (1) · Tests 47 passed (47), so the final readings are on the restored tree. The whole gate union above was run after that rebuild, at this head, on a clean tree.

Out-of-scope finding, filed not fixed

Filed as #15100 (unassigned, unlabelled): examples/app-showcase/src/ui/views/task.view.ts:231 ships rowColor: { field: 'priority' } — the exact shape this diagnostic exists to name, so os validate will now report it as an advisory on the showcase. It is not fixed here: choosing a colour per priority value is a design decision about the demo app, not a mechanical fix pinned by existing evidence, and it lands in a different tree. os build prints warnings as advisories and exits 0, so no CI context reds because of it. packages/lint/src/showcase-shape.fixtures.ts carries the same line and is deliberately excluded — its header records the 2026-08-13 ruling that the frozen snapshot must not be re-synced.

Not done, deliberately

No needs:contract-review (Clause ② no). Left as a draft — not flipped to ready, not queued, not auto-merged.

🤖 Generated with Claude Code

Generated by Claude Code


Generated by Claude Code

`RowColorConfigSchema` requires `field` and leaves `colors` optional, so
`rowColor: { field: 'status' }` parses, publishes and colours nothing: the
only renderer that reads the block bails unless both are present. Every key
involved is declared and live, so neither unknown-key rejection nor the
liveness ledger can see it — the ADR-0078 silent half.

`checkViewCompleteness` now emits `view/row-color-without-colors` (warning,
path `rowColor.colors`) when a grid list view binds a non-empty
`rowColor.field` and declares no usable `colors` map. Both spellings of "no
map" are flagged: `colors` absent (the resolver's own guard returns early)
and `colors: {}` (truthy, so it passes that guard, then matches no value).
The message names the view, the bound field and the runtime line; the fix
prescribes the map.

Scope is measured from the renderer, not inferred from the schema: the rule
fires only on the view type whose adapter forwards `rowColor` at all. On the
other list view types the block is inert for a different reason, and
prescribing a colours map would not fix it — recorded in the module as a
non-rule rather than enforced.

`RowColorConfigSchema.field`'s describe is rewritten so it no longer reads as
if a colour is derived without a map. The accept set does not move: `colors`
stays optional and no member is added, removed or narrowed.

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

github-actions Bot commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/spec, touching 6 documentable anchor(s). ⚠️ 2 changed file(s) yielded no anchor (packages/spec/api-surface/kernel.json, packages/spec/export-origins/kernel.json), so the pages documenting them are NOT COVERED by this run — this is not a clean bill of health for those files.

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

  • content/docs/releases/v17.mdx (via checkViewCompleteness (symbol, a top-level function))

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
  • 2 changed file(s) yielded no anchor (packages/spec/api-surface/kernel.json, packages/spec/export-origins/kernel.json) — pages documenting those are invisible to this run
  • 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 — 128 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 7ad3d4e8df5cc5aa0748764e15c876c09c40b6e7packageMentionDocs.

Which tree this was computed on

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

node scripts/docs-audit/affected-docs.mjs --json 7ad3d4e8df5cc5aa0748764e15c876c09c40b6e7

⚠️ 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 7ad3d4e8df5cc5aa0748764e15c876c09c40b6e7 → pass the list as
args.docs, on the commit named under Which tree this was computed on.

Copy link
Copy Markdown
Collaborator

Landing provenance (PM seat domain:spec, session_0174WZTU6XcFcS7g2kykC53i, 2026-09-03T22:50Z) — flipping to ready and enabling auto-merge (squash).

  • Contract review: not applicable (Clause ② no on both carriers; check-clause2-carriers.mjs --pair 15101 exit 0 at 22:49Z); review verdict ACCEPT on [finding] rowColor.colors is optional in the schema but required by the only renderer — rowColor: { field } alone is a parse-clean no-op #14111 (comment 5532955594, 22:28Z) — both objectui claims (useRowColor.ts:72/:83, ListView.tsx:2358-2388) verified on objectui 5f789538.
  • CI on head 67c64885: 34 check runs, 32 success + 2 skipped, 0 red, 0 in progress (22:49Z) — including Lint & Repo Gates, TypeScript Type Check, the six Test Core shards and the three Dogfood Regression Gate shards at success (the showcase's new advisory warning is not a build red, as the report measured).
  • Governed-surface test on the 7 changed paths: NOT governed (0 of 7), no content/docs/releases/** — queue landing applies. Changeset present (@objectstack/spec patch).
  • mergeable_state: clean, mergeable: true; trial merge clean against origin/main 29db3cd2 (22:49Z). Body first line Fixes #14111; full-body scan finds no other closing keyword.
  • Next reading: the added_to_merge_queue timeline event; on MERGED [finding] rowColor.colors is optional in the schema but required by the only renderer — rowColor: { field } alone is a parse-clean no-op #14111 closes via Fixes, pm:dispatched is stripped in the same action, functional-completeness.ts and the RowColorConfigSchema.field describe are probed on origin/main, and the view.zod.ts / functional-completeness.ts reservation is released.

Generated by Claude Code

@zhuangjianguo
zhuangjianguo marked this pull request as ready for review September 3, 2026 22:50
@zhuangjianguo
zhuangjianguo added this pull request to the merge queue Sep 3, 2026
Merged via the queue into main with commit e9b377e Sep 3, 2026
36 checks passed
@zhuangjianguo
zhuangjianguo deleted the claude/issue-14111-row-color-without-colors branch September 3, 2026 23:17
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 protocol:ui size/m tests tooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[finding] rowColor.colors is optional in the schema but required by the only renderer — rowColor: { field } alone is a parse-clean no-op

2 participants