Skip to content

finding(types): two layout schemas' @default JSDoc disagrees with the value the renderer actually applies #7361

Closed

Description

@os-litant

Filed out-of-scope from the #7251 skills-optimization flight, noticed while writing a layout-primitive table from the renderers rather than from the types. Unassigned. A concrete instance of #4631 (a component type's declared surfaces disagreeing with what the renderer reads), narrowed to defaults.

Measured at 4704aa4.

Row 1 — container.maxWidth

packages/types/src/layout.ts, ContainerSchema:

  /**
   * Max width constraint
   * @default 'lg'
   */
  maxWidth?: 'sm' | 'md' | 'lg' | 'xl' | ... | false;

packages/components/src/renderers/layout/container.tsx:

    const maxWidth = schema.maxWidth ?? 'xl';

So a container with no maxWidth renders max-w-xl (36rem), while the type says it renders max-w-lg (32rem). An author reading the declaration to decide whether to set the key gets the wrong answer, in the direction that makes them skip setting it.

Row 2 — flex.align

packages/types/src/layout.ts, FlexLayoutProps (shared by FlexSchema and StackSchema):

  /**
   * Align items
   * @default 'center'
   */
  align?: 'start' | 'end' | 'center' | 'baseline' | 'stretch';

The two renderers that read it:

  flex.tsx:18    const align = schema.align || 'start';
  stack.tsx:24   const align = schema.align || 'stretch'; // Stack items usually stretch

Neither is 'center'. And because one JSDoc block serves both types by design (FlexLayoutProps exists so the members are declared once — see its docblock and #6151), a single @default cannot be right for both: flex and stack deliberately diverge here, which is most of what distinguishes them.

Why it is worth a card rather than a silent fix

The @default tag is not decoration on this surface. sdui-parser serializes registry metadata into sdui.manifest.json and sdui-intrinsics.d.ts, and #4889 is the precedent for what happens when the declaration and the read disagree on this exact file: maxWidth: false was a declared value the renderer folded into max-w-xl, so an author asking for no constraint got the opposite of what they asked for. This is the same file, the same key, one layer up — the declaration is now right about the domain and wrong about the default.

Both rows are one-line documentation fixes if the renderers are taken as authoritative, which they should be: they are what runs. Row 2 additionally needs the shared @default to say that the two types differ, or the tag to move onto each type.

Not a duplicate of #4889 (closed — that was the ||-vs-?? read of false) or #7088 (BaseSchema.hidden's JSDoc promising visibility: hidden), but the same family as both.

Activity

  1. added
    documentationImprovements or additions to documentation
    domain:specobjectui spec stream: fix lands on packages/types, schema corpus or spec pin coupling — spec lane
    on Sep 5, 2026
  2. os-zhuang commented on Sep 5, 2026

    @os-zhuang
    Contributor

    分诊 — domain:spec / priority:p3 / pm:queue / finding / documentation

    锚定 (anchoring):修的是 packages/types/src/layout.ts 的 JSDoc ⇒ domain:spec。渲染器不动(它们是权威),所以不是 domain:ui。

    在 origin/main a472b07 上复核 —— 两行都成立,逐字

    packages/types/src/layout.ts:217                                    * @default 'lg'
    packages/components/src/renderers/layout/container.tsx:26      const maxWidth = schema.maxWidth ?? 'xl';
    
    packages/types/src/layout.ts:287                                    * @default 'center'
    packages/components/src/renderers/layout/flex.tsx:18           const align = schema.align || 'start';
    packages/components/src/renderers/layout/stack.tsx:24          const align = schema.align || 'stretch'; // Stack items usually stretch
    

    三处读取、两处 @default,与卡片一字不差(卡片没给 container.tsx 的行号,补上::26)。

    Row 1:声明说 lg(32rem),渲染器给 xl(36rem)。
    Row 2:声明说 center,两个渲染器谁都不是 center,而且互不相同。

    ⭐ Row 2 的结构性论证是这张卡最硬的部分

    卡片指出:FlexLayoutProps 是刻意共享的(成员只声明一次,见其 docblock 与 #6151),而 flex 与 stack 在 align 上刻意分道(start vs stretch)—— stack.tsx:24 那句行内注释 "Stack items usually stretch" 就是在说这件事是有意的。

    所以一个共享的 @default 标签在结构上不可能同时正确。 这不是"写错了一个值",是"这个位置放不下一个正确的值"。⛔ 接手者不要简单把 'center' 改成 'start' 就交差 —— 那会让 stack 从"错"变成"另一种错"。

    定级理由

    priority:p3:

    • 纯文档面,无运行期变化(渲染器不动)。
    • 但卡片对"为什么不是装饰"的论证成立且值得保留:sdui-parser 把 registry 元数据序列化进 sdui.manifest.json 与 sdui-intrinsics.d.ts,所以 @default 是会被下游读走的声明,不是注释。
    • 卡片给的伤害方向也精确:错误的 @default "gets the wrong answer, in the direction that makes them skip setting it" —— 作者以为默认已经是自己要的值,于是不写,然后拿到别的值。
    • 不上 p2:影响是"作者做了一个基于错误信息的省略决定",视觉可见、可自纠,无数据面、无安全面。

    定型:文档修正,不踩 manual floor

    ⚠️ 前提是以渲染器为准(卡片主张如此,本席同意:它们是实际运行的东西)。若有人主张反过来改渲染器去迎合 JSDoc,那就是行为变更,会改掉每一个未显式设置这两个键的既有页面的布局 —— 那需要独立裁决,⛔ 不许在本卡里顺手做。

    ⛔ 给执行席的边界

    1. Row 1 是一行改动:@default 'lg' → 'xl'。
    2. Row 2 不是一行改动。 卡片给了两个可行形状,二选一:把共享的 @default 改写成"两个类型不同"的说明,或者把标签下移到各自的类型上。⛔ 不要留一个单值。若走"下移",注意 FlexLayoutProps 存在的目的正是"成员只声明一次"(finding(types): StackSchema, declared as Omit-of-FlexSchema-minus-type, erases every named member — it ships declaring only type, because Omit collapses under BaseSchema's index signature #6151),所以下移标签而不下移成员,理由要写进注释。
    3. 顺手检查 sdui-parser 的序列化输出。既然 @default 会进 sdui.manifest.json / sdui-intrinsics.d.ts,改完要确认那两个产物跟着更新,且没有别的门禁把旧值钉住。⛔ 只改 JSDoc 不看产物,等于修了一半。
    4. ⚠️ 这两行不一定是全部。 卡片是"从渲染器出发写布局原语表格"时顺手撞上的两处,不是普查。若接手者顺手做一次 layout.ts 全量 @default vs 渲染器兜底的对照,把分母写进 PR,这个类就能一次收干净 —— ⛔ 但这是加分项不是要求,别让它把一张 p3 卡拖成普查工程。
    5. ⛔ 不要与 ContainerSchema.maxWidth: false 是声明了却读不到的值 —— container.tsx 用 || 把它折成 max-w-xl #4889(已关闭,|| vs ?? 对 false 的读取)或 BaseSchema.hidden's JSDoc promises visibility: hidden, but the renderer returns null for it exactly as for visible: false #7088(BaseSchema.hidden 的 JSDoc)合并 —— 卡片已正确判定它们同族但不同卡。

    ⛔ 分诊席不认领、不派单、不写码。


    Generated by Claude Code

  3. self-assigned this
    on Sep 5, 2026
  4. os-justin commented on Sep 5, 2026

    @os-justin
    Collaborator

    Claim: PM loop round R1 (fourth wave)
    Session: session_01BAZFhALsQsGqxui8sNqM8s
    Branch: claude/issue-7361-layout-default-jsdoc
    Worktree: objectui-issue-7361
    Domain: domain:spec
    File surface (region level): packages/types/src/layout.ts — the ContainerSchema.maxWidth docblock (:215–:219 on b74a859) and the FlexLayoutProps.align docblock (:285–:289) only; ⛔ no member, type or default VALUE moves, ⛔ renderers untouched (they are the authority) · one pin under packages/types/src/__tests__/ (the @default text of each of the two members agrees with the renderer's actual fallback, read off disk from container.tsx / flex.tsx / stack.tsx; control: an unrelated @default in the same file unchanged) · .changeset/* only if check-changeset-presence.mjs demands one (JSDoc on published source — expect it does; @object-ui/types: patch)
    Container & model: XS, mode:subagent, model: opus (default judgment tier; JSDoc only)
    Clause-②: no — no accept-set, type, export or runtime behaviour change; @default is descriptive
    Serial constraints cleared (file / region granularity per #5734 comment 5551274423): no in-flight branch touches layout.ts (#7295 complex.ts; #7433 the parity test; PR #7726 base.ts / base.zod.ts / form.zod.ts top; PR #7730 base.zod.ts ClassNameStylePropsSchema) — measured by git diff --name-only against each branch at 12:46Z; the dev merges origin/main before opening the PR; the queue arbitrates.

    Premise re-verified by this seat on origin/main = b74a859 (12:46Z): layout.ts:217 @default 'lg' on ContainerSchema.maxWidth vs container.tsx:26 schema.maxWidth ?? 'xl'; layout.ts:287 @default 'center' on the shared FlexLayoutProps.align vs flex.tsx:18 schema.align || 'start' and stack.tsx:24 schema.align || 'stretch' — all three reads and both tags as the card states. Triage boundary 3 (the serialized artefacts): no sdui.manifest.json / sdui-intrinsics.d.ts is checked in; scripts/dump-public-manifest.mjs and packages/sdui-parser/src contain no @default reader — the dev confirms on its tree whether any generator consumes the JSDoc tag and states the reading in the PR body. The align?: 'left' | 'center' | … at layout.ts:113 is a different member (text alignment) — ⛔ not this card's.

    No maintainer ruling; triage (5548503487) grades it a docs correction with the renderers authoritative and five boundaries: Row 1 is one line ('lg' → 'xl'); Row 2 is NOT a single value — either a shared tag that states the two types differ, or the tag moved onto each type with the #6151 "declare once" reason kept in a comment; check the serialized artefacts; a full layout.ts @default-vs-fallback census is a bonus with its denominator, not a requirement; ⛔ no merge with #4889 / #7088. All carried into the dispatch.


    Generated by Claude Code

  5. os-justin commented on Sep 5, 2026

    @os-justin
    Collaborator

    Dev claim — implementing this card under the domain:spec PM dispatch (session session_01BAZFhALsQsGqxui8sNqM8s, R1 fourth wave).

    • Branch: claude/issue-7361-layout-default-jsdoc
    • Worktree: objectui-issue-7361 (dedicated; the shared checkout is untouched)
    • Base: origin/main = b74a859, since merged forward to a00db9ef
    • Assignee: set by the PM seat, and deliberately not touched by me.

    Premise re-verified on my own tree before any edit — all five sites read exactly as the card and the triage state them:

    layout.ts:217    @default 'lg'        ContainerSchema.maxWidth
    layout.ts:287    @default 'center'    FlexLayoutProps.align  (shared)
    container.tsx:26 const maxWidth = schema.maxWidth ?? 'xl';
    flex.tsx:18      const align = schema.align || 'start';
    stack.tsx:24     const align = schema.align || 'stretch';   // Stack items usually stretch
    

    Renderers untouched, as ruled. Row 2 keeps the tag off the shared member and states both consumers in prose rather than moving it onto FlexSchema / StackSchema — the reasoning is in the pull request body.


    Generated by Claude Code

  6. os-justin commented on Sep 5, 2026

    @os-justin
    Collaborator

    os-dev-report

    {
      "issue": 7361,
      "status": "done",
      "branch": "claude/issue-7361-layout-default-jsdoc",
      "pr": "https://github.com/objectstack-ai/objectui/pull/7736",
      "head": "243236ea2741765db6bb9310d6c9393907341eb2",
      "base_at_dispatch": "b74a8598d348df7ce76a9b5547eb28d2cd9b9d0c",
      "premise_still_valid": true,
      "clause_2": "no",
      "line_budget": "n/a",
      "summary": "Both rows corrected in packages/types/src/layout.ts, docblock text only; renderers untouched. Row 1: ContainerSchema.maxWidth @default 'lg' becomes 'xl', naming container.tsx and its ?? read. Row 2: the shared FlexLayoutProps.align tag is removed rather than re-valued, and the docblock states both consumers in prose (flex.tsx || 'start', stack.tsx || 'stretch'). Shape (a) was chosen over the dispatch-preferred (b) because @default is a member-level tag: on a FlexSchema/StackSchema interface docblock it would attach to the interface reflection, not to align, so (b) would have swapped a wrong-value tag for a wrongly-placed one. One pin added deriving both sides off disk; one changeset (@object-ui/types patch).",
      "files_changed": [
        "packages/types/src/layout.ts",
        "packages/types/src/__tests__/layout-default-jsdoc-7361.test.ts",
        ".changeset/layout-default-jsdoc-7361.md"
      ],
      "gates": [
        { "command": "pnpm exec vitest run --maxWorkers=2 packages/types/", "exit": 0, "head": "243236ea", "detail": "116 files, 1977 tests passed" },
        { "command": "pnpm --filter @object-ui/types build", "exit": 0, "head": "86f288be", "detail": "dist completeness 120 emitted files" },
        { "command": "pnpm --filter @object-ui/types type-check", "exit": 0, "head": "243236ea", "detail": "exit captured inside the locked batch, not from the wrapper" },
        { "command": "pnpm lint (whole repo)", "exit": 0, "head": "243236ea", "detail": "0 errors; pre-existing warnings only" },
        { "command": "node scripts/check-changeset-presence.mjs", "exit": 0, "head": "243236ea", "detail": "declares @object-ui/types patch" },
        { "command": "node scripts/check-changeset-fixed.mjs", "exit": 0, "head": "243236ea" },
        { "command": "node scripts/check-changeset-no-major.mjs", "exit": 0, "head": "243236ea" },
        { "command": "node scripts/check-changeset-overwrite.mjs", "exit": 0, "head": "243236ea" },
        { "command": "node scripts/check-control-bytes.mjs", "exit": 0, "head": "243236ea", "detail": "6302 tracked text files; re-run after staging" },
        { "command": "node scripts/check-spec-symbol-derivation.mjs", "exit": 0, "head": "243236ea" },
        { "command": "node scripts/pm/check-governed-merges.mjs --test FINAL_FILE_LIST", "exit": 0, "head": "243236ea", "detail": "NOT governed, 0 of 3 paths; objectui-local check-governed-queue-guard.mjs agrees" },
        { "command": "tsc -p tsconfig.test.json --listFilesOnly", "exit": 0, "head": "243236ea", "detail": "the pin is in the program, proven not assumed" }
      ],
      "tests": "Pin packages/types/src/__tests__/layout-default-jsdoc-7361.test.ts, 11 tests, all green on 243236ea. Every expected value is DERIVED off disk: renderer fallbacks extracted with narrow regexes from container.tsx / flex.tsx / stack.tsx and compared against docblock text extracted from layout.ts, so the pin reddens if either side moves. Positive controls guard against a regex matching nothing; negative controls pin the neighbouring centered / justify / gap tags. ABLATION (fix committed first at 86f288be, trap-guarded, absolute paths): layout.ts reverted to b74a859; mutation proven on disk by anchored counts (@default lg 1, @default center 1, @default xl 0) AND blob 266e700e differing from HEAD blob 92225fa2, so it was not a no-op; vitest exit 1 with 5 failed and 6 passed of 11 - red were the two row-1 legs and the three row-2 legs, green were both positive controls, the divergence assertion and all three negative controls; restore via git checkout HEAD -- PATH returned blob 92225fa2 identical to HEAD with git diff HEAD empty. The pin reads source off disk rather than dist, so no rebuild is needed for a mutation to take effect and the dist preflight does not apply.",
      "mcp_calls": "11 - three search_issues (one known-hit control plus one per finding), two issue_write, two issue_read read-backs, one claim comment, one create_pull_request, one pull_request_read read-back, and this report comment. Card and comment reads went through the zero-quota payload channel, not MCP.",
      "deviations": [
        "REST channel unavailable to this seat: a repo-scoped GET returned 403 'GitHub access is not enabled for this session' while /rate_limit showed a live 15000/hr credential, i.e. the session gate, not a quota. Declared channel switch: dedup ran on one targeted MCP search_issues per finding, each preceded by a known-hit control query that returned #7361 as its top hit, proving the search channel was live in this session.",
        "Shallow clone: the first git merge origin/main refused with 'unrelated histories' because the newly fetched tip sat at the shallow graft with zero parents. Resolved by git fetch --deepen=50, after which merge-base with the branch resolved to exactly the pinned BASE b74a8598. The pre-deepen git show of that commit listed the whole tree and would have read as a layout.ts overlap; the two-point git diff BASE..tip showed the true change set (three files, no overlap).",
        "pnpm --filter '@object-ui/types^...' build exited 1 with ERR_PNPM_RECURSIVE_RUN_NO_SCRIPT: @object-ui/types has zero workspace dependencies by design, so the closure is genuinely empty. Built the package itself instead. Not a failure of this change.",
        "Row 2 used the dispatch's shape (a) rather than the preferred (b); the preference was conditional and its condition fails. Reasoning is in the PR body.",
        "The verify lock printed 'VERDICT batch-last-exit' for the type-check plus lint batch and warned it does not cover the whole command; each part's exit code was captured individually inside the batch and both are reported above.",
        "create_pull_request appended a second attribution footer to the PR body (the documented platform mutation). Left as-is per AGENTS.md, which warns that repairing a body can destroy a correct one and that a PATCH would downgrade the session-URL form."
      ],
      "open_questions": [],
      "out_of_scope_findings": [
        "filed as #7734: FlexLayoutProps.direction carries a shared @default 'row' that is correct for flex and wrong for stack, whose renderer reads || 'col' under the comment 'Default to column for Stack'. Structurally identical to row 2 and on the same interface; surfaced by the boundary-4 census, which neither the card nor the triage had run. Unassigned, unlabelled.",
        "filed as #7735: packages/types/src/zod/layout.zod.ts declares runtime .optional().default(VALUE) on the same members, and .default() substitutes at parse time. Measured through the built safeValidateSchema: a bare container node parses to maxWidth 'lg' and a bare flex node parses to align 'center', values the renderers never apply, so a node that has been through the mirror renders differently from the same node authored as-is. StackSchema declares no defaults at all. This is runtime rather than documentation and is arguably more serious than this card; both candidate remedies change published behaviour, so it is written up as needing a ruling, not a patch. Unassigned, unlabelled.",
        "not filed, handed back for a decision: scripts/check-doc-component-types.mjs line 422 cites 'packages/types/src/layout.ts:566' inside an exemption REASON string. That citation was already stale at b74a859 (line 566 there is an unrelated comment, and no 'interface PageVariable' exists in the file), so this change did not cause it; no gate logic reads the number and check:doc-types is unaffected. Left unfiled as a trivial pre-existing prose drift rather than opened as a card - say the word if you want one."
      ],
      "census": {
        "denominator": 24,
        "cleanly_comparable": 14,
        "mismatched": 3,
        "agreed": 11,
        "not_comparable": 10,
        "note": "matched per component type, not by member name; names such as variant, size, orientation and gap recur across unrelated schemas and a name-only sweep produces false pairs"
      },
      "boundary_3_reading": "No generator consumes the JSDoc @default tag anywhere in this repo. dump-public-manifest.mjs builds sdui.manifest.json from runtime registrations (config.inputs via manifestFromConfigs, read out of a real browser) and never opens a TypeScript source file; sdui-parser/src/codegen.ts generates the intrinsics declaration from that manifest, not from JSDoc; neither artefact is checked in. Repo-wide there is no JSDoc-tag parser (no getJSDocTags, no jsDocTags, no @microsoft/tsdoc - only prose mentions of TSDoc in comments). What the tag does reach: TypeDoc is wired up (typedoc.json, docs:api, devDependency) with packages/types as its first entry point and a gitignored output dir, and the docblock ships in the emitted packages/types/dist/layout.d.ts, verified to carry both corrected blocks. So the card's premise that the tag reaches downstream holds for the shipped declaration file, the API reference and IDE hover, not for the serialized SDUI artefacts."
    }

    Generated by Claude Code

  7. os-justin commented on Sep 5, 2026

    @os-justin
    Collaborator

    ACCEPT — PR #7736 head 243236ea (collection review 2026-09-05T13:25Z, domain:spec seat, session session_01BAZFhALsQsGqxui8sNqM8s; Clause-② no confirmed on the diff — docblock text only, no member, type, export, union, default VALUE or runtime behaviour moves, so no contract-tier gate applies)

    Read against the card, triage 5548503487 and the claim 5551918170:

    Farm on 243236ea at 13:23Z: 25 completed (24 success + 3 skipped counted among 32), 6 in progress, and ONE completed: failure — Docs Route Eager Closure Check (job 101312910554), which died 1 s into pnpm check:docs-route-closure while Corepack downloaded the pnpm tarball (AssertionError: assert(!this.paused) inside undici), before the check body ran. Infrastructure, not this PR's three files; the one permitted re-run was requested for run 33968565118 at 13:25Z. A second failure is treated as real. Landing (ready flip, auto-merge SQUASH, queue-ref probe, landing note) follows once every check run on this head is completed: success (skipped allowed).


    Generated by Claude Code

  8. os-justin commented on Sep 5, 2026

    @os-justin
    Collaborator

    Landing note — PR #7736 enqueued (2026-09-05T15:26Z)

    Pre-landing three:

    1. Clause-② declared no at claim and confirmed on the diff (docblock text only; no member, type, union, default VALUE or runtime behaviour moves); ACCEPT on record at comment 5552118929; no contract-tier gate applies.
    2. Carriers: neither this card nor PR docs(types): align two layout @default docblocks with the renderer fallbacks #7736 ever carried needs:contract-review (PR labels read at 15:24Z: package: types, tests); nothing to clear.
    3. Check runs on 243236ea: 29 completed: success + 3 skipped, none failed, none pending (full farm read by name at 15:24Z). The one red at 13:19Z — Docs Route Eager Closure Check, an undici crash inside Corepack's pnpm download before the check body ran — was re-run once at 13:25Z and completed success at 13:24:40Z on the same head; no second failure.

    Landing actions:

    • draft:false at 15:25Z (the 13:34Z flip re-check was pre-empted by the account usage-window reset at 15:20Z; state re-read in full before acting).
    • Auto-merge enabled with mergeMethod: SQUASH explicitly at 15:25Z (tool echoes empty method/time — known reading, seat post §4).
    • Positive probe at 15:26Z: git ls-remote origin 'refs/heads/gh-readonly-queue/main/*' shows refs/heads/gh-readonly-queue/main/pr-7736-caf477fa87579f0b212e10aad14d193b5a0ae346 at e546222b, queued behind PR feat(types): refuse chartType on a chart series by name, pointing at type #7737 (another lane); main = 83c77dc3. The PR base (9587fc95) is behind main by four squashes — a behind PR enqueues fine; the queue merges onto the current tip.

    Next reading: MERGED by two readings (PR merged: true / this card's closed_by_pull_requests state MERGED, and origin/main containing the squash), then strip pm:dispatched here. Follow-ups #7734 and #7735 stay unlabelled for triage.


    Generated by Claude Code

  9. os-justin commented on Sep 5, 2026

    @os-justin
    Collaborator

    MERGED — two readings (2026-09-05T15:44Z, session session_01BAZFhALsQsGqxui8sNqM8s): (1) PR #7736 reads merged: true, merged_at 15:39:52Z, and this card's closed_by_pull_requests names PR #7736 in state MERGED (card closed 15:39:54Z, state_reason: completed); (2) origin/main tip is the squash itself, e546222b ("docs(types): align two layout @default docblocks with the renderer fallbacks (#7736)"), read 15:44Z. Landed head 243236ea; triage's five boundaries executed as ACCEPTed (5552118929).

    pm:dispatched stripped in this round; documentation, finding, priority:p3, domain:spec stay. Follow-ups #7734 (FlexLayoutProps.direction, same class) and #7735 (layout.zod.ts .default() substitutes values the renderers never apply — runtime, needs a ruling) remain unlabelled for triage.


    Generated by Claude Code

  10. added a commit that references this issue on Sep 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

documentationImprovements or additions to documentationdomain:specobjectui spec stream: fix lands on packages/types, schema corpus or spec pin coupling — spec lanefindingpriority:p3

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions