docs(adr): 0004 — chat-react debug frames package shape (#84)#85
Conversation
Decision record for issue #84: ship useChatDebugFrames as a new @workkit/chat-react package rather than a subpath of @workkit/chat or a type-only export. Design-only — implementation remains help-wanted. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 51 minutes and 2 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughA new Architecture Decision Record (ADR) is added documenting the package shape and design rationale for introducing the Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Possibly related issues
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Bundle Size Report
|
There was a problem hiding this comment.
Pull request overview
Adds ADR 0004 documenting the accepted decision to ship useChatDebugFrames as a new @workkit/chat-react package (instead of a @workkit/chat/react subpath export or a type-only export), aligning with the monorepo’s runtime-separation principles.
Changes:
- Introduces ADR 0004 capturing the decision, rationale, and alternatives for the
useChatDebugFramespackage shape. - Defines intended consequences (package layout, dependency approach, and v1 API surface) for a future implementation tracked in #84.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| - `peerDependencies`: `react >= 18`, `@workkit/chat: workspace:*`. | ||
| `peerDependenciesMeta.react.optional: false` (React is required; the | ||
| package is useless without it). |
There was a problem hiding this comment.
The ADR is internally inconsistent about how @workkit/chat-react depends on @workkit/chat: option (b) describes @workkit/chat as a dependencies entry, but this section lists it under peerDependencies. Please pick one (dependency vs peerDependency) and make the wording consistent throughout, ideally aligning with existing cross-package patterns in the repo (e.g., @workkit/notify uses @workkit/mail as a peerDependency).
| - `peerDependencies`: `react >= 18`, `@workkit/chat: workspace:*`. | |
| `peerDependenciesMeta.react.optional: false` (React is required; the | |
| package is useless without it). | |
| - `peerDependencies`: `react >= 18`. | |
| `peerDependenciesMeta.react.optional: false` (React is required; the | |
| package is useless without it). `dependencies` includes | |
| `@workkit/chat: workspace:*`. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@adr/0004-chat-react-debug-frames-package-shape.md`:
- Around line 51-53: Update the ADR high-level summary (lines referencing the
new workspace package) so it matches the Decision/Consequences by declaring
`@workkit/chat` as a peerDependency using the workspace:* protocol (not a regular
dependency); specifically change the text that currently states "`dependencies`
`@workkit/chat`" to state "`peerDependencies` `@workkit/chat: workspace:*`" so
it aligns with the Decision section and the re-export design of
`@workkit/chat-react`.
🪄 Autofix (Beta)
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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 014ac63b-dafd-4787-a9ad-be6a2e6cc63b
📒 Files selected for processing (1)
adr/0004-chat-react-debug-frames-package-shape.md
Addresses CodeRabbit + Copilot review feedback on #85: option (b) described `@workkit/chat` as a `dependencies` entry in the summary but as a `peerDependencies` entry in the Consequences section. Align to peerDependencies (matches the notify → mail pattern and the v1 API plan).
|
Addressed CodeRabbit + Copilot inconsistency in f110f56 — aligned option (b) summary to |
ADR 0004 picks option (b): ship
useChatDebugFramesas a new@workkit/chat-reactpackage, not a subpath of@workkit/chat(a) and not a type-only export (c). Rationale is pinned on constitution rule 4 and the runtime-concern separation the rest of the monorepo already uses (mail/notify,ai-gateway/agent).Design-only, no implementation. Implementation remains help-wanted on #84.
Summary by CodeRabbit