docs(rfc): replace RFC 29's blanket banner with a linted per-symbol status ledger - #3630
Conversation
…tatus ledger RFC 29 proposes a reset of a chat library that already shipped, and it is landing piecemeal. All 59 sub-pages carried the same page-level banner - "this page documents the proposed API shape - not yet implemented" - which is a claim no machine can check, so it rotted the moment #3277 shipped the prop-getter surface, made `mergeProps` public, and landed the IME guard. 57 of 59 page titles already resolved to live exports while the banner said otherwise. Status pass. Every page now carries a status block that names, symbol by symbol, what `veryfront/chat` exports today and what it does not, plus a third list for props and hook members that are not exports. Deltas that have actually landed are badged on their own heading - `shipped` or `partly shipped` - with the source file and line they landed in. `docs/rfcs/29-chat-api-shape/README.md` explains how to read a status block and rolls up the complete landed set. Seven deltas were found to have landed, not the two already known: `mergeProps` public, `useMessageBranches`, the `ChatInputContext*` naming, the `ChatInput` flat sub-part exports (all whole); `useChatInput` + its five prop getters, `useChatScroll`, and `ChatInput.Field`'s IME guard + native textarea surface (all partial, with the remainder spelled out). Self-policing. `scripts/lint/audit-rfc-status.ts` is a sibling of `audit-chat-composability.ts` and does the same class of mechanical doc-truth check: it bans the blanket phrasings, requires a status block on every page, and asserts that every symbol a page claims is *not* implemented is genuinely absent from the real public surface (barrel exports plus `Object.assign` and object-literal compound parts) - the rule the drift needed. It also verifies the reverse direction, that every `shipped` badge cites a `path:line` that resolves, and that the banner's landed-ness word agrees with the badges below it, so the page-level summary can never drift from its deltas again. Confirmed failing before the pass: 124 violations on the corpus as it stood, including the exact known-wrong claim - helpers.md:5: "mergeProps" is documented as not yet implemented, but it ships from `veryfront/chat` today. Wired into `lint:ci`, `verify`, and `verify:quick`. `lint:chat-composability` was in `verify` but not in `lint:ci`, so CI never ran it; both audits now run in the lint shard, and both their test files join `test:scripts`. Also: `collectCompoundParts` now recognises shorthand properties, so `Object.assign(ChatBase, { Message, ErrorBanner })` stops reading as a lie; and `ban-chat-antipatterns.ts` and `audit-chat-composability.ts` now cite the RFC rules they mechanise, which is what makes the corpus discoverable from the code.
|
Warning Review limit reached
Next review available in: 2 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughRFC 29 documentation now records verified export and implementation status. A new Deno lint audits those records against the chat public surface and source anchors. Verification tasks run the audit and its tests. Chat composability linting masks nested structures and recognizes uppercase shorthand parts. ChangesRFC 29 status verification
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 13
🧹 Nitpick comments (2)
deno.json (1)
544-544: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRun both audit tests in CI.
lint:ciruns in the CI lint job, but it does not runscripts/lint/audit-rfc-status.test.tsorscripts/lint/audit-cross-runtime-jsr.test.ts. Add both tests to a CI-executed command.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@deno.json` at line 544, The test:scripts command omits audit-rfc-status.test.ts and audit-cross-runtime-jsr.test.ts from CI execution. Add both test files to the CI-executed test command, preserving the existing test configuration and command flow.Source: Learnings
docs/rfcs/29-chat-api-shape/hooks/use-upload.md (1)
5-10: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider adding a "Not in
src/today" list for the proposed members.
useUploadis exported, so the ledger reports the page as fully covered. TheUseUploadResultshape at Lines 33-41 documentsgetDropTargetProps,getAttachInputProps, andretry, which the README at Line 20 says belong in the third list. Without that list, no rule pins those member claims, and the page can drift the same way the old banner did.The README explicitly names
getDropTargetPropsas an example for this list.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/rfcs/29-chat-api-shape/hooks/use-upload.md` around lines 5 - 10, Add a “Not in src/ today” status list to the RFC status block, documenting the proposed UseUploadResult members getDropTargetProps, getAttachInputProps, and retry as not currently present. Keep useUpload under the existing exported-symbol list so the status ledger distinguishes the hook export from its proposed result-shape members.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/rfcs/29-chat-api-shape.md`:
- Line 12: Change the “Already landed - shipped” heading in the RFC to level 2
(`##`) instead of level 3, preserving the heading text so the existing
SHIPPED_RE audit continues to match and markdownlint MD001 passes.
- Line 14: Update the status sentence in the RFC introduction to remove the
uncheckable document-wide claim that nothing else is implemented. Direct readers
to the per-piece ledgers as the authority, using wording that describes
remaining deltas as proposals while preserving the existing lint-authority
statement.
In `@docs/rfcs/29-chat-api-shape/components/agent-picker.md`:
- Around line 5-10: The status ledgers omit symbols documented as public
exports. In docs/rfcs/29-chat-api-shape/components/agent-picker.md lines 5-10,
add AgentPickerItem to the exported-symbol ledger or remove its flat-export
documentation; in docs/rfcs/29-chat-api-shape/hooks/use-sources.md lines 5-10,
add extractSourcesFromParts to the ledger or stop documenting it as a
veryfront/chat export, keeping each page’s ledger aligned with its API and
import sections.
In `@docs/rfcs/29-chat-api-shape/components/app-shell.md`:
- Around line 5-10: Correct the AppShell status block to use the actual package
boundary identified by the page, `veryfront/ui`, instead of `veryfront/chat`.
Update the related status/audit references consistently so the listed AppShell
symbols and their import guidance reflect the UI package.
In `@docs/rfcs/29-chat-api-shape/components/attachment-pill.md`:
- Around line 5-10: Align the status blocks with the documented public imports:
in docs/rfcs/29-chat-api-shape/components/attachment-pill.md:5-10, classify
AttachmentPillRetry and AttachmentPillRetryProps as not exported or mark their
import as proposed; in
docs/rfcs/29-chat-api-shape/components/attachments-panel.md:5-10, list
AttachmentsPanelItemName separately from AttachmentsPanel.Item.Name and
AttachmentsPanel.Item.Size; in
docs/rfcs/29-chat-api-shape/components/chat-agent-picker.md:5-10, add
agentsToPickerOptions and ChatAgentPickerProps to exported symbols; and in
docs/rfcs/29-chat-api-shape/hooks/use-chat-context.md:5-10, add
useChatContextOptional to exported symbols.
In `@docs/rfcs/29-chat-api-shape/components/chat-input.md`:
- Around line 180-184: Align the landing-site references for the ChatInput.Field
delta: update the badge anchor on the “changed” row or the README roll-up to use
the same primary file, and explicitly identify the other file as the secondary
implementation location if both remain relevant. Preserve the existing
descriptions of ChatInputFieldProps and handleInputBoxKeyDown.
In `@docs/rfcs/29-chat-api-shape/helpers.md`:
- Line 41: Update the sentence beginning “What has not landed with it” in
helpers.md to point readers to the reference table above rather than saying
“below,” while preserving the distinction between the proposed public helpers
and the current barrel.
In `@docs/rfcs/29-chat-api-shape/hooks/use-conversations-context.md`:
- Around line 5-10: Complete the status ledgers by adding
useConversationsContextOptional in
docs/rfcs/29-chat-api-shape/hooks/use-conversations-context.md:5-10,
useMessageContextOptional in
docs/rfcs/29-chat-api-shape/hooks/use-message-context.md:5-10, and
groupPartsInOrder in docs/rfcs/29-chat-api-shape/hooks/use-message-parts.md:5-10
to each page’s “Exported from veryfront/chat today” list.
In `@docs/rfcs/29-chat-api-shape/README.md`:
- Line 10: Update the RFC introduction around the sentence beginning “Those
five” to match the seven-row complete landed table and avoid treating exported
symbols as shipped deltas. Refer readers to the status block for shipped-delta
interpretation, while preserving the rule that exported symbols alone do not
constitute landed changes.
In `@scripts/lint/audit-chat-composability.test.ts`:
- Around line 60-75: Update the imports used by the test containing “collects
shorthand properties as sub-parts” to use describe/it from
`#veryfront/testing/bdd.ts` and assertions from `#veryfront/testing/assert.ts`,
removing the `#std/`... test-helper imports while preserving the test behavior.
In `@scripts/lint/audit-chat-composability.ts`:
- Around line 74-78: The shorthand discovery loop using shorthandRe must collect
only properties at the direct level of the Object.assign compound object,
excluding identifiers inside nested object literals such as Config: { Message }.
Track brace/object depth while scanning or replace the regex approach with an
AST-based property scan, and add a regression test covering nested shorthand
properties; preserve the existing direct shorthand detection used by
findCompositionLies and loadSurface.
In `@scripts/lint/audit-rfc-status.test.ts`:
- Around line 1-2: Update the imports in the audit RFC status test to use
describe and it from `#veryfront/testing/bdd.ts` and assertEquals from
`#veryfront/testing/assert.ts`, replacing the direct `#std/testing/bdd` and
`#std/assert` entry points.
In `@scripts/lint/audit-rfc-status.ts`:
- Around line 213-240: Update scripts/lint/audit-rfc-status.ts:213-240 in
parseLedger to detect more than one status banner, preserve the duplicate-block
violation through its return value or error state, and have auditPage report it
instead of silently using the last banner. Update
scripts/lint/audit-rfc-status.test.ts:70-259 with a case containing both
LANDED_BANNER and UNLANDED_BANNER that asserts the duplicate-block violation.
---
Nitpick comments:
In `@deno.json`:
- Line 544: The test:scripts command omits audit-rfc-status.test.ts and
audit-cross-runtime-jsr.test.ts from CI execution. Add both test files to the
CI-executed test command, preserving the existing test configuration and command
flow.
In `@docs/rfcs/29-chat-api-shape/hooks/use-upload.md`:
- Around line 5-10: Add a “Not in src/ today” status list to the RFC status
block, documenting the proposed UseUploadResult members getDropTargetProps,
getAttachInputProps, and retry as not currently present. Keep useUpload under
the existing exported-symbol list so the status ledger distinguishes the hook
export from its proposed result-shape members.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 2dc21666-0332-47be-9d8a-182e9669aad6
📒 Files selected for processing (69)
deno.jsondocs/rfcs/29-chat-api-shape.mddocs/rfcs/29-chat-api-shape/README.mddocs/rfcs/29-chat-api-shape/components/agent-card.mddocs/rfcs/29-chat-api-shape/components/agent-picker.mddocs/rfcs/29-chat-api-shape/components/app-shell.mddocs/rfcs/29-chat-api-shape/components/attachment-pill.mddocs/rfcs/29-chat-api-shape/components/attachments-panel.mddocs/rfcs/29-chat-api-shape/components/branch-picker.mddocs/rfcs/29-chat-api-shape/components/chat-actions.mddocs/rfcs/29-chat-api-shape/components/chat-agent-picker.mddocs/rfcs/29-chat-api-shape/components/chat-empty-state.mddocs/rfcs/29-chat-api-shape/components/chat-error-boundary.mddocs/rfcs/29-chat-api-shape/components/chat-input.mddocs/rfcs/29-chat-api-shape/components/chat-message-list.mddocs/rfcs/29-chat-api-shape/components/chat-root.mddocs/rfcs/29-chat-api-shape/components/chat-sidebar.mddocs/rfcs/29-chat-api-shape/components/chat-theme-scope.mddocs/rfcs/29-chat-api-shape/components/chat.mddocs/rfcs/29-chat-api-shape/components/inline-citation.mddocs/rfcs/29-chat-api-shape/components/markdown.mddocs/rfcs/29-chat-api-shape/components/message-action-bar.mddocs/rfcs/29-chat-api-shape/components/message.mddocs/rfcs/29-chat-api-shape/components/model-selector.mddocs/rfcs/29-chat-api-shape/components/reasoning.mddocs/rfcs/29-chat-api-shape/components/sources.mddocs/rfcs/29-chat-api-shape/components/step-indicator.mddocs/rfcs/29-chat-api-shape/components/tool-call.mddocs/rfcs/29-chat-api-shape/helpers.mddocs/rfcs/29-chat-api-shape/hooks/consumed-from-ui.mddocs/rfcs/29-chat-api-shape/hooks/use-agent-card.mddocs/rfcs/29-chat-api-shape/hooks/use-agent-metadata.mddocs/rfcs/29-chat-api-shape/hooks/use-agent-picker.mddocs/rfcs/29-chat-api-shape/hooks/use-agent.mddocs/rfcs/29-chat-api-shape/hooks/use-agents.mddocs/rfcs/29-chat-api-shape/hooks/use-attachment-pill.mddocs/rfcs/29-chat-api-shape/hooks/use-attachments-panel.mddocs/rfcs/29-chat-api-shape/hooks/use-attachments.mddocs/rfcs/29-chat-api-shape/hooks/use-chat-actions.mddocs/rfcs/29-chat-api-shape/hooks/use-chat-context.mddocs/rfcs/29-chat-api-shape/hooks/use-chat-error-handler.mddocs/rfcs/29-chat-api-shape/hooks/use-chat-input-context.mddocs/rfcs/29-chat-api-shape/hooks/use-chat-input.mddocs/rfcs/29-chat-api-shape/hooks/use-chat-scroll.mddocs/rfcs/29-chat-api-shape/hooks/use-chat-sidebar-item.mddocs/rfcs/29-chat-api-shape/hooks/use-chat.mddocs/rfcs/29-chat-api-shape/hooks/use-clipboard.mddocs/rfcs/29-chat-api-shape/hooks/use-completion.mddocs/rfcs/29-chat-api-shape/hooks/use-conversation-chat.mddocs/rfcs/29-chat-api-shape/hooks/use-conversation.mddocs/rfcs/29-chat-api-shape/hooks/use-conversations-context.mddocs/rfcs/29-chat-api-shape/hooks/use-conversations.mddocs/rfcs/29-chat-api-shape/hooks/use-message-branches.mddocs/rfcs/29-chat-api-shape/hooks/use-message-context.mddocs/rfcs/29-chat-api-shape/hooks/use-message-parts.mddocs/rfcs/29-chat-api-shape/hooks/use-model-selector.mddocs/rfcs/29-chat-api-shape/hooks/use-reasoning.mddocs/rfcs/29-chat-api-shape/hooks/use-sources.mddocs/rfcs/29-chat-api-shape/hooks/use-step-indicator.mddocs/rfcs/29-chat-api-shape/hooks/use-streaming.mddocs/rfcs/29-chat-api-shape/hooks/use-tool-call.mddocs/rfcs/29-chat-api-shape/hooks/use-upload.mddocs/rfcs/29-chat-api-shape/hooks/use-voice-input.mddocs/rfcs/29-chat-api-shape/providers.mdscripts/lint/audit-chat-composability.test.tsscripts/lint/audit-chat-composability.tsscripts/lint/audit-rfc-status.test.tsscripts/lint/audit-rfc-status.tsscripts/lint/ban-chat-antipatterns.ts
kwakayama
left a comment
kwakayama
left a comment
There was a problem hiding this comment.
Found three medium-severity documentation-lint correctness gaps.
-
[Medium] False “exported” compound parts pass validation.
resolvesOnSurfaceaccepts a dotted member whenever a same-named flat declaration exists, even when the member is not attached to the public compound (scripts/lint/audit-rfc-status.ts:188). Thus a privateChatInputPreviewdeclaration makes a ledger claim thatChatInput.Previewis exported pass despite noChatInput.Previewruntime property. Require an actual compound attachment/alias chain, and add the missing negative test. -
[Medium] The promised “exactly one status ledger” rule is not implemented. The parser silently overwrites each matched banner/list while scanning (
scripts/lint/audit-rfc-status.ts:221), andauditPagevalidates only that final ledger (scripts/lint/audit-rfc-status.ts:315). A page can retain an incorrect first ledger and append a correct second ledger; the lint passes. Count and reject duplicate banners and each required list, with a regression test. -
[Medium]
shippedanchors prove only that an arbitrary file and line exist, not that the cited change landed there. The validator performs only existence and line-range checks (scripts/lint/audit-rfc-status.ts:375), while the docs present the anchor as the source where the delta landed (docs/rfcs/29-chat-api-shape/README.md:24). An unrelated in-rangesrc/line passes, allowing stale or fabricated shipping evidence. Validate the cited symbol/change at the anchor (or use a stronger structured reference) and test an unrelated-but-in-range anchor.
| Rubric | Score |
|---|---|
| Correctness | 27/40 |
| Tests | 11/20 |
| Reliability/security | 15/15 |
| Maintainability | 11/15 |
| Scope/docs | 7/10 |
| Total | 71/100 |
Review-Gate:
Reviewer: Codex
Reviewed-SHA: 2f9e57d
Score: 71/100
Actionable-Findings: 3
Verdict: REQUEST_CHANGES
- RFC root: h1 -> h2 for the landed section (MD001), and drop the
un-checkable document-wide "nothing else has been implemented"
negative in favour of pointing at the per-piece ledgers.
- audit-rfc-status: ban that phrasing class by pattern, not substring,
so the next paraphrase cannot slip through; enforce rule 2 (exactly
one status block per page) instead of only documenting it.
- audit-chat-composability: collect only the compound's OWN properties.
`Object.assign(Base, { Config: { Message } })` was registering
`Message` as a direct `Base.Message` part, which let an invalid
composition token pass.
- Complete six status ledgers with exports the pages already document
(extractSourcesFromParts, agentsToPickerOptions, ChatAgentPickerProps,
useChatContextOptional, useConversationsContextOptional,
useMessageContextOptional, groupPartsInOrder).
- ChatInput.Field landed in two files; say so on the page and in the
roll-up instead of citing one anchor in each.
- README: 'those five' no longer reads as 'five deltas shipped'; note
that Import blocks show the proposed shape.
- app-shell: AppShell's home is veryfront/ui AND it is re-exported from
veryfront/chat - say both rather than implying it is not on the chat
surface.
- helpers: the reference table is above, not below.
# Conflicts: # deno.json
kwakayama
left a comment
kwakayama
left a comment
There was a problem hiding this comment.
-
[P1] Restore the build cache ignore marker.
src/build/production-build/build/build-orchestrator.ts:87now initializes the build immediately after logging, removing the priorensureCacheDirIgnored()call. Outside production, the default cache is<cwd>/.cache(src/utils/cache-dir.ts:85), and its own implementation documents that adopted projects otherwise commit generated bundles viagit add -A(src/utils/cache-dir.ts:152). The targeted regression test was also deleted fromtests/integration/server/build/build.test.ts. Restore the best-effort marker before build cache use and retain that test. -
[P1] Do not present a bare root
curlas deployment verification.docs/getting-started/deploy-project.md:97–101sayscurl -sSf <environment-url>confirms success, but protected environments deliberately return302without a session (src/proxy/proxy-access-control.ts:268), whichcurl -ftreats as success. It also incorrectly rejects valid API-only/dynamic-only deployments: deployment readiness intentionally has no route when no static page exists (cli/shared/deployment/deploy-project.ts:1293). Restore a route-specific status/redirect check and the protected-environment guidance. -
[P2] Enforce one complete ledger, not merely one banner.
scripts/lint/audit-rfc-status.ts:251–256overwrites each symbol list every time it encounters a matching label, while:349rejects only duplicate banners. A page can therefore put a falseNot exportedline first and a clean line later;auditPagereports zero violations. Count and reject duplicate Exported/Not-exported/Not-in-src lines within the status block, with a regression test. -
[P2] The RFC still contains the blanket claims this lint is meant to prohibit.
docs/rfcs/29-chat-api-shape.md:14says “Every other delta is still a proposal,” anddocs/rfcs/29-chat-api-shape/README.md:34says “Everything else … is still a proposal.” These are uncheckable corpus-wide negatives, yetBLANKET_CLAIMSonly matches “nothing else” (scripts/lint/audit-rfc-status.ts:64). Remove these claims or make the complete delta set machine-readable and validate it.
Rubric: correctness 28/40 · tests 10/20 · reliability/security 13/15 · maintainability 9/15 · scope/docs 4/10.
Review-Gate:
Reviewer: Codex
Reviewed-SHA: 399428a
Score: 64/100
Actionable-Findings: 4
Verdict: REQUEST_CHANGES
…e blanket claim
Two gaps in the RFC 29 status lint, both the same shape as bugs it had
already fixed one level up.
`parseLedger` overwrote each symbol list every time it met a matching
label while `auditPage` rejected only duplicate *banners*, so a page
could put a false `Not exported today:` line first and a clean one below
it and audit clean - the drift the ledger exists to catch, hidden inside
a single well-formed status block. Each list now carries its own line
numbers and every line past the first is rejected, exactly as extra
banners already were.
`BLANKET_CLAIMS` matched only the negative voice ("nothing else ...
implemented"), so the same un-checkable corpus-wide negative survived
inverted into the positive voice: "Every other delta is still a
proposal", "Everything else in this corpus is still a proposal". A
fourth pattern keys on a residual quantifier followed by "proposal",
which cannot reach the per-page banners - they say "proposed" of one
named page and carry no residual quantifier.
Deleting those two sentences alone would only hide the claim, because
the reference index's table still asserts it is "the complete set". So
the set is now validated (rule 7): every page badging a delta `shipped`
must have a row, and every row must point at a page that carries one.
|
Thanks for the detailed pass. Two of the four hold up and are now fixed; the two P1s do not apply to this branch, and I want to lay out the evidence rather than just assert it. The two P1s are stale-base artifactsThe gate reviewed The cleanest way to see it: this PR does not touch either file. Its entire diff against P1 — "Restore the build cache ignore marker"At the current head the marker is present, and the file is byte-identical to
The regression test is also present, not deleted: What happened is the reverse of the finding: the call and its test were added on P1 — "Do not present a bare root
|
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/rfcs/29-chat-api-shape/README.md (1)
32-43: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winUse a neutral heading for the mixed-status roll-up.
The table contains both
shippedandpartly shippedrows, but the heading saysWhat has landed - shipped. This makes partly shipped deltas appear fully shipped. Rename the heading toWhat has landed, and update the Line 10 fragment link if needed.Proposed fix
-> Those five symbols resolve on the public surface today. That is **not** the same as a landed delta - see [reading the status block](`#reading-the-status-block`). For the deltas that have actually landed, see [what has landed](`#what-has-landed---shipped-srcreactcomponentschatchathooksuse-chat-inputts85`). +> Those five symbols resolve on the public surface today. That is **not** the same as a landed delta - see [reading the status block](`#reading-the-status-block`). For the deltas that have actually landed, see [what has landed](`#what-has-landed`). -### What has landed - `shipped` (src/react/components/chat/chat/hooks/use-chat-input.ts:85) +### What has landed🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/rfcs/29-chat-api-shape/README.md` around lines 32 - 43, Rename the mixed-status roll-up heading from “What has landed - shipped” to “What has landed” so it accurately covers both shipped and partly shipped entries. Update the corresponding fragment link in the mergeProps row if the heading change alters its generated anchor.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/rfcs/29-chat-api-shape/README.md`:
- Around line 22-23: Update both references to the ChatInput export block in the
RFC to use the consistent src/chat/index.ts:250 anchor, and rename the “What has
landed - shipped” status heading so it accurately covers both shipped and partly
shipped entries.
In `@scripts/lint/audit-rfc-status.ts`:
- Around line 389-424: Update auditRollup and parseRollupRows to preserve and
report duplicate page rows instead of collapsing them through the listed Map,
while retaining each row’s line for violations. Broaden ROLLUP_ROW_RE parsing to
accept bare and ../ links, normalizing targets against RFC_DIR, or emit a
violation for shipped rows whose targets cannot be parsed. Add coverage in
audit-rfc-status.test.ts for duplicate rows and both non-./ link forms.
---
Outside diff comments:
In `@docs/rfcs/29-chat-api-shape/README.md`:
- Around line 32-43: Rename the mixed-status roll-up heading from “What has
landed - shipped” to “What has landed” so it accurately covers both shipped and
partly shipped entries. Update the corresponding fragment link in the mergeProps
row if the heading change alters its generated anchor.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: befb433f-fb7b-4e6c-972f-d729668592b4
📒 Files selected for processing (16)
deno.jsondocs/rfcs/29-chat-api-shape.mddocs/rfcs/29-chat-api-shape/README.mddocs/rfcs/29-chat-api-shape/components/app-shell.mddocs/rfcs/29-chat-api-shape/components/chat-agent-picker.mddocs/rfcs/29-chat-api-shape/components/chat-input.mddocs/rfcs/29-chat-api-shape/helpers.mddocs/rfcs/29-chat-api-shape/hooks/use-chat-context.mddocs/rfcs/29-chat-api-shape/hooks/use-conversations-context.mddocs/rfcs/29-chat-api-shape/hooks/use-message-context.mddocs/rfcs/29-chat-api-shape/hooks/use-message-parts.mddocs/rfcs/29-chat-api-shape/hooks/use-sources.mdscripts/lint/audit-chat-composability.test.tsscripts/lint/audit-chat-composability.tsscripts/lint/audit-rfc-status.test.tsscripts/lint/audit-rfc-status.ts
🚧 Files skipped from review as they are similar to previous changes (11)
- docs/rfcs/29-chat-api-shape/hooks/use-conversations-context.md
- docs/rfcs/29-chat-api-shape/hooks/use-message-parts.md
- scripts/lint/audit-chat-composability.ts
- docs/rfcs/29-chat-api-shape/hooks/use-message-context.md
- docs/rfcs/29-chat-api-shape/components/app-shell.md
- deno.json
- docs/rfcs/29-chat-api-shape/hooks/use-chat-context.md
- docs/rfcs/29-chat-api-shape/helpers.md
- docs/rfcs/29-chat-api-shape.md
- docs/rfcs/29-chat-api-shape/components/chat-agent-picker.md
- docs/rfcs/29-chat-api-shape/components/chat-input.md
kwakayama
left a comment
kwakayama
left a comment
There was a problem hiding this comment.
[P2] Validate the roll-up per delta, not merely per page
scripts/lint/audit-rfc-status.ts:392-399 reduces both sides of Rule 7 to a set of page paths. Consequently, any row for a page with one shipped heading is accepted, regardless of which delta the row names or how many shipped deltas the page has.
The current docs demonstrate the gap: [README.md:43-44] lists two distinct ChatInput deltas, but [components/chat-input.md:180] contains only the ChatInput.Field shipped badge—there is no shipped heading for “ChatInput flat sub-part exports.” The lint passes because both rows collapse to the same page path. This makes the claimed “complete set” at [README.md:34] untrustworthy.
Parse and compare row targets/anchors against individual badges (and reject duplicates), then add coverage for multiple badges on one page and a mismatched row. The existing roll-up tests at audit-rfc-status.test.ts:379-403 cover only one badge per page.
Scoring: correctness 31/40, tests 14/20, reliability/security 15/15, maintainability 12/15, scope/docs 8/10 = 80/100.
Review-Gate:
Reviewer: Codex
Reviewed-SHA: b66554a
Score: 80/100
Actionable-Findings: 1
Verdict: REQUEST_CHANGES
Rule 7 reduced both sides of the comparison to a set of page paths, so any row for a page carrying one `shipped` badge was accepted regardless of which delta the row named or how many deltas that page had actually landed. The corpus already demonstrated the hole: the index listed two distinct ChatInput deltas, `components/chat-input.md` badged only `ChatInput.Field`, and the lint passed because both rows collapsed to the same page path. The table's "complete set" claim was decorative - the same un-checkable corpus-wide assertion the blanket banners made, which is what this rule exists to stop. Rows are now paired with badges by the delta's own heading anchor. A badge with no row, a row whose anchor names no badge, a row that links a page without naming a delta, and two rows claiming the same delta are all violations. `parseRollupRows` also stops requiring a `./` prefix: a row written `[…](helpers.md)` or `[…](../29-chat-api-shape.md)` links the same document, and dropping it from the parse excused the row rather than checking it. Fixing the rule turned up the documentation gap it was blind to: "ChatInput flat sub-part exports" had landed (`src/chat/index.ts:250`) and was described in prose on the page, but carried no `shipped` badge, so it was never in the machine-checkable set. The docs moved, not the rule - the delta is now badged. The index's `src/chat/index.ts:253` anchor was also off by a declaration (253 is `type AttachmentPillProps`); it now cites 250 like the prose above it.
…ibes README:22 asserted that the flat named exports had landed and linked only the page. Now that the delta carries its own badge, point at it, so the prose and the roll-up row cite the same heading.
[P2] Validate the roll-up per delta — fixed, and the rule now bitesThe finding was correct. Rule 7 was added in this PR to make the "complete set" claim machine-checkable, and as written it checked it at page granularity, so it did not check the claim it existed for. Reproduced firstThree cases written as tests before touching the rule. All three passed under the old code — the audit returned zero violations for every one: The fixRows and badges are paired by the delta's own heading anchor, not by page path. The docs moved, not the ruleAgainst the real corpus the strengthened rule produced 11 violations: six rows carried no anchor, and the two I badged the delta rather than weakening the rule. The corpus lints clean at 63 pages. The index's Mutation-testedA rule reporting zero violations is indistinguishable from a no-op until it is shown to bite:
Tree restored after each. Gates
Both outstanding CodeRabbit threads (README.md:23, audit-rfc-status.ts:424) are answered and resolved — the second raised the duplicate-row and link-form halves of this same defect. |
Re: Review-Gate
|
|
The P2 on rule 7 validating per page rather than per delta is addressed as of Two commits:
Worth noting the second commit exists because the rule was tightened first and the real corpus then failed — which is the rule working. The docs were fixed to match reality rather than the rule being loosened to match the docs. All checks green, no unresolved threads. Happy to revisit if the gate re-runs and disagrees. |
# Conflicts: # deno.json
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scripts/lint/audit-rfc-status.ts`:
- Around line 327-328: Update headingSlug and the document-order heading scan to
preserve Unicode characters and generate GitHub-compatible unique anchors by
appending -1, -2, etc. for repeated slugs instead of overwriting earlier entries
in the badge Map. Ensure completeness checks retain every heading’s badge, and
add tests covering duplicate headings and Unicode headings.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: ca844c85-a279-4bf4-9aae-4bf69abb2214
📒 Files selected for processing (5)
deno.jsondocs/rfcs/29-chat-api-shape/README.mddocs/rfcs/29-chat-api-shape/components/chat-input.mdscripts/lint/audit-rfc-status.test.tsscripts/lint/audit-rfc-status.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- deno.json
- docs/rfcs/29-chat-api-shape/components/chat-input.md
- scripts/lint/audit-rfc-status.test.ts
- docs/rfcs/29-chat-api-shape/README.md
Rule 7 keyed each page's `shipped` badges by heading slug, minting one slug per badge with an ASCII, collision-blind rule. Both halves were wrong, and both shrank the set the rule claims to check exactly. Colliding headings collapsed. The slug rule strips punctuation, so a single comma can be the whole difference between two headings - this corpus already carries four such pairs. `new Map(badges.map(...))` kept only the last, so the earlier delta left the checkable set entirely and its missing roll-up row was never reported, while the later heading's real `-1` anchor matched no badge and was reported as broken. Unicode was deleted. `\w` is ASCII-only in JavaScript, so `Café` anchored as `caf` and a CJK heading slugged to bare punctuation - an anchor no heading has. The row regex had the same ASCII class, so a correctly-linked row fell out of the parse entirely. Headings are now numbered in document order across every heading on the page, badged or not, because that is what GitHub numbers; repeats take its `-1`, `-2` suffix. The slug class is Unicode-aware, and code fences are tracked (CommonMark marker rules) so a `#` line in a sample cannot take a number GitHub never issued. Verified against `github-slugger`, the slugger GitHub's renderer uses: identical output on all 943 headings in this corpus and on Greek, Cyrillic, CJK, emoji, and ligature cases. Mutation-tested against the real corpus. Two colliding badges with one row: the pre-fix script reported "every status ledger matches"; it now names the `-1` delta as unrowed. The same page with the `-1` row GitHub really renders, and a Unicode heading with its real anchor, each linted clean where the pre-fix script raised a false violation. No page needed changing - 63 pages still lint clean.
Numbering repeats with a per-base counter is not what GitHub does, and the gap reintroduces the exact collapse the previous commit closed. A suffixed slug can collide with a heading whose own text ends `-1`. Given headings `foo`, `foo-1`, `foo`, GitHub issues `foo`, `foo-1`, `foo-2`; a plain counter hands the third `foo-1` a second time, so two deltas share one identity and one of them leaves the checkable set again - one layer further in than where it was just fixed. `parseShippedBadges` now runs `github-slugger`'s own loop: re-check the candidate against every slug already issued, and keep spending the base's counter until one is free. Fuzzed against `github-slugger` rather than reasoned about. The plain counter diverges on 139 of 405 collision cases; this loop on none, and the real `parseShippedBadges` matches it across 3000 randomized documents mixing collisions, literal `-1` headings, and Unicode. The new test fails under the plain counter, expecting `-1` where GitHub issues `-2`.
2 participants
The drift
RFC 29 is a pending spec, not archaeology: the chat UI library shipped first, and RFC 29 proposes to reset it. The corpus is accurate and live — which is exactly why its status claim was dangerous.
All 59 sub-pages carried one identical, page-level banner:
That is a claim about a whole page, so no machine can check it, and an RFC that lands piecemeal silently falsifies it. It was already false: 57 of 59 page titles resolve to live exports, and #3277 shipped the prop-getter surface, made
mergePropspublic, and landed the IME guard without a single banner moving.1. The status pass
The blanket banner is gone. Every page now carries a status block that names, symbol by symbol, what
veryfront/chatexports today and what it does not:The third list carries props and hook members, which are not exports — the
submitMode/getDropTargetPropsclass of claim.Deltas that have actually landed are badged on their own heading, with the source they landed in:
### `ChatInput.Field` - `changed` - `partly shipped` (src/react/components/chat/chat/composition/chat-composer.types.ts:18)README.mdgains a Reading the status block section and the roll-up of the complete landed set. An exported symbol is explicitly not a landed delta —ChatInput.Submitships; the RFC's reshape of it does not — and the docs now say so instead of leaving it implied.What was actually found to have landed
I did not assume the two named in the brief. Checking the RFC's kept/changed/new/removed ledgers against the real exports turned up seven:
mergePropsmade publicshippedsrc/react/components/chat/chat/hooks/use-chat-input.ts:85useMessageBranchesshippedsrc/react/components/chat/chat/contexts/message-context.tsx:87useChatInputContext/…Optional/ChatInputContextProvidernamingshippedsrc/react/components/chat/chat/contexts/composer-context.tsx:83ChatInputflat sub-part exports (ChatInputField, … +Propstypes)shippedsrc/chat/index.ts:253useChatInput+ five prop getterspartly shippedsrc/react/components/chat/chat/hooks/use-chat-input.ts:155useChatScrollpartly shippedsrc/react/components/chat/chat/hooks/use-stick-to-bottom.ts:177ChatInput.FieldIME guard + full native textarea surfacepartly shippedsrc/react/primitives/input-box.tsx:37Each
partly shippedsection spells out what landed and what did not — e.g.useChatInputships the getters but still takes no options argument, still exposesinput/isLoadingrather thanvalue/status, and has nosubmit/stop/clear/attachorgetDropTargetProps.All 14
newbadges were re-verified and are still honest:Message.File,Message.Image,AttachmentsPanel.Item.Name/.Size,ChatSidebar.Item.Title,ChatSidebar.Item.Menu.Triggergenuinely do not exist.2. Making it self-policing
scripts/lint/audit-rfc-status.tsis a sibling ofaudit-chat-composability.tsand reuses itscollectCompoundParts— same class of mechanical doc-truth check, aimed at the RFC pages instead of the Storybook trees.The rule the drift needed, and five more that keep the fix from rotting:
shippedbadge must cite apath:linethat resolves (file exists, line in range) — the reverse direction, so a fake "shipped" is caught too.Resolution is real, not textual: barrel exports plus
Object.assigncompounds, plain object-literal compounds (ChatEmptyState),…Compoundaliases (ChatSidebar.Item.Menu), and flat declarations.Confirmed failing first
Against the corpus as it stood, 124 violations. And with the ledger written but
mergePropsstill on the not-implemented side — the known-wrong page — the check produced exactly the finding it exists for:17 unit tests in
audit-rfc-status.test.tscover each rule in both directions, including that regression by name.Wiring
lint:rfc-statusjoinslint:ci,verify, andverify:quick. While wiring it I found thatlint:chat-composabilitywas inverifybut never inlint:ci— CI has not been running the existing doc-truth audit at all. Both now run in the lint shard, and both test files jointest:scripts(neither was listed).3. Attribution
ban-chat-antipatterns.tsandaudit-chat-composability.tsenforce RFC 29's principles but never cited it. Both now name the rules they mechanise (ban-chat-antipatterns→ hard rules 1 and 7, theicon-slot ban, ref-as-prop;audit-chat-composability→ the node contract), so the corpus is discoverable from the code that enforces it.Incidental
collectCompoundPartsnow recognises shorthand properties, soObject.assign(ChatBase, { Message, ErrorBanner })stops reading as missing anatomy. Covered by a new test;lint:chat-composabilitystays green.Verification
deno task lint:rfc-status— 63 pages, greendeno task lint:chat-composability— 22 compounds, greendeno task lint:chat-ratchets— greendeno task docs:check-links— 1305 links OKdeno fmt --check(Deno 2.7.7, CI's pin) — 5007 files, cleanaudit-rfc-status.test.ts+audit-chat-composability.test.ts— 23 steps, greenNothing in
docs/rfcswas deleted.Summary by CodeRabbit
Documentation
Quality Improvements
Chores