feat(console): register-trigger + state tool views; fix duplicated request pane - #392
Conversation
…quest pane - FunctionCallMessage: suppress the top request pane for every completed call, fixing REQUEST rendering twice on generic (no-custom-view) tool cards. - engine::register_trigger: new rich view — trigger_type → function_id, config chips for state triggers, harness::react model/join(id·key·expect·rearm)/task, plus the harness subscribe variant (label, once, no function_id → "notify session"). Falls back to raw request JSON so the terminal pane is never blank. - state::* : new family view for get/set/delete/update/list/list_groups — scope/key chips + the unwrapped result as one highlighted JSON block. - Tests (engine + state parsers) and Storybook fixtures (EngineFamily, new StateFamily gallery). Claude-Session: https://claude.ai/code/session_01SB8sknFhJLojcaBdQazmPH
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
skill-check — worker0 verified, 30 skipped (no docs/).
Four for four. Nicely done. |
📝 WalkthroughWalkthroughThis PR adds a new ChangesState function family
Engine register_trigger view
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant FunctionCallMessage
participant StateToolView
participant StateView
participant StateParsers
FunctionCallMessage->>StateToolView: isStateFunction(functionId)
StateToolView->>StateParsers: safeParseRequest(input)
StateToolView->>StateView: tryRender(functionId, input, output, running)
StateView->>StateParsers: unwrapEnvelope(output)
StateView-->>FunctionCallMessage: rendered JSON/running/empty view
sequenceDiagram
participant EngineIndex
participant RegisterTriggerView
participant Parsers
EngineIndex->>RegisterTriggerView: tryRender(input, output, running)
RegisterTriggerView->>Parsers: parse RegisterTriggerRequest(input)
RegisterTriggerView->>Parsers: parse RegisterTriggerResponse(output)
RegisterTriggerView-->>EngineIndex: rendered trigger status/details UI
Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
There was a problem hiding this comment.
🧹 Nitpick comments (2)
console/web/src/components/chat/engine/RegisterTriggerView.tsx (1)
26-167: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winSolid fallback-first design; consider adding a component test.
The parse-then-fallback pattern (raw JSON if request doesn't parse, raw JSON for unmatched config/metadata) is a good defensive default for the "always render something" requirement. This component has several conditional render branches (state chips vs raw config, react vs raw metadata, join details, running vs registered) that aren't covered by any dedicated component test in this slice — only the underlying schemas are unit-tested. A rendering test (e.g. with the new fixtures) would catch regressions in the branch logic itself.
🤖 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 `@console/web/src/components/chat/engine/RegisterTriggerView.tsx` around lines 26 - 167, Add a dedicated rendering test for RegisterTriggerView to cover the fallback-first branch logic. Use RegisterTriggerView and its existing parse/fallback paths to verify that invalid input renders raw JSON, state triggers render chips instead of raw config, react triggers render model/allow/join/task sections, and unmatched metadata/config fall back correctly. Keep the test fixture-driven so it exercises the same branches gated by safeParseRequest, safeParseResponse, hasStateChips, and react rather than only the schema validators.console/web/src/components/chat/engine/parsers.ts (1)
302-307: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueClarify the Zod object behavior
z.object()strips unknown keys by default, so this comment should say extra option keys are tolerated, not passed through.🤖 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 `@console/web/src/components/chat/engine/parsers.ts` around lines 302 - 307, Update the comment on reactOptionsSchema to match Zod’s actual behavior: z.object() strips unknown keys by default, so describe that extra option keys are tolerated/ignored rather than passed through. Keep the note focused on reactOptionsSchema and ReactOptions so it accurately reflects what the parser accepts.
🤖 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 `@console/web/src/components/chat/engine/parsers.ts`:
- Around line 302-307: Update the comment on reactOptionsSchema to match Zod’s
actual behavior: z.object() strips unknown keys by default, so describe that
extra option keys are tolerated/ignored rather than passed through. Keep the
note focused on reactOptionsSchema and ReactOptions so it accurately reflects
what the parser accepts.
In `@console/web/src/components/chat/engine/RegisterTriggerView.tsx`:
- Around line 26-167: Add a dedicated rendering test for RegisterTriggerView to
cover the fallback-first branch logic. Use RegisterTriggerView and its existing
parse/fallback paths to verify that invalid input renders raw JSON, state
triggers render chips instead of raw config, react triggers render
model/allow/join/task sections, and unmatched metadata/config fall back
correctly. Keep the test fixture-driven so it exercises the same branches gated
by safeParseRequest, safeParseResponse, hasStateChips, and react rather than
only the schema validators.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c7c47473-0352-4e7e-bfe0-f700103227c7
📒 Files selected for processing (12)
console/web/src/components/chat/FunctionCallMessage.stories.tsxconsole/web/src/components/chat/FunctionCallMessage.tsxconsole/web/src/components/chat/engine/RegisterTriggerView.tsxconsole/web/src/components/chat/engine/__tests__/parsers.test.tsconsole/web/src/components/chat/engine/index.tsxconsole/web/src/components/chat/engine/parsers.tsconsole/web/src/components/chat/state/StateView.tsxconsole/web/src/components/chat/state/__tests__/parsers.test.tsconsole/web/src/components/chat/state/index.tsxconsole/web/src/components/chat/state/parsers.tsconsole/web/src/stories/fixtures/engine-fixtures.tsconsole/web/src/stories/fixtures/state-fixtures.ts
…values
state::* results are arbitrary JSON, so running every result through the
shared sandbox error parser flagged a successful get of a value like
{ status: "denied" } / { denied_by: … } as a red "Denied" error, hiding
the real JSON on the default tab. Only run the parser for non-success
envelopes; genuine errors ({ error: { kind: "function_error" } }, wire/
denial shapes) still surface since they are never { content, details }.
Also dedupe the harness::react allow-list chips so the React key stays
unique when the engine-supplied allow list repeats a function id.
Adds state view render tests covering both directions.
What
Adds two custom tool-views to the console chat transcript and fixes a rendering bug.
1. Fix duplicated
REQUESTpaneFor any completed, generic tool call (one without a custom view), the request JSON pane rendered twice.
showRequestPaneAboveonly excluded the completed-with-custom-view case, so the top pane and the generic completed branch both emittedREQUEST. Now it's suppressed for every completed state — a one-line root-cause fix that repairs every generic tool card.2.
engine::register_triggerview (rich, react-aware)Legible view for trigger registration. Handles both wire shapes seen under this id:
RegisterTriggerInput—{ trigger_type, function_id, config, metadata }SubscribeArgs—{ trigger_type, config?, label?, once?, function_id? }(nofunction_id⇒ "notify session")Surfaces
trigger_type → function_id, config chips forstatetriggers,label/once(one-shot vs persistent), the registration/subscriptionid, and forharness::reactthemodel,options.functions.allow, join summary (id · key · expect[] · rearm), andtask. Falls back to raw request JSON so the terminal pane is never blank.3.
state::*view (new family)Minimal, uniform view for
get / set / delete / update / list / list_groups: scope/key chips + the unwrapped result rendered as one highlighted JSON block.Tests / verification
pnpm typecheckclean ·pnpm test817 pass (new engine + state parser tests) · biome lint clean ·storybook buildsucceeds.RegisterTriggerViewunder EngineFamily (react/cron/subscribe/running fixtures);StateViewunder the new StateFamily gallery.https://claude.ai/code/session_01SB8sknFhJLojcaBdQazmPH
Summary by CodeRabbit
New Features
Bug Fixes
Tests
Post-review hardening (follow-up commit
8e76e77d)Ran
/review+/shipon the branch. Two informational findings, both fixed:state::*results are arbitrary JSON, but the view ran every result through the shared sandbox error parser — so a successfulstate::getreturning{ status: "denied" }/{ denied_by: … }rendered as a red "Denied" error. Now error-parsing is skipped for success envelopes ({ content, details }); genuine errors ({ error: { kind: "function_error" } }, wire/denial shapes) still surface. Matters for this feature's gate/deploy domain, where denial-shaped state values are realistic. Addedstate/__tests__/view.test.tsx(3 tests) locking both directions.harness::reactallow-list chips now dedupe so the React key stays unique on repeated function ids.Verified:
pnpm test840 pass,tsc -bclean, biome clean on changed files.