chore(ui): add shared tool contract export#669
Merged
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (12)
✅ Files skipped from review due to trivial changes (2)
🚧 Files skipped from review as they are similar to previous changes (7)
📝 WalkthroughWalkthroughThis PR centralizes tool-name string constants into a single ChangesTool Contract Centralization and Adoption
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request centralizes tool name constants by introducing a new @opencode-ai/ui/tool-contract module and replacing hardcoded string literals throughout the app and ui packages. It also adds contract tests to verify consistency with the opencode package and updates the frontend architecture manifest. I have no feedback to provide.
Perf delta summaryComparator: pass
|
This was referenced May 16, 2026
Astro-Han
force-pushed
the
codex/frontend-architecture-governance
branch
from
d45ab7b to
e3be0a7
Compare
Astro-Han
force-pushed
the
codex/frontend-interface-contracts
branch
from
1550833 to
f2519e5
Compare
This was referenced May 16, 2026
Astro-Han
force-pushed
the
codex/frontend-interface-contracts
branch
from
f2519e5 to
ee885f6
Compare
Astro-Han
changed the base branch from
codex/frontend-architecture-governance
to
dev
This was referenced May 16, 2026
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/ui/src/components/tool-contract.test.ts (1)
25-26: ⚡ Quick winMake literal matching quote-agnostic in compatibility checks.
The current assertion can fail on harmless quote-style changes (
'vs"), creating noisy test failures.Proposed change
- expect(source.includes(`"${tool}"`)).toBe(true) + expect(source).toMatch(new RegExp(`["']${tool}["']`))🤖 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 `@packages/ui/src/components/tool-contract.test.ts` around lines 25 - 26, The test assertion is brittle to quote-style changes: instead of checking source.includes(`"${tool}"`), update the assertion in tool-contract.test.ts to be quote-agnostic by matching the tool identifier with either single or double quotes (e.g., use a regular-expression test or check both quoted variants) when calling expect on source; reference the variables/source and the expect(...) assertion so you replace the includes check with a regex that matches ['"]{tool}['"] (or a logical OR of `'${tool}'` and `"${tool}"`) to avoid failing on harmless quote-style changes.
🤖 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 `@packages/ui/src/components/tool-contract.test.ts`:
- Around line 25-26: The test assertion is brittle to quote-style changes:
instead of checking source.includes(`"${tool}"`), update the assertion in
tool-contract.test.ts to be quote-agnostic by matching the tool identifier with
either single or double quotes (e.g., use a regular-expression test or check
both quoted variants) when calling expect on source; reference the
variables/source and the expect(...) assertion so you replace the includes check
with a regex that matches ['"]{tool}['"] (or a logical OR of `'${tool}'` and
`"${tool}"`) to avoid failing on harmless quote-style changes.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 1113a46b-a377-417e-bbfe-cd5f716489cc
📒 Files selected for processing (11)
packages/app/src/pages/session/session-status-extractors.tspackages/app/src/ui-tool-contract.test.tspackages/ui/package.jsonpackages/ui/src/components/message-part-registry.test.tspackages/ui/src/components/message-part-stale.test.tspackages/ui/src/components/message-part/grouping.tspackages/ui/src/components/message-part/parts/tool.tsxpackages/ui/src/components/message-part/shared-utils.tspackages/ui/src/components/tool-contract.test.tspackages/ui/src/components/tool-contract.tspackages/ui/src/components/tool-info.ts
Astro-Han
force-pushed
the
codex/frontend-interface-contracts
branch
from
ee885f6 to
c07ea8d
Compare
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Suggested priority: P2 (includes user-path files (packages/app/src/pages/session/session-status-extractors.ts, packages/app/src/ui-tool-contract.test.ts)).
P1/P0 are reserved for maintainer confirmation. Please relabel manually if this is a release blocker, security issue, data-loss risk, or updater/runtime failure.
Astro-Han
force-pushed
the
codex/frontend-interface-contracts
branch
from
c07ea8d to
7d909e7
Compare
github-actions
Bot
added
the
harness
Owner
Author
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
Astro-Han
added a commit
that referenced
this pull request
May 16, 2026
Goal: Extract the timeline staging owner out of MessageTimeline as the first #601 message-flow stack root. Scope: - Move createTimelineStaging into packages/app/src/pages/session/session-timeline-staging.ts. - Keep MessageTimeline wired with the same sessionKey, turnStart, renderedUserMessages, and { init: 10, batch: 3 } config. - Add browser-condition staging tests for non-windowed render, staged batches, active-session message growth, completed-session backfill, and session switch rAF cancellation. - Preserve active staging when the same session receives more messages mid-stage so the historical window does not pop to full render. - Add the concrete #670 boundary to the frontend architecture manifest. Verification: - bun --cwd packages/app test --preload ./happydom.ts src/pages/session/session-timeline-staging.test.ts src/pages/session/use-session-history-window.test.ts src/pages/session/session-timeline-scroll-controller.test.ts src/pages/session/session-timeline-scroll-anchors.test.ts -> 1110 pass / 2674 expects - bun run typecheck -> 8 successful tasks - git diff --check - GitHub checks green, including ci, unit-app, unit-opencode, unit-desktop, unit-ui-focused, desktop smoke, e2e artifacts, perf-probe-baseline, CodeQL, and CodeRabbit. - reviewThreads unresolved = 0 Review follow-ups: - Fixed Gemini staging-pop thread and resolved it after replying in-thread. - Added the missing manifest entry that the PR body claimed. - Refreshed PR body verification after #667 and #669 landed on dev. Residual risk: - Electron manual verification was not run because this is scoped to behavior-preserving extraction plus tests, with no visible UI or copy change.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Add a small public tool-name contract for frontend consumers:
@opencode-ai/ui/tool-contractWhy
#638 calls out hardcoded tool/event names drifting silently across packages. This PR starts with the smallest reversible contract boundary: shared tool-name constants that both
packages/appandpackages/uican import, with tests that fail if the public export disappears or the opencode tool literal changes.Related Issue
Related to #638 and #599.
Human Review Status
Pending. A human should make the final merge decision after reviewing the final diff and verification evidence.
Review Focus
@opencode-ai/ui/tool-contractthe right public home for frontend tool-name constants?Topology
devcodex/frontend-interface-contractsTouched Files
packages/ui/package.jsonpackages/ui/src/components/tool-contract.tspackages/ui/src/components/tool-contract.test.tspackages/app/src/ui-tool-contract.test.tspackages/app/src/pages/session/session-status-extractors.tspackages/ui/src/components/message-part-registry.test.tspackages/ui/src/components/message-part-stale.test.tspackages/ui/src/components/message-part/grouping.tspackages/ui/src/components/message-part/parts/tool.tsxpackages/ui/src/components/message-part/shared-utils.tspackages/ui/src/components/tool-info.tsPublic Contract Diff
@opencode-ai/ui/tool-contractTOOL_TODOWRITE,TOOL_WEBFETCH,TOOL_WEBSEARCH,TOOL_QUESTION,TOOL_AGENT_LEGACY,TOOL_AGENTTOOL_CONTRACT_NAMES,ToolContractName, andHIDDEN_TOOL_NAMESCompatibility:
session-status-extractors.tsstill re-exports the four constants it previously owned, so existing app imports do not break.@opencode-ai/ui/tool-contract, proving the new package export resolves for an app consumer.packages/opencode/src/tool/*source literals.Risk Notes
Low runtime risk. This changes constants and imports only. The main review risk is public API shape: if
@opencode-ai/uishould not own this contract, this PR should be redirected before more consumers depend on it.How To Verify
CI note, 2026-05-16:
perf-probe-baselinefailed in the base-side capture job, not in this PR's changed code path. The observed failure wassession-scroll-reading-longbase coverage0.6457533077251387 < 0.95; this PR does not touch app perf/scroll/runtime files. The failed run was rerun after this body cleanup.Screenshots or Recordings
Not applicable. No visible UI changes.
Checklist
dev, and my PR title and commit messages use Conventional Commits in EnglishSummary by CodeRabbit