Skip to content

chore(chat-react)!: delete the legacy sandbox-ui picker adapter - #392

Merged
drewstone merged 2 commits into
mainfrom
chore/delete-legacy-picker-adapter
Aug 6, 2026
Merged

chore(chat-react)!: delete the legacy sandbox-ui picker adapter#392
drewstone merged 2 commits into
mainfrom
chore/delete-legacy-picker-adapter

Conversation

@drewstone

Copy link
Copy Markdown
Contributor

What

Deletes the deprecated ComposerAgentControls adapter from /chat-react outright, now that the canonical pickers (ModelPicker / EffortPicker / AgentSessionControls in @tangle-network/agent-app/web-react, #390) have merged and sandbox-ui is deleting its legacy pickers in a sibling PR.

Removed:

  • src/chat-react/composer-agent-controls.tsxComposerAgentControls + ComposerAgentControlsProps
  • The selection types that only existed to serve it: ComposerModelSelection, ComposerHarnessSelection, ComposerEffortSelection, ComposerProfileSelection
  • The legacy sandbox-ui type re-exports from /chat-react (HarnessType, ModelInfo, ReasoningLevel, ReasoningEffort, AgentProfileCapability, AgentProfileDraft, AgentProfileOption) — the file's own comment said they were kept only so the adapter's props kept resolving

Kept, retyped: EntryComposer genuinely needs its controls seam, so the agent prop stays but is now the canonical AgentSessionControlsProps and renders /web-react's AgentSessionControls directly (no more menuPlacement="down" override — the canonical menus open upward; no more responsive layout — canonical layouts are inline/compact).

Consumers migrated:

  • playground/src/routes/WorkspaceRoute.tsx → canonical flat props (models/model/onModelChange, harness/onHarnessChange/availableHarnesses, effort/onEffortChange); named-profile picking dropped (not part of the canon cluster — product-owned)
  • examples/default-workspace.md capability table + sample → canonical shape
  • docs/ui-picker-canon.md → adapter marked removed (was "deprecated"); the props mapping stays as the migration path for products holding the old nested selection shape
  • AGENTS.md picker-canon section + default-workspace section updated to match
  • src/chat-react/chat-react.test.tsx → adapter behavior tests deleted (that policy now lives and is tested in /web-react + /harness); the EntryComposer assembly contract tests (control row renders iff agent is supplied, attach/submit gating) kept and re-pointed at the canonical row

Migration for downstream products: props mapping in docs/ui-picker-canon.md ("Migration: sandbox-ui → agent-app canon"). The adapter's wiring products relied on — canonical-id boundary, harness-snap suppression for per-harness remembered picks, chat-context trim — becomes product-side state; the coherence grammar itself is identical in the canonical component.

Note for reviewers: docs/api/design-canvas-react*.md + docs/api/web-react.md also changed — pnpm docs:gen picks up pre-existing drift on main (resolveNodeCachePixelRatio / NODE_CACHE_PIXEL_RATIO_MAX are exported from main's source but missing from main's committed docs). Required for the codemap-freshness gate to pass; no source change involved.

Gates

  • tsc --noEmit: 0 errors
  • vitest run src/chat-react src/web-react tests/codemap-fresh.test.ts: 28 files / 391 tests green
  • CI=true pnpm build-storybook: exit 0
  • CI=true pnpm knip: exit 0
  • pnpm docs:gen run; tests/codemap-fresh.test.ts passes
  • pnpm build (tsup): exit 0
  • Full-suite note: 19 DB-backed test files (better-sqlite3) fail in this environment because the native binding was never compiled (build/Release absent) — pre-existing, unrelated to this change; no test imports the removed symbols

BREAKING CHANGE: /chat-react no longer exports ComposerAgentControls, ComposerAgentControlsProps, ComposerProfileSelection, ComposerModelSelection, ComposerHarnessSelection, ComposerEffortSelection, HarnessType, ModelInfo, ReasoningLevel, ReasoningEffort, AgentProfileCapability, AgentProfileDraft, AgentProfileOption. EntryComposer's agent prop takes AgentSessionControlsProps instead of ComposerAgentControlsProps.

@tangletools tangletools left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Auto-approved drewstone PR — 0654d38f

This PR was opened by the trusted drewstone account.
The full PR reviewer audit still runs separately and will publish findings if it detects issues.

tangletools · auto-approval · reason: drewstone_author · 2026-08-06T08:50:18Z

@tangletools tangletools left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Value Audit — sound

Verdict sound
Concerns 0 (none)
Heuristic 0.0s
Duplication 0.0s
Interrogation 209.6s (2 bridge agents)
Total 209.6s

💰 Value — sound

Deletes the legacy ComposerAgentControls adapter (303 lines) from /chat-react now that canonical pickers in /web-react (#390) replace it; EntryComposer renders AgentSessionControls directly, no indirection.

  • What it does: Deletes ComposerAgentControls and its four Composer*Selection types from /chat-react, removes six sandbox-ui type re-exports that only existed to serve the adapter, and retypes EntryComposer's agent prop from the adapter's ComposerAgentControlsProps (nested model/harness/profile/effort sub-objects) to the canonical flat AgentSessionControlsProps from /web-react. The playgro
  • Goals it achieves: Eliminates a now-redundant adapter layer over sandbox-ui's legacy picker strip — the canonical pickers (ModelPicker/EffortPicker/AgentSessionControls in /web-react) merged in #390 (the parent commit) already replace it. Removes the indirection: EntryComposer goes from ComposerAgentControls → sandbox-ui's AgentSessionControls to directly rendering /web-react's canonical `AgentSessionC
  • Assessment: A coherent deletion following the established codebase pattern (cf. #350 removing deprecated same-origin proxy transport after all fleet migrations). The adapter was explicitly marked as deprecated/legacy before this PR; its file header, the prior chat-react/index.tsx header, and the prior AGENTS.md all warned it was frozen. Zero external consumers — grep across the full repo (excluding node
  • Better / existing approach: none — this is the right approach. The canonical pickers are in place, the adapter was already marked deprecated, and the codebase has a precedent for removing deprecated shared adapters once fleet migrations complete (#350). No alternative implementation exists — the canonical AgentSessionControls is the one and only alternative, and this PR wires directly to it.
  • Model: opencode/deepseek/deepseek-v4-pro
  • Bridge attempts: 3
  • Bridge warning: opencode/kimi-for-coding/k2p7: Bridge returned 503: {"error":{"message":"cli-bridge admission timed out after 30000ms","type":"admission_rejected","reason":"queue_timeout","admission":{"active":20,"queued":3,"maxActive":20,"maxQueue":48}}}; opencode/zai-coding-plan/glm-5.2: Bridge returned 503: {"error":{"message":"cli-bridge admission timed out after 30000ms","type":"admission_rejected","reason":

🎯 Usefulness — error

usefulness agent produced no parseable value-audit JSON.

  • Model: opencode/deepseek/deepseek-v4-pro
  • Bridge attempts: 3
  • Bridge error: opencode/zai-coding-plan/glm-5.2: Bridge returned 503: {"error":{"message":"cli-bridge admission timed out after 30000ms","type":"admission_rejected","reason":"queue_timeout","admission":{"active":20,"queued":2,"maxActive":20,"maxQueue":48}}}; opencode/kimi-for-coding/k2p7: Bridge returned 503: {"error":{"message":"cli-bridge admission timed out after 30000ms","type":"admission_rejected","reason":

No concerns — sound change, no better or existing approach found. ✅


What this audit checks

It judges the change on its merits — not whether it was tasked out in an issue. Unticketed, fast-moving work is fine; the question is whether the change is good and whether a better or existing approach should be used instead.

Pass What it asks
Heuristic Vague title? Whitespace-only or cruft-bearing diff? (content signals only)
Duplication Do added function/class names already exist elsewhere in the repo?
Value Audit What does it do? What goal does it achieve? Is it good? Better architecture or already-exists?
Usefulness Audit Does it integrate and fit? Will it hold up in real use and actually get used?

Findings are concerns, not blocks — the human reviewer decides what to do with them.

value-audit · 20260806T091453Z

@tangletools

Copy link
Copy Markdown

✅ No Blockers — 0654d38f

Review health 100/100 · Reviewer score 61/100 · Confidence 95/100 · 19 findings (2 medium, 17 low)

opencode GLM 5.2 opencode DeepSeek v4 Pro opencode DeepSeek v4 Flash aggregate
Readiness 80 73 61 61
Confidence 95 95 95 95
Correctness 80 73 61 61
Security 80 73 61 61
Testing 80 73 61 61
Architecture 80 73 61 61

Reviewer score is advisory once the run is complete and the verdict has no blockers.

Full multi-shot audit completed 8/8 planned shots over 17 changed files. Global verifier still owns final merge decision. | Full multi-shot audit completed 8/8 planned shots over 17 changed files. Global verifier still owns final merge decision. | Full multi-shot audit completed 8/8 planned shots over 17 changed files. Global verifier still owns final merge decision.

🟠 MEDIUM Breaking agent prop shape on EntryComposer — no runtime guard — src/chat-react/entry-composer.tsx

The agent prop type changed from ComposerAgentControlsProps to AgentSessionControlsProps. This renames every field: model: {value, onChange, models} becomes flat model, onModelChange, models; harness: {value, onChange, available} becomes flat harness, onHarnessChange, availableHarnesses; effort: {value, onChange} becomes flat effort, onEffortChange; profile and context are removed entirely. Every consumer's is a compile error. The ! commit marker and docs acknowledge this, but there is no runtime back-compat adapter or descriptive error to help a product that updates the package without reading the migration doc. Recommend a one-line wrapper in the first release that maps the old shape to the new for one minor cycle.

🟠 MEDIUM Chat-context trim (cli-base / non-chat models) no longer enforced by the shared composer assembly — src/chat-react/entry-composer.tsx

The deleted ComposerAgentControls defaulted context='chat': it stripped cli-base from the harness picker (harnessesForContext) and dropped non-chat-capable models (modelsForContext: image/tts/embedding/audio-in) from the model picker, with 9 tests pinning it (removed in chat-react.test.tsx). The replacement wiring agent ? <AgentSessionControls {...agent} /> renders the canonical cluster with NO such trim: HarnessPicker.options falls back to Object.keys(HARNESS_LABELS) which includes 'cli-base'→'CLI', and models pass through unfiltered (web-react/agent-session-controls.tsx:104,210). Any product migrating agent per the props mapping in docs/ui-picker-canon.md without adding availableHarnesses and pre-filtering models silently re-exposes exactly the defect the adapter prevented, contradic

🟡 LOW CODEMAP syncs a pre-existing staleness unrelated to the PR's stated change — docs/CODEMAP.md

The ./design-canvas-react row moves 129→131 and ./engine 75→77 because NODE_CACHE_PIXEL_RATIO_MAX and resolveNodeCachePixelRatio (src/design-canvas-react/export-math.ts:94,104, re-exported through engine.ts export * from './export-math') were previously omitted from the generated map. git diff base→head over src/design-canvas-react/ is empty, so these exports predate this PR and the base CODEMAP was stale — the regeneration correctly fixes it. Not a regression; informational only. No action required.

🟡 LOW Breaking deletion is in the codemap but consumer migration is out of this shot — docs/codemap.json

The codemap accurately reflects a BREAKING CHANGE: /chat-react drops 13 public symbols (ComposerAgentControls, AgentProfile*, HarnessType, ModelInfo, Reasoning*, Composer*Selection). The file itself is correct and freshness-gated, so this is a note, not a defect in this file: a published-symbol removal shipped as chore(chat-react)! must be paired with an import audit of the fleet (docs/ui-picker-canon.md migration mapping is updated in the same PR). Verified the codemap deletion set is byte-exact with src/chat-react/index.tsx:26-32 and types.ts (only ComposerPlanModeSelection remains).

🟡 LOW Codemap regen bundles catch-up entries outside the commit's stated scope — docs/codemap.json

The commit message scopes this to /chat-react picker-adapter deletion, but the codemap diff also adds NODE_CACHE_PIXEL_RATIO_MAX + resolveNodeCachePixelRatio (design-canvas) and the ChatMessages chrome prop (web-react) — symbols that landed in the base commit c1b12a6 whose codemap was stale. This commit fixes the staleness (correct direction), so it is not a defect; just an FYI that the regen touched entries beyond the commit's headline scope. No action required.

🟡 LOW Codemap regeneration includes non-PR catch-up entries — docs/codemap.json

The additions to ./design-canvas-react and ./design-canvas-react/engine (NODE_CACHE_PIXEL_RATIO_MAX, resolveNodeCachePixelRatio) are catch-up for exports that already existed in source at the base commit but were missing from the stale codemap. These aren't bugs — they're correct additions — but their presence alongside the PR's intentional chat-react removals makes the diff larger than the PR's actual scope. Consider regenerating the codemap in a separate hygiene commit next time to keep PRs surgical.

🟡 LOW ComposerPlanModeSelection doc description is module-level, not interface-specific — docs/llms-full.txt

The new description 'The vocabulary the composer surfaces speak.' is the first line of the JSDoc block in src/chat-react/types.ts, which describes the whole module's vocabulary rather than what ComposerPlanModeSelection (an {enabled, setEnabled, saving?} shape) actually is. Faithfully regenerated, so not a docs-gen bug, but a reader scanning llms-full.txt gets a generic phrase where the pre-change had none. Low-impact: fix would be in the source JSDoc (move the module summary above the interface, give the interface its own @description), not in this generated file.

🟡 LOW ComposerPlanModeSelection doc description lost its explanatory detail — docs/llms-full.txt

The entry's prose is now only 'The vocabulary the composer surfaces speak', replacing what the removed types.ts comment explained (the vocabulary is re-exported from owning packages; a second spelling of HarnessType is how products disagree). The regeneration faithfully transcribed the new source comment, so this is faithful — but the resulting line is low-information for an LLM consumer reading the full API surface. Optional; not blocking.

🟡 LOW Regenerated-by-hand docs have no in-CI sync check — docs/llms-full.txt

docs/llms-full.txt is generated by an external 'agent-docs' tool (per the header) and is not regenerated or diff-checked in CI or in the local signoff gate (signoff.config.mjs runs no doc-regeneration step). This PR happens to be fully in sync — every hunk verified against source — but nothing mechanically prevents a future source change from shipping a stale surface. Low severity because the artifact is accurate now; fix is a generator + 'git diff --exit-code' step in the gate.

🟡 LOW 'Omit an unavailable capability' is stale against the new required-props contract — examples/default-workspace.md

Lines 97-99 (retained) claim 'Omit an unavailable capability and its control stays hidden', but the canonical AgentSessionControlsProps (src/web-react/agent-session-controls.tsx:142-167) makes models/model/onModelChange/harness/onHarnessChange/effort/onEffortChange ALL REQUIRED — a product cannot omit just the model control; only the whole agent prop is optional (entry-composer.tsx:34). The capability-driven framing (lines 78-79) now holds only at whole-row granularity. The doc also omits the real per-control switch, showHarness

🟡 LOW Profile-picker placement in the modes dock contradicts modes' contract — examples/default-workspace.md

Line 94-95 suggests a product-owned profile picker live in 'the modes dock or a settings surface', but EntryComposerProps.modes (src/chat-react/entry-composer.tsx:36-39) is documented as 'an on/off switch, not a value picker, so it sits apart from agent identity'. A named-profile picker IS a value picker, so placing it in the modes dock conflicts with the component's own seam contract. Impact: products following the doc will jam a value picker into an on/off switch slot, muddying the modes affordance. Fix: recommend 'a settings surface (or a picker beside the composer)' only, and drop the modes-dock suggestion.

🟡 LOW Demo model catalog silently defeats the harness->model coherence snap for 2 of 3 offered harnesses — playground/src/routes/WorkspaceRoute.tsx

The static models array (lines 38-57) contains only anthropic ids, but availableHarnesses offers 'codex' and 'opencode'. AgentSessionControls' onHarness uses snapModelToHarness (src/web-react/agent-session-controls.tsx:183-187), and harness/index.ts:133 documents that when no catalog id fits, snapModelToHarness returns the ORIGINAL model. So selecting Codex in the demo leaves anthropic/claude-opus-4-8 selected and the UI shows an incoherent harness+model pair with no indication, and the coherence behavior (the feature AgentSessionControls exists to demo) is unobservable for 2 of 3 harnesses. This is demo-data, not a code defect (the snap-returns-orig

🟡 LOW Initial effort value silently changed from 'auto' to 'medium' — playground/src/routes/WorkspaceRoute.tsx

The previous useState<ReasoningLevel>('auto') became useState('medium'). Functionally equivalent for the canonical EffortPicker (whose DEFAULT_EFFORT_LEVELS are off/low/medium/high — 'auto' is not a level id and was already falling through to the index-2 'medium' fallback via selected = levels.find(...) ?? levels[2], src/web-react/controls.tsx:402). No user-visible regression, but worth a glance if any downstream demo expected 'auto' on the wire. No fix required.

🟡 LOW Fragile textContent test assertions — src/chat-react/chat-react.test.tsx

Tests assert document.body.textContent).toContain('GPT-4.1 Mini') and .toContain('Thinking') — broad substring matches that pass if the string appears anywhere on the page (e.g. in a different component rendered inside EntryComposer). The prior tests used screen.getByLabelText and within(menu) scoping, which were more precise. Risk is low in this controlled fixture but makes future contributor changes fragile.

🟡 LOW No-agent-controls test checks harness picker only — src/chat-react/chat-react.test.tsx

The test asserts document.querySelectorAll('[title="Agent backend"]') has length 0, which only proves the HarnessPicker is absent. If AgentSessionControls ever renders with showHarness={false} but a visible ModelPicker, this test would falsely pass. The intent is to verify the entire agent control row is absent — the simpler and more accurate check is expect(container.querySelector('.flex.items-center.gap-1\.5') or screen.queryByText('GPT-4.1 Mini')).toBeNull() since the row renders as a flex div with gap.

🟡 LOW Test comment misattributes where harness↔model coherence is tested — src/chat-react/chat-react.test.tsx

The comment states 'picker behavior — catalog grouping, harness↔model coherence, effort levels — is tested where it is implemented (src/web-react/controls.test.tsx and src/harness)'. But controls.test.tsx (44 lines) only covers ModelPicker loading/empty/search states — it does not test coherence at all. The snap functions themselves are unit-tested in harness/index.test.ts, but the WIRING in AgentSessionControls.useCoherentHandlers (lines 173-190 of agent-session-controls.tsx: onHarness calls snapModelToHarness and fires onModelChange) has no component-level test. Impact: a regression in the coherence wiring (e.g., swapped args, missing onModelChange cal

🟡 LOW cli-base chat-surface filtering and harness-snap suppression lost their framework guard — src/chat-react/entry-composer.tsx

The old ComposerAgentControls automatically dropped cli-base from chat surfaces and suppressed the model snap on harness change (to preserve per-harness remembered picks). Both are now product-side responsibilities: a product must (a) exclude 'cli-base' from availableHarnesses on chat surfaces, and (b) store per-harness picks itself. docs/ui-picker-canon.md:72-78 documents this migration, but there is no remaining framework-level guard, lint, or test that catches a product passing cli-base on a chat surface or losing per-harness pick state. This is a deliberate design decision (domain-as-parameter), not a bug in this shot's code, but the protection layer that prevented the original defects (AGENTS.md cites 'cli-base reaching a chat composer' as defect #5) is gone. Low severity because it i

🟡 LOW Breaking removal of 9 published /chat-react exports without a version bump at head — src/chat-react/index.tsx

index.tsx removes ComposerAgentControls, ComposerAgentControlsProps, ComposerProfileSelection, ComposerModelSelection, ComposerHarnessSelection, ComposerEffortSelection and re-exported types AgentProfileCapability/AgentProfileDraft/AgentProfileOption/HarnessType/ModelInfo/ReasoningEffort/ReasoningLevel, and EntryComposerProps.agent changes from nested (ComposerAgentControlsProps) to flat (AgentSessionControlsProps) with effort/onEffortChange now required even for products with no reasoning concept. Package.json at head is still 0.45.28; AGENTS.md module-map doctrine says removal of a published export is a major. The commit's ! marker is the only guard that the release step bumps the major — every fleet product importing these symbols or passing the old agent shape breaks at typecheck unt

🟡 LOW Removed type re-exports without deprecation cycle — src/chat-react/index.tsx

Seven type re-exports removed from the barrel: ComposerAgentControlsProps, ComposerModelSelection, ComposerHarnessSelection, ComposerProfileSelection, ComposerEffortSelection (from deleted composer-agent-controls) plus AgentProfileCapability, AgentProfileDraft, AgentProfileOption, HarnessType, ModelInfo, ReasoningEffort, ReasoningLevel (from types.ts). All were available from their original packages, so consumers import them from there. No internal consumer of /chat-react uses these types. The ! commit signals breaking change, which is sufficient.


tangletools · 2026-08-06T09:40:38Z · trace

@tangletools tangletools left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Approved — 19 non-blocking findings — 0654d38f

Full multi-shot audit completed 8/8 planned shots over 17 changed files. Global verifier still owns final merge decision. | Full multi-shot audit completed 8/8 planned shots over 17 changed files. Global verifier still owns final merge decision. | Full multi-shot audit completed 8/8 planned shots over 17 changed files. Global verifier still owns final merge decision.

Full immutable report for this review: trace

Summary comment for this run: full summary


tangletools · 2026-08-06T09:40:38Z · immutable trace

Remove ComposerAgentControls + ComposerAgentControlsProps and the
selection types that only existed to serve it (ComposerModelSelection,
ComposerHarnessSelection, ComposerEffortSelection, ComposerProfileSelection),
plus the legacy sandbox-ui type re-exports (HarnessType, ModelInfo,
ReasoningLevel, ReasoningEffort, AgentProfile*) from /chat-react.

EntryComposer keeps its controls seam: the `agent` prop is now the
canonical AgentSessionControlsProps from /web-react and renders the
canonical AgentSessionControls directly (its model menu IS the canonical
ModelPicker). The playground workspace route and the default-workspace
example migrate to the canonical flat props; named-profile picking is
product-owned (not part of the canon cluster).

BREAKING CHANGE: /chat-react no longer exports ComposerAgentControls,
ComposerAgentControlsProps, ComposerProfileSelection,
ComposerModelSelection, ComposerHarnessSelection, ComposerEffortSelection,
HarnessType, ModelInfo, ReasoningLevel, ReasoningEffort,
AgentProfileCapability, AgentProfileDraft, or AgentProfileOption.
EntryComposer's `agent` prop takes AgentSessionControlsProps instead of
ComposerAgentControlsProps. Migration mapping: docs/ui-picker-canon.md.
Review-driven sweep over the adapter-removal diff:

- playground: span the demo catalog across providers so the harness→model
  coherence snap is observable for Codex — an anthropic-only catalog made
  the snap a silent no-op that kept an incoherent harness+model pair.
- examples/default-workspace.md: state capability-hiding at the granularity
  the canon actually gives (omit `agent` for the row, `showHarness` for the
  backend control, reasoning-gated effort pill), and drop the modes-dock
  suggestion for profile pickers — a mode is an on/off switch, not a value
  picker, so that placement contradicted the component's own seam contract.
- web-react: pin the AgentSessionControls coherence wiring with component
  tests (harness change snaps an incompatible model, model change snaps the
  harness, compatible change fires no model snap). The snap policy units
  already lived in /harness; the wiring between them had no guard.
- chat-react: correct the test header's coverage pointers and give
  types.ts a module doc that describes what remains instead of the deleted
  adapter's vocabulary.
- docs: regenerate (agent-docs).
@drewstone
drewstone force-pushed the chore/delete-legacy-picker-adapter branch from 0654d38 to 625ed7e Compare August 6, 2026 21:30
@drewstone

Copy link
Copy Markdown
Contributor Author

signoff PASS 625ed7e (rebased onto main with the template-pin fix) — all steps ok incl. generated projects, unit tests 2 seeds, knip. Clean tree, Node 22.23.1.

@drewstone
drewstone merged commit cf18a8c into main Aug 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants