Skip to content

finding(plugin-detail): the record:details registration comment says the spec STRIPS undeclared section keys — measured, it REFUSES them, and the same comment block already says so two sentences earlier #7127

Description

@os-warren

Filed by the domain:ui seat (session session_012wwHa4aaFybxXrfmfHioDM) on behalf of #7064's implementer, who measured it while verifying that card and could not file it themselves — see the channel note at the bottom, which is a finding in its own right.

The defect

packages/plugin-detail/src/index.tsx, in the record:details registration comment:

Those are undeclared upstream, so the spec's section object STRIPS them on parse: publishing them here would advertise keys the contract throws away…

Measured against the installed @objectstack/spec (17.2.0): it does not strip them, it REFUSES them. RecordDetailsProps.safeParse over a section carrying hideEmpty returns success: false with unrecognized_keys: ['hideEmpty'] and the message "Unrecognized key(s) on this record:details section". Control in the same probe: a section carrying columns: 2 parses and the value survives — so the failure is about the key, not a broken probe.

⭐ The comment contradicts itself, two sentences apart

This is what makes it worth a card rather than a shrug. The same block, a few lines earlier, already states the correct behaviour:

Re-adding it here would republish a key the spec now rejects on parse

So one paragraph says rejects and the next says STRIPS. Those are materially different contracts — a stripped key is silently dropped and the page still renders; a refused key fails the parse and the page does not. A reader who reaches the second sentence first will believe undeclared keys are harmless.

The comment even records why the behaviour changed — "Until #4001 batch A an undeclared prop was dropped in silence…" — so the block contains its own evidence that "STRIPS" is the pre-#4001 wording that was never updated when the rest of the paragraph was.

Why it matters beyond tidiness

That comment is a decision record: it exists to stop a future editor from publishing title / showBorder / hideEmpty as authorable inputs. Its argument is "the contract throws them away". The real argument is stronger — the contract rejects the document — and an editor who tests the weaker claim, finds a hard parse failure, and concludes the comment is unreliable may discount the whole block, including the part that is right.

⭐ Same class the seat has been tracking all round: a comment that certifies behaviour the code no longer has — #7092 (PALETTE_EXCLUSIONS reason falsified by a renderer), #7070 (the #3129 note vouching for the gantt branch beside it), #7008 (RequiredFieldsDialog's "takes no id" workaround rationale, stale since #7009). Four instances in one round on one lane.

Scope

One comment. ⛔ Not a code change — the surrounding decision (do not publish those keys as inputs) is correct and stays verbatim; only the stated mechanism is wrong. Direct precedent for this shape: #6071 and #7092, both of which corrected a stated reason in place without touching the decision.

⚠️ Whoever takes it should re-measure against the spec version installed at that time rather than quoting 17.2.0 from this card.

Related — a real contract question, deliberately NOT folded in

The same measurement exposes a three-way divergence about one key, which is decision-shaped and cross-repo, so it is routed on #7064 rather than fixed here:

party says about hideEmpty on a record:details section
@objectstack/spec 17.2.0 ⛔ refuses it (unrecognized_keys)
@object-ui/types views.ts:230 ✅ declares it (hideEmpty?: boolean)
packages/types/src/zod/views.zod.ts ⛔ absent — 0 hits, against a control (headerColor) hitting 2 in the same file
RecordDetailsRenderer ✅ honours it

(The seat verified the two objectui-side rows independently, with the control shown.)

Activity

  1. self-assigned this
    on Sep 1, 2026
  2. os-warren commented on Sep 1, 2026

    @os-warren
    CollaboratorAuthor

    🔒 Claimed — domain:ui execution seat (objectui)

    • Session: session_012wwHa4aaFybxXrfmfHioDM · Branch: claude/issue-7127-record-details-strips-comment
    • Build tier: opus (⚠️ Fable 5 quota exhausted; the exemption lowers the build tier only)

    Not clause ② — one comment. The surrounding decision (⛔ do not publish title / showBorder / hideEmpty as authorable inputs) is correct and stays verbatim; only the stated mechanism is wrong.

    ⚠️ Selected as a one-line card and it must stay one. Direct precedent for the shape: #6071 and #7092, both of which corrected a stated reason in place without touching the decision. #7092 landed today as 44ea62d29 doing exactly this.

    The measurement is already done (by #7064's implementer, verifying PR #7123): RecordDetailsProps.safeParse on a section carrying hideEmpty returns success: false with unrecognized_keys, against a control (columns: 2) that parses and whose value survives. ⚠️ Re-measure against the spec version installed at claim time rather than quoting 17.2.0 from the card.

    ⭐ The self-contradiction is the strongest evidence and the easiest thing to verify first: the same comment block says "rejects" two sentences before it says "STRIPS", and it records why the behaviour changed ("Until #4001 batch A an undeclared prop was dropped in silence"). So "STRIPS" is pre-#4001 wording that survived an edit to the rest of the paragraph.

    ⛔ Not this card: the four-party divergence about hideEmpty (spec refuses / @object-ui/types declares / zod mirror omits / renderer honours) is a maintainer decision, routed on #7129. Do not resolve it by editing a declaration.


    Generated by Claude Code

  3. os-warren commented on Sep 1, 2026

    @os-warren
    CollaboratorAuthor

    os-dev-report

    {
      "issue": 7127,
      "status": "done",
      "branch": "claude/issue-7127-record-details-strips-comment",
      "pr": "https://github.com/objectstack-ai/objectui/pull/7136",
      "premise_still_valid": true,
      "spec_behaviour_remeasured_at_installed_version": "REFUSES, not strips. Probed from the worktree against node_modules/@objectstack/spec/dist/ui/index.mjs: RecordDetailsProps.safeParse on a section carrying title / showBorder / hideEmpty each returns success:false with code unrecognized_keys naming the key at path sections.0. Message verbatim: 'Unrecognized key(s) on this `record:details` section: `hideEmpty`. Until #4001 batch A an undeclared prop was dropped in silence: the props schema stripped it and `PageComponent.properties` is an open bag, so the key reached objectui's renderer, was not read there, and the author got a success receipt for configuration that did nothing.' Control in the same probe: a section carrying columns: 2 PARSES and the value survives (data.sections[0].columns === 2); bare { label, fields } also parses.",
      "installed_spec_version": "17.2.0 (re-measured, not quoted from the card: root node_modules and packages/plugin-detail/node_modules both resolve to 17.2.0; plugin-detail declares ^17.1.0)",
      "self_contradiction_confirmed": true,
      "old_text": "// undeclared upstream, so the spec's section object STRIPS them on parse:\n// publishing them here would advertise keys the contract throws away, the\n// same trap as declaring a top-level `readonly` on `record:highlights`\n// below. The renderer tolerating them is not a licence to teach them.",
      "new_text": "// undeclared upstream, and the spec's section object REFUSES them on parse\n// rather than stripping them: `RecordDetailsProps.safeParse` on a section\n// carrying any of the three returns `success: false` with\n// `unrecognized_keys` naming the key (measured on the installed pin, 17.2.0,\n// against a control - `columns: 2` - that parses and whose value survives).\n// So publishing them here would advertise keys that make the whole document\n// fail to validate, not keys the contract quietly throws away - the same\n// trap as declaring a top-level `readonly` on `record:highlights` below. The\n// renderer tolerating them is not a licence to teach them. (This said\n// \"STRIPS\" until objectui#7127: that was the pre-#4001-batch-A behaviour the\n// spec's own refusal message still recounts, and the `layout` paragraph\n// above already said `rejects`.)",
      "other_stale_claims_found": [
        "FIXED (same class, same file, one sweep): the neighbouring `record:highlights` registration comment at index.tsx:562 said 'the spec strips the unknown key on parse without error' for a top-level `readonly`. Measured: RecordHighlightsProps.safeParse({ fields: ['phone'], readonly: true }) is REFUSED with unrecognized_keys: ['readonly']; control { fields: ['phone'], layout: 'vertical' } parses and layout survives. Its decision (readonly documented inside the fields description, never a top-level input) is unchanged.",
        "LEFT (measured TRUE at 17.2.0, no edit): index.tsx:422 - `layout` is a tombstone the spec 'rejects on parse'. Probe { layout: 'auto' } is REFUSED, invalid_type, message 'removed in @objectstack/spec 17.0.0 (#6946, ADR-0087 D2)'.",
        "LEFT (measured TRUE): index.tsx:447 - hideFields is z.array(z.string()) and rejects {name}/{field} entries. Probe { hideFields: [{ name: 'phone' }] } REFUSED (expected string, received object); control ['phone'] parses.",
        "LEFT (measured TRUE): index.tsx:469 - inlineEdit / showHeader are plain booleans, 1 / 'true' / null rejected by value. All three REFUSED with invalid_type; control false parses.",
        "CARD FRAMING CORRECTED (see assumptions_falsified): the '#4001 batch A / dropped in silence' sentence is not in index.tsx at all."
      ],
      "decision_text_unchanged": true,
      "declarations_untouched": true,
      "tests_pinning_old_wording": "None. No test reads packages/plugin-detail/src/index.tsx as text; the only readFileSync(join(SRC_DIR, ...)) in plugin-detail's tests targets renderers/record-details.tsx. Controls for those zeros: 7 test files in that directory side-effect-import '../index' (so the edited module is loaded and executed by the green run), and 204 test files repo-wide use readFileSync. Empirical half: the two spec-parity files plus the console registry-inputs parity test are green after the edit.",
      "changeset_gate_verdict_verbatim": "Staged first, then run. BEFORE the changeset (exit 1): 'Compared the working tree with 220c18d05 (merge-base with origin/main): 1 file(s) changed, 1 of them published source of a package the release covers, 0 of them a manifest whose published contract moved, 0 under a package changesets ignores, 0 changeset(s) added.' then 'X  1 source file(s) of 1 released package(s) changed, and this change adds no changeset:'. AFTER adding .changeset/7127-record-details-refuses-not-strips.md with EMPTY frontmatter (exit 0): 'OK  1 source file(s) of 1 released package(s) changed, and this change declares 1 changeset(s): .changeset/7127-record-details-refuses-not-strips.md.' / 'Every one of them has an EMPTY frontmatter - declared as releasing nothing, which is the explicit exemption and a complete answer to this gate.' (The two leading glyphs are the gate's own cross and check marks.)",
      "gates": [
        { "name": "vitest (root-relative, 3 files: recordDetailsInputs.spec-parity, recordHighlightsInputs.spec-parity, apps/console registry-inputs-spec-parity)", "exit": 0, "verdict": "Test Files  3 passed (3) / Tests  131 passed (131)", "raw": "RUN v4.1.10 /home/user/objectui-7127 ... Duration 14.80s; shared-lock line: 'os-verify-lock: VERDICT command-exit 0 . held the lock 17s . waited 0s'" },
        { "name": "eslint packages/plugin-detail/src/index.tsx (plain form)", "exit": 0, "verdict": "31 problems (0 errors, 31 warnings)", "raw": "all 31 warnings pre-existing on lines 148-243 (react-refresh/only-export-components, @typescript-eslint/no-explicit-any); none on a line this PR touches" },
        { "name": "node scripts/check-changeset-presence.mjs", "exit": 0, "verdict": "1 source file(s) of 1 released package(s) changed, and this change declares 1 changeset(s) ... Every one of them has an EMPTY frontmatter", "raw": "non-empty population: '2 file(s) changed, 1 of them published source of a package the release covers ... 1 changeset(s) added'" },
        { "name": "node scripts/check-changeset-no-major.mjs", "exit": 0, "verdict": "No changeset declares a `major` bump.", "raw": "empty frontmatter declares no bump at all" },
        { "name": "node scripts/check-control-bytes.mjs", "exit": 0, "verdict": "check-control-bytes: OK (scanned 5908 tracked text file(s); skipped 85 binary).", "raw": "plus a direct scan of the edited file: grep -naP '[\\x00-\\x08\\x0b\\x0c\\x0e-\\x1f\\x7f]' returns no match" },
        { "name": "pnpm --filter @object-ui/plugin-detail run type-check", "exit": 2, "verdict": "NOT MEASURED - unbuilt dependency closure, declared narrowing", "raw": "21 diagnostics, all TS2307 'Cannot find module @object-ui/components / core / react / types / i18n / fields / permissions' plus the implicit-any cascade behind them, in a fresh worktree with no built dist. Pre-existing and unrelated: this diff changes 0 non-comment lines. Not read as red, not read as green. NOTE the script is spelled `type-check` (hyphen) in this package." }
      ],
      "controls_used": [
        "spec probe A: section { columns: 2 } PARSES and the value survives (2) - so the three refusals are about the key, not a broken probe",
        "spec probe A2: bare { label, fields } section PARSES",
        "spec probe C: RecordHighlightsProps { fields, layout: 'vertical' } PARSES and layout survives - the control for the highlights refusal",
        "DISCARDED AND RE-TAKEN: my first top-level control on RecordDetailsProps used columns: 2, which the top-level schema declares as an enum of STRINGS - it came back invalid_value, i.e. it was no control. Re-taken as columns: '2', which parses and survives.",
        "DISCARDED AND RE-TAKEN: the control for 'no test imports ../index' first used the pattern \"from '../index'\" and returned 0 - a control that itself returns zero is no control. Re-taken as \"'../index'\" (bare side-effect import): 7 files.",
        "grep control for the '#4001 batch A' zero: grep -i silence on index.tsx DOES hit (lines 74, 209, 265) - just not that sentence",
        "on-disk edit proof: removed-phrase counts went to 0 ('STRIPS them on parse', 'the contract throws away', 'silently discards') and injected-phrase counts to 1 each, plus git diff --stat non-empty. ('strips the unknown key on' still counts 1 - deliberately, the new text quotes the old wording it replaces.)",
        "commit-content proof: git show HEAD:packages/plugin-detail/src/index.tsx greps 1 / 1 for the two new phrases and 0 for the old one",
        "diff-shape proof: git diff -U0 yields 40 changed lines and 0 that do not match '^[+-]  // ', and 0 added lines containing @ts- / eslint-disable / block-comment delimiters"
      ],
      "assumptions_falsified": [
        "PM assumption 'the block also records why the behaviour changed (Until #4001 batch A an undeclared prop was dropped in silence)' - FALSE AS TO LOCATION. That sentence is not in index.tsx. It is the SPEC's own refusal message (printed verbatim by the probe) and is echoed in the sibling test-file headers (recordDetailsInputs.spec-parity.test.ts:257, recordHighlightsInputs.spec-parity.test.ts:80). The self-contradiction itself IS in the block and IS confirmed: line 422 'the spec now rejects on parse' vs line 428 'STRIPS them on parse', six lines apart in one contiguous // block.",
        "PM assumption 'this is the only stale claim in that comment block' - FALSE, but only one neighbour: the record:highlights comment at line 562 carries the same stale mechanism, measured and fixed here (see other_stale_claims_found). The three other parse claims in the file were probed and are true.",
        "Card sub-claim 'STRIPS is pre-#4001 wording that survived an edit' - NOT MEASURABLE FROM HISTORY HERE. This checkout is shallow (git rev-parse --is-shallow-repository = true); deepened to 525 commits and index.tsx still shows only 3 touching commits, so git log -S collapses all three phrases (STRIPS / rejects / strips-the-unknown-key) onto the graft-boundary commit. The pre-#4001 reading rests instead on the spec's own message ('the props schema stripped it'), which is independent evidence that stripping was the real pre-#4001 behaviour.",
        "PM assumptions that HELD: the spec still refuses at the installed version (so the card is NOT closed as measured-and-declined); no test pins the old wording; no code change is needed - nothing in the code depends on the comment."
      ],
      "summary": "Corrected the stated mechanism in the record:details registration comment: the spec REFUSES undeclared section keys (unrecognized_keys) rather than stripping them, re-measured at the installed @objectstack/spec 17.2.0 with a passing control. The sweep the dispatch asked for found the same stale mechanism one registration below, in the record:highlights comment about a top-level readonly, measured with its own control and corrected the same way. Comment-only: 40 changed lines, all of them // lines, 0 declarations / types / renderers / inputs touched, both decisions verbatim in substance (do not publish those keys as authorable inputs; readonly stays documented inside the fields description). The four-party hideEmpty divergence is untouched and stays routed on #7129. Empty-frontmatter changeset declares no release.",
      "tests": "At final commit 783f04300 (union re-run after the commit, cited from that run): vitest root-relative on packages/plugin-detail/src/__tests__/recordDetailsInputs.spec-parity.test.ts + recordHighlightsInputs.spec-parity.test.ts + apps/console/src/__tests__/registry-inputs-spec-parity.test.ts -> exit 0, 'Test Files  3 passed (3)', 'Tests  131 passed (131)', shared-lock 'VERDICT command-exit 0'. eslint on the edited file -> exit 0, '31 problems (0 errors, 31 warnings)', all pre-existing. check-changeset-presence -> exit 0 on a non-empty population. check-changeset-no-major -> exit 0. check-control-bytes -> exit 0. Heavy runs went through /home/user/objectstack/scripts/pm/os-verify-lock.sh with OS_VERIFY_LOCK_SLOT=objectui-7127. No ablation was run: the change is comment-only, so there is no behaviour to ablate; the substitute evidence is that the edited module is imported and executed by the green spec-parity run. type-check is declared NOT MEASURED (unbuilt closure) - see gates.",
      "mcp_calls": "6",
      "open_questions": [],
      "out_of_scope_findings": []
    }

    Generated by Claude Code

  4. os-warren commented on Sep 1, 2026

    @os-warren
    CollaboratorAuthor

    🟢 Landed — ba3b279a6

    PR #7136 merged via the merge queue; main advanced to ba3b279a6 at 04:45:11Z. Fixes closed this card automatically.

    Landing verified by content, with a control — not by sha

    A squash merge creates a new commit, so the PR's head.sha does not exist on main (I have made that exact mistake twice in this seat and both citations resolved to "unknown revision"). So the landing was probed against main by content, with a control to prove the instrument was alive:

    probe in packages/plugin-detail/src/index.tsx on main expect got
    SUBJECT (new) REFUSES them on parse ≥1 1 ✅
    SUBJECT (old) STRIPS them on parse 0 0 ✅
    CONTROL record:highlights >0 4 ✅

    The control matters: a subject-only reading of "0 hits for the old string" is indistinguishable from a grep that pointed at nothing at all. record:highlights hitting 4 in the same file proves the reading was live when it reported the old wording gone.

    What the card asked for vs. what shipped

    The card named one stale comment. The dev swept the neighbourhood rather than fixing the named line and stopping, and found a second stale claim one registration below: the record:highlights comment claimed the spec strips a top-level readonly. Measured against the spec: it refuses. Both are fixed in this PR.

    Three other parse claims in the same block were probed and measured TRUE — they were left alone. That is the right outcome to report explicitly: "I checked these and they were correct" is a result, and without it the sweep's coverage is unknowable from the diff.

    One sub-claim came back NOT MEASURABLE — and was not papered over

    The card's premise included that the comment's wording predated #4001. The dev tried to establish that from history and reported it NOT MEASURABLE in this environment: the checkout is shallow and the blame walk hits a graft boundary, so the answer is not "the wording is older" and not "it isn't" — it is unknown, and a shallow-clone blame that stops at the boundary will happily print a plausible commit that is merely the oldest one present.

    Rather than assert it anyway, the dev re-grounded the fix on evidence that is available in the tree: the spec's own refusal message. That makes the comment correct on today's code regardless of how the wording got there, which is what the comment has to be true of. Reporting a premise as unmeasured instead of quietly converting it to a green is exactly the discipline this seat is trying to hold — thank you.

    Ledger

    • Scope: docs/comment truthfulness only; no behaviour change, no public surface change. Clause ② not engaged.
    • Changeset: per the gate's verdict, not my prediction (I have mispredicted that gate twice, in opposite directions).
    • No follow-up filed — the sweep closed the neighbourhood.

    Generated by Claude Code

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

Metadata

Metadata

Assignees

Labels

domain:uiobjectui ui stream: fix lands on the published library or apps — objectui execution seatfindingpackage: types

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions