docs: replace the stale chat README with a contributor map - #3629
Conversation
src/react/components/chat/README.md was a consumer tutorial that nothing
policed. Audited against deno.json's exports and the real components:
- All 15 import statements named modules absent from deno.json's `exports`
(8x `veryfront/react`, 7x `veryfront/agent/react`). Nothing in the file
was copy-pasteable from a consumer project. The real path is
`veryfront/chat`, which also exports `useChat`/`useAgent`.
- `AgentCard` was shown with a `theme` prop that AgentCardProps does not have.
- The "advanced" renderer switched on `case "tool-call"`, which no UI
ChatMessagePart ever is (tool activity arrives as `tool-${toolName}` with
`.state`), so it silently dropped every tool call.
- It hand-rolled `getTextContent`, which the barrel exports.
- It advertised 7 composition identifiers; the compound exposes Chat.Empty,
Chat.Skeleton, Chat.If and Chat.ErrorBanner too, and the barrel exports 354
names against the file's claimed "Total: 4 styled components".
- Its remaining vocabulary ("Phase 6 Complete", "Next: Phase 7", "Layer 2
primitives") maps to nothing live.
The two accurate blocks (ChatTheme, AgentTheme) are already in the generated
docs/api-reference/veryfront/chat.md with source links, and the guides that
cover this ground are contract-tested, so rewriting the tutorial in place would
have recreated the same failure: a fourth copy of the chat API surface with no
check behind it. 375b41f already hand-patched this file once during the
composition migration and it drifted again within weeks.
Kept the file, because src/ modules carry READMEs by convention and this
directory's internals are real orientation the public docs don't carry, but
scoped it to what only a contributor needs: the layout, the three-barrel
parity rule and the checks that enforce it, and a table pointing each kind of
API claim at the doc that owns it plus the check that keeps it true. It states
no signatures and shows no usage examples, so there is no API surface left in
it to go stale. Every path in it is a relative link, so
scripts/lint/check-doc-links.ts (which walks src/) now fails CI if the
structure it describes moves.
📝 WalkthroughWalkthroughThe chat README now provides contributor guidance instead of user-facing usage documentation. It documents authoritative references, repository structure, barrel export contracts, composability requirements, and validation workflows. ChangesChat contributor documentation
Estimated code review effort: 1 (Trivial) | ~5 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2e397858a1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
kwakayama
left a comment
There was a problem hiding this comment.
P2 — Correct the claimed barrel-parity rule. src/react/components/chat/README.md:49 says all three module specifiers must expose the same function objects, and :58 requires every new export to flow through all of them. That contradicts the intentional public/private split: src/react/components/chat/index.ts:26 and :36 export ColorModeProvider and chatTokens, while src/chat/index.test.ts:281 and :283 explicitly assert those names must not be exported by veryfront/chat. Revise this section to distinguish the curated public barrel from the two internal aliases; otherwise it directs contributors to accidentally widen the public API.
P2 — Do not describe dark mode as token-only. src/react/components/chat/README.md:44 says “Dark mode is tokens, not dark: variants,” but this directory uses direct dark: variants in src/react/components/chat/chat-actions-settings.tsx:89, :96, :240, and :242. So the contributor map now gives false styling guidance. Soften or remove the exclusivity claim.
Rubric: Correctness 33/40; Tests 13/20; Reliability/Security 15/15; Maintainability 13/15; Scope/Docs 8/10.
Review-Gate:
Reviewer: Codex
Reviewed-SHA: 2e39785
Score: 82/100
Actionable-Findings: 2
Verdict: COMMENT
The parity section told contributors that every new export must be threaded through all three barrels. The barrels are deliberately unequal: src/chat/index.test.ts asserts chatTokens, getChatTokensCSS and ColorModeProvider stay *out* of veryfront/chat while the components barrel exports them, and veryfront/chat carries hooks that do not live in this directory at all. State what is actually enforced: presence across the three barrels for the 32 names in CompoundChatRuntimeExport, function identity for the ChatInput leaves and the canonical hook set, and the exact public key list. Also name src/react/public.ts, the third barrel the check compares.
There was a problem hiding this comment.
🧹 Nitpick comments (3)
src/react/components/chat/README.md (3)
103-107: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd the guide contract check to the workflow.
Lines 26-27 identify
deno task docs:validateas the check for the chat guides, but this workflow lists only the chat ratchets, composability, API-reference, and link checks. Adddeno task docs:validate, or state that these commands exclude guide documentation.Proposed change
`deno task lint:chat-ratchets`, `deno task lint:chat-composability`, and -`deno task docs:api-reference:check` are the chat-specific gates; +`deno task docs:api-reference:check`, and `deno task docs:validate` are the +chat documentation gates; `deno task docs:check-links` validates the relative links in this file.As per coding guidelines, run the narrowest relevant tests first and document the required validation workflow.
🤖 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 `@src/react/components/chat/README.md` around lines 103 - 107, Update the “Working on this directory” workflow in the chat README to include deno task docs:validate alongside the existing chat-specific and documentation checks, preserving the guidance that this command validates the chat guides.Source: Coding guidelines
7-19: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRewrite the warning as concise contributor guidance.
The paragraph uses past-tense history and editorial wording such as “the worst of the four” and “exactly that.” State the current rule and authoritative sources directly.
Proposed rewrite
-> **This file is not the chat API documentation, and must not become it.** +> **Keep this file as contributor orientation, not API documentation.** > -> Every published name, signature, prop, and usage example is already carried by -> generated or contract-tested files (below). A hand-written fourth copy beside -> the source is the worst of the four: it looks authoritative because it sits -> next to the code, and nothing checks it. +> Use the generated API reference and contract-tested guides for names, signatures, +> props, and usage examples. Add runnable examples to the relevant tested guide.As per coding guidelines, use direct, concise, active, present-tense public language.
🤖 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 `@src/react/components/chat/README.md` around lines 7 - 19, Rewrite the warning in the README as concise, present-tense contributor guidance: state that the file is for orientation only, identify the generated or contract-tested files as authoritative, and direct contributors to place runnable examples in test-covered sources. Remove past-tense history and editorial phrasing while preserving the rule that this README must not become API documentation.Source: Coding guidelines
46-46: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd
chat-markdown.tsxto the layout map.
ChatMarkdownis not publicly exported, but chat composition, reasoning, and tool components use it. It adds chat-specific renderer diagnostics and forwards Markdown overrides. Do not add its test file to this source-only table.🤖 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 `@src/react/components/chat/README.md` at line 46, Update the layout map in the README to include chat-markdown.tsx alongside the other standalone components shipped through the barrel. Keep the entry source-only and do not include ChatMarkdown’s test file.Source: Learnings
🤖 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.
Nitpick comments:
In `@src/react/components/chat/README.md`:
- Around line 103-107: Update the “Working on this directory” workflow in the
chat README to include deno task docs:validate alongside the existing
chat-specific and documentation checks, preserving the guidance that this
command validates the chat guides.
- Around line 7-19: Rewrite the warning in the README as concise, present-tense
contributor guidance: state that the file is for orientation only, identify the
generated or contract-tested files as authoritative, and direct contributors to
place runnable examples in test-covered sources. Remove past-tense history and
editorial phrasing while preserving the rule that this README must not become
API documentation.
- Line 46: Update the layout map in the README to include chat-markdown.tsx
alongside the other standalone components shipped through the barrel. Keep the
entry source-only and do not include ChatMarkdown’s test file.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 9fb86410-1b6e-4b6a-86b7-389300e78e1c
📒 Files selected for processing (1)
src/react/components/chat/README.md
src/react/components/chat/README.mdwas a 391-line consumer tutorial sittingbeside the source with nothing policing it. It declared "Status: Phase 6
Complete" and "Module:
veryfront/react". A developer reading it would codeagainst it, because it looks authoritative in a way a
docs/file doesn't.What was actually wrong
Audited against
deno.json'sexports,src/chat/index.ts, and the componentsunder
src/react/components/chat/:deno.json'sexports— 8xveryfront/react, 7xveryfront/agent/react. Nothing in the file was copy-pasteable from a consumer project. The real path isveryfront/chat, which also exportsuseChat/useAgent.<AgentCard theme={{ thinking, tool }} />AgentCardPropshas notheme— it'sname/avatarUrl/messages/toolCalls/status/thinking/className/children/ref.switch (part.type),case "tool-call"ChatMessagePartis ever"tool-call". Tool activity arrives asChatToolPart—type: `tool-${toolName}`with.state/.output. It type-checks ("tool-call"is assignable to the template literal) and then silently drops every real tool call at runtime. The headline example taught a broken renderer.getTextContenthelpergetTextContentis exported from the barrel.veryfront/chatexports 354 names (144 of them runtime, persrc/chat/index.test.ts).Chat.Empty,Chat.Skeleton,Chat.If,Chat.ErrorBanner;ChatInputhas 10 leaves andMessage9+ parts.src/issrc/react/primitives/README.md; "Layer 2" survives in RFC 29 with an unrelated meaning (opt-in root context).Correct and current: the
ChatThemeandAgentThemeinterface blocks, the<Message isStreaming>semantics, and the dark-mode paragraph. That's it.Why not just fix it
The accurate parts are already policed elsewhere, with tighter guarantees:
docs/api-reference/veryfront/chat.mdis generated and CI-enforced viadocs:api-reference:check.ChatThemeis in it, with a source link.docs/guides/chat-ui.mdanddocs/guides/chat-hooks.mdare contract-testedby
tests/docs/guide-*.test.ts— the examples are compiled and asserted.Rewriting the tutorial in place would have restored the same thing that broke:
a fourth copy of the chat API surface, this one with no check behind it.
There's direct evidence that spot-fixing doesn't hold here: 375b41f ("Complete
chat composition migration") already hand-patched this file in July —
{...chat}→chat={chat},Chat.Header→Chat.Root— and left the importpath wrong and the new composition parts undocumented. It drifted again
immediately, because nothing runs against it.
Why not delete it either
Two things in this directory are real orientation that neither the generated
reference nor the guides carry, and both are contributor-only:
veryfront/chat→src/chat/index.tsisthe only specifier in
exports;veryfront/react/components/chatandveryfront/components/chatare internal aliases ontosrc/react/components/chat/index.ts. They must expose the same functionobjects, and a new export has to be threaded through all of them.
src/chat/index.test.tsandsrc/react/chat-barrels.check.tsenforce this,but nothing told you it existed.
chat/{composition,contexts, components,hooks,persistence,utils}.src/modules carry READMEs by convention (29 of them, andsrc/react/README.mdannotatescomponents/ [has README]), and these READMEsare excluded from the published package — they're for contributors.
What it is now
113 lines, down from 391. No signatures, no prop lists, no usage examples, no component
catalog — so there is no API surface left in it to go stale. It carries the
layout, the parity rule, and a table routing each kind of API claim to the doc
that owns it and the check that keeps that doc true, with a stated rule at the
top that this file must not become the API documentation again.
Every path in it is a relative markdown link, which makes it partly
self-policing:
scripts/lint/check-doc-links.tswalkssrc/, so CI now failsif any directory or file it describes is moved or renamed. Verified locally:
All 1264 doc links OK.No inbound references to the old file existed anywhere in the repo.
Review follow-up (570f075)
The parity section originally said every new export must be threaded through
all three barrels. That was wrong, and the test it cited proves it:
src/chat/index.test.ts:278-284assertschatTokens,getChatTokensCSSandColorModeProviderstay out ofveryfront/chatwhilesrc/react/components/chat/index.ts:25-37exports exactly those, andveryfront/chatin turn carries hooks (useChat,useAgent,useCompletion,…) that do not live in this directory at all. The section now states what is
actually enforced — presence across the barrels for the 32 names in
CompoundChatRuntimeExport(src/react/chat-barrels.check.ts:187-230),function identity for the
ChatInputleaves and the canonical hook set, andthe exact public key list asserted by
Object.keys— and namessrc/react/public.ts, the third barrel that check actually compares.Docs-only; no source, test, or generated artifact touched.
Summary by CodeRabbit