Skip to content

feat(engine-ux): redesign Engine page with harness-first layout and integrated model#1175

Merged
aaight merged 3 commits intodevfrom
feature/engine-page-ux-redesign
Apr 23, 2026
Merged

feat(engine-ux): redesign Engine page with harness-first layout and integrated model#1175
aaight merged 3 commits intodevfrom
feature/engine-page-ux-redesign

Conversation

@aaight
Copy link
Copy Markdown
Collaborator

@aaight aaight commented Apr 23, 2026

Summary

  • Engine-first layout: Replaces the two-card layout (detached "Model & Runtime" card + separate "Engine Settings & Credentials" card) with a single cohesive card featuring a top-level Default Engine <Select> control followed by per-engine tabs
  • Model moves inside engine tabs: The ModelField is now nested inside each engine's tab panel, so model options are always contextually relevant to the selected engine — eliminates the confusing "floating model" above the configuration
  • Hierarchy: Engine → Model → Settings → Iterations → Credentials: Each engine tab renders sections in this order, matching the user's natural decision flow
  • Simplified default selection: Per-tab "Set as Default Engine" buttons replaced by a single <Select> above the tabs that also switches the active tab when changed
  • No backend changes: projects.update already accepts all needed fields; this is a pure frontend refactor

Closes: https://trello.com/c/69ea7fb93e65c94acdfeadd6

Key Decisions

  • Controlled Tabs with independent activeTab state: The active tab and the default engine are decoupled — changing the Default Engine select also switches the active tab (natural UX), but users can still click tabs to inspect other engines' settings without changing the default
  • model state remains project-level: The DB stores a single model column; the ModelField shown in each tab is the same state variable but uses the tab's engine for model option context. Clear text-xs text-muted-foreground label explains "Project default. Per-agent overrides in the Agents tab."
  • Form ID preserved pattern: form="engine-config-form" with the submit button in CardFooter keeps the same working pattern; form moved inside CardContent
  • Removed CardDescription "Model & Runtime" card: No data model change — maxIterations moved inside each engine tab (alongside model + credentials), making it clear it's per-run context

Testing

  • All 471 web unit tests pass (npm test focused on tests/unit/web/)
  • tests/unit/web/project-navigation.test.ts passes — harness section label "Engine" and path "harness" unchanged
  • TypeScript type check passes (npm run typecheck)
  • Lint passes (npm run lint)
  • Pre-existing unrelated test failures in 5 backend test files are present on main too

Files Changed

  • web/src/components/projects/project-harness-form.tsx — primary redesign (only file changed)

🕵️ claude-code · claude-sonnet-4-6 · run details

Copy link
Copy Markdown
Collaborator

@nhopeatall nhopeatall left a comment

Choose a reason for hiding this comment

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

Summary

The PR implements the engine-first layout cleanly, but introduces a critical state mismatch bug where a user can inadvertently corrupt the project configuration by modifying the project-level model and maxIterations while viewing a non-default engine tab.

Architecture & Design

  • [BLOCKING] State Mismatch / Data Corruption Risk: The model and maxIterations state variables are project-level and apply specifically to the selected Default Engine. However, the PR renders their input fields unconditionally inside every engine tab (engines.map(...)), bound to the same model and maxIterations state.
    If a user has claude-code as the default engine, but clicks the codex tab and changes the model to gpt-4o, they are updating the projects single modelstate. When they save, the project is configured withagentEngine="claude-code"butmodel="gpt-4o"`, which will cause runs to fail due to incompatible options.

Code Issues

Blocking

  • web/src/components/projects/project-harness-form.tsx:223: The Model section must be conditionally rendered only when the tab matches the selected Default Engine (isDefault). For non-default engines, there is no "project default model" to configure—models for overrides are configured on the Agent. Wrapping this section in {isDefault && ( ... )} prevents the mismatch.
  • web/src/components/projects/project-harness-form.tsx:254: Similarly, the Max Iterations section must be conditionally rendered with {isDefault && ( ... )} to prevent a user from inadvertently changing the global iteration limit when they think they are configuring an engine override`s limits.

🕵️ opencode · google/gemini-3.1-pro-preview · run details

Model and maxIterations are project-level settings that apply only to the
default engine. Rendering their inputs on every tab created a data corruption
risk: a user editing these fields while viewing a non-default tab would save
incompatible engine/model combinations (e.g. agentEngine=claude-code but
model=gpt-4o). Both sections are now wrapped in {isDefault && (...)} so they
only appear on the default engine's tab.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Copy link
Copy Markdown
Collaborator

@nhopeatall nhopeatall left a comment

Choose a reason for hiding this comment

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

Summary

Overall this is a fantastic UX improvement. The new layout is much more intuitive and matching the hierarchy of Engine -> Model -> Settings -> Credentials makes a lot of sense. The move to controlled tabs is exactly the right approach, but there's a subtle React state initialization bug with it.

Code Issues

Blocking

  • web/src/components/projects/project-harness-form.tsx:85: The activeTab state will get out of sync with the true default engine if defaultsQuery loads asynchronously.

On initial render, defaultsQuery may still be loading, so defaults is undefined and systemDefaultEngineId defaults to 'claude-code'. useState(effectiveEngineId) initializes activeTab to 'claude-code'.
When defaultsQuery completes, it might reveal the true system default is 'codex'. effectiveEngineId reactively updates to 'codex', but activeTab stays 'claude-code' because useState ignores subsequent updates to its initial value.
As a result, the "Default Engine" select will show Codex, but the active tab will be Claude Code (and because isDefault is false for Claude Code, it won't show the Model or Max Iterations fields).

To fix this, allow activeTab to track effectiveEngineId reactively until the user manually switches tabs:

// Controlled active tab — starts at the effective engine, can be navigated independently
const [activeTab, setActiveTab] = useState<string | null>(null);
const currentTab = activeTab ?? effectiveEngineId;

Then update the Tabs component to use currentTab:

<Tabs value={currentTab} onValueChange={setActiveTab}>

Since handleEngineSelectChange already calls setActiveTab(newEffective), changing the Default Engine dropdown will correctly override the null state and switch the tab.

🕵️ opencode · google/gemini-3.1-pro-preview · run details

Initialize activeTab as null so it reactively follows effectiveEngineId
until the user manually switches tabs. This prevents the mismatch where
defaultsQuery loads after initial render and reveals a different system
default engine than the one useState initialized with.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 23, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

Copy link
Copy Markdown
Collaborator

@nhopeatall nhopeatall left a comment

Choose a reason for hiding this comment

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

Summary

The UI refactor of the Engine settings page looks solid. The transition from independent cards to an Engine-first layout with nested settings works beautifully and simplifies the user experience without requiring backend data model changes.

Architecture & Design

  • Excellent component mapping: The changes gracefully preserve the exact same / / / state variables while restructuring them into the correct domain context (active engine).
  • Correct isolation: I verified that the model input is conditionally shown using isDefault correctly so that the system still stores a single global model property rather than confusing it for a per-engine model map, maintaining exact backend API compatibility.

LGTM! All 471 web unit tests passed, typecheck passed, and lint passed.

🕵️ opencode · google/gemini-3.1-pro-preview · run details

@aaight aaight merged commit 3e555f9 into dev Apr 23, 2026
9 checks passed
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