feat(workflow-settings): simplify workflow model lanes#1502
Merged
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughThis PR exposes default-workflow Plan/Triage, Executor, and Reviewer lanes in Project Models via workflow-scoped setting values, groups WorkflowSettingsPanel values by intent with display metadata, and updates tests, docs, and a changeset to reflect the split-authority storage and precedence changes. ChangesDefault Workflow Model Lanes Settings
Sequence Diagram(s)sequenceDiagram
participant User
participant SettingsModal
participant ProjectModelsSection
participant WorkflowAPI as Workflow Settings API
User->>SettingsModal: Open Project Models
SettingsModal->>ProjectModelsSection: Render with projectId, addToast
ProjectModelsSection->>WorkflowAPI: fetchWorkflowSettingValues(projectId, defaultWorkflowId)
WorkflowAPI-->>ProjectModelsSection: { stored, effective, orphaned }
ProjectModelsSection->>ProjectModelsSection: Derive lane values
User->>ProjectModelsSection: Select new executor model
ProjectModelsSection->>ProjectModelsSection: Update pending edits
User->>ProjectModelsSection: Click Save workflow models
ProjectModelsSection->>WorkflowAPI: updateWorkflowSettingValues({ settingId, patch })
alt API Success
WorkflowAPI-->>ProjectModelsSection: Updated values
ProjectModelsSection->>ProjectModelsSection: Clear pending
ProjectModelsSection->>SettingsModal: Toast success
else API Validation Error
WorkflowAPI-->>ProjectModelsSection: 400 with rejections
ProjectModelsSection->>ProjectModelsSection: Map rejections to lanes
ProjectModelsSection->>SettingsModal: Toast error
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 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 |
Contributor
Greptile SummaryThis PR adds a first-class proxy surface in Project Models settings for the default workflow's Plan/Triage, Executor, and Reviewer model lanes, backed by
Confidence Score: 5/5Safe to merge; the core proxy-edit path is well-guarded and the persistence boundary between project settings and workflow setting values is correctly maintained throughout. The load/save/reject cycle in ProjectModelsSection is covered by targeted tests for all key paths (stale workflow fallback, null-patch reset, inline rejection, no-project disabled state). The reqSeq concurrency guard prevents stale responses from landing. Workflow lane saves are deliberately separated from the regular settings save, and the MOVED_SETTINGS_KEYS tombstone boundary is not touched. No files require special attention; the vite.config.ts proxy regex has a minor edge case with query-string-suffixed source module URLs in dev, but this has no production impact. Important Files Changed
Sequence DiagramsequenceDiagram
participant U as User
participant PMS as ProjectModelsSection
participant API as Dashboard API
participant DB as workflow_settings
U->>PMS: Open Project Models (projectId set)
PMS->>API: fetchWorkflowSettingValues(defaultWorkflowId, projectId)
alt 404 – stale workflow
API-->>PMS: 404
PMS->>API: fetchWorkflowSettingValues("builtin:coding", projectId)
end
API-->>PMS: "{ stored, effective, orphaned }"
PMS-->>U: Render Plan/Triage, Executor, Reviewer dropdowns
U->>PMS: Change lane value
PMS-->>PMS: updateWorkflowLane → workflowPending
U->>PMS: Click Save workflow models
PMS->>API: updateWorkflowSettingValues(resolvedWorkflowId, workflowPending, projectId)
alt Success
API-->>DB: persist lane values
API-->>PMS: updated payload
PMS-->>U: Toast Workflow model settings saved
else 400 Rejection
API-->>PMS: "{ rejections: [{settingId, message}] }"
PMS-->>U: Inline error per lane, pending preserved
end
Reviews (3): Last reviewed commit: "Address PR review feedback (#1502)" | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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.
Inline comments:
In
`@packages/dashboard/app/components/settings/sections/ProjectModelsSection.tsx`:
- Around line 167-173: The catch block resets workflow payload, rejections and
resolvedWorkflowId but leaves workflowPending intact, risking stale edits being
applied to a different workflow; inside the same conditional (when
reqSeq.current === seq) call the state reset for pending edits (use
setWorkflowPending to clear to the initial/empty shape you use elsewhere), so
that after a failed reload you clear workflowPending along with
setWorkflowPayload, setWorkflowRejections and setResolvedWorkflowId (referencing
workflowPending, setWorkflowPending, reqSeq.current, seq, setWorkflowPayload,
setWorkflowRejections, setResolvedWorkflowId, defaultWorkflowId).
🪄 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 Plus
Run ID: 6577ab27-4fe2-4c7e-927b-6e9585256f9d
📒 Files selected for processing (12)
.changeset/workflow-model-lanes-settings.mddocs/dashboard-guide.mddocs/plans/2026-06-08-001-feat-workflow-settings-simplification-plan.mddocs/settings-reference.mdpackages/dashboard/app/components/CustomModelDropdown.tsxpackages/dashboard/app/components/SettingsModal.tsxpackages/dashboard/app/components/WorkflowSettingsPanel.csspackages/dashboard/app/components/WorkflowSettingsPanel.tsxpackages/dashboard/app/components/__tests__/SettingsModal.test.tsxpackages/dashboard/app/components/__tests__/WorkflowSettingsPanel.test.tsxpackages/dashboard/app/components/settings/sections/ProjectModelsSection.tsxpackages/dashboard/app/components/workflow-setting-display.ts
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
packages/dashboard/vite.config.ts (2)
202-202: ⚡ Quick winAdd an explanatory comment for the
.tsexclusion pattern.The regex pattern's negative lookahead is subtle and may confuse future maintainers. Please add a comment explaining that
.tsfiles are excluded to allow Vite to serve source modules from the/apidirectory while still proxying actual API endpoints.📝 Suggested comment
server: { proxy: { + // Proxy API requests to the backend, but exclude .ts files so Vite can + // serve source modules from app/api/ during development (fixes HMR 404s) "^/api(?!/.*\\.ts$)(/|$)": {🤖 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/dashboard/vite.config.ts` at line 202, Add a brief inline comment next to the proxy rule that uses the regex "^/api(?!/.*\\.ts$)(/|$)" in vite.config.ts explaining that the negative lookahead excludes .ts files so Vite can serve source modules under /api while still proxying actual API endpoints; update the comment to mention why the exclusion is necessary and give a short example (e.g., allowing /api/foo.ts to be served as a module but proxying /api/foo and /api/foo/).
202-202: ⚡ Quick winConsider excluding
.tsxfiles as well.The regex only excludes
.tsfiles, but if there are React component files (.tsx) under theapp/apidirectory, they would still be proxied to the backend and result in 404s. Consider extending the pattern to also exclude.tsx:"^/api(?!/.*\\.tsx?$)(/|$)": {This would match both
.tsand.tsxfiles.🤖 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/dashboard/vite.config.ts` at line 202, The proxy rule key in vite.config.ts currently uses the pattern "^/api(?!/.*\\.ts$)(/|$)" which only excludes .ts files; update that proxy key to also exclude .tsx files so React API route files aren't proxied (i.e., make the negative lookahead match both .ts and .tsx). Locate the proxy entry whose key is the "^/api(?!/.*\\.ts$)(/|$)" string and replace the pattern with one that excludes both extensions, keeping the rest of the proxy config intact.
🤖 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.
Inline comments:
In `@packages/dashboard/vite.config.ts`:
- Line 202: The proxy regex string "^/api(?!/.*\\.ts$)(/|$)" in vite.config.ts
fails to exclude requests like "/api/foo.ts?x=1"; update this negative lookahead
to also reject paths ending in .ts, .tsx (and optionally .jsx) even when
followed by query strings (i.e., extend the negative lookahead to match file
extensions optionally followed by a "?" and query), and add a short inline
comment next to the proxy entry explaining that these extensions are excluded so
Vite serves TS/TSX/JSX modules and HMR instead of proxying them to the backend.
---
Nitpick comments:
In `@packages/dashboard/vite.config.ts`:
- Line 202: Add a brief inline comment next to the proxy rule that uses the
regex "^/api(?!/.*\\.ts$)(/|$)" in vite.config.ts explaining that the negative
lookahead excludes .ts files so Vite can serve source modules under /api while
still proxying actual API endpoints; update the comment to mention why the
exclusion is necessary and give a short example (e.g., allowing /api/foo.ts to
be served as a module but proxying /api/foo and /api/foo/).
- Line 202: The proxy rule key in vite.config.ts currently uses the pattern
"^/api(?!/.*\\.ts$)(/|$)" which only excludes .ts files; update that proxy key
to also exclude .tsx files so React API route files aren't proxied (i.e., make
the negative lookahead match both .ts and .tsx). Locate the proxy entry whose
key is the "^/api(?!/.*\\.ts$)(/|$)" string and replace the pattern with one
that excludes both extensions, keeping the rest of the proxy config intact.
🪄 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 Plus
Run ID: f1eb963e-dd83-4a0c-af9a-a5e3108778f7
📒 Files selected for processing (1)
packages/dashboard/vite.config.ts
- Bump workflow model lane changeset to minor - Clear pending workflow model edits after value reload failures - Exclude Vite source module requests with query strings from API proxy
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
Project Models now gives users a direct place to set the default workflow's Plan/Triage, Executor, and Reviewer model lanes without digging through the full workflow policy editor. The workflow settings panel is also more usable for advanced policy editing: existing workflows open on grouped Values first, while declaration editing remains available for custom workflows.
The lane proxy persists through workflow setting values instead of resurrecting the old project-scoped model keys. It resolves stale default workflow IDs back to
builtin:coding, keeps save rejections inline on the affected lane, and uses clearer inherited/default wording in the model dropdown.Verification
pnpm --filter @fusion/dashboard typecheckpnpm --filter @fusion/dashboard exec vitest run --project dashboard-app-quality-components-b app/components/__tests__/WorkflowSettingsPanel.test.tsx --silent=passed-only --reporter=dotpnpm --filter @fusion/dashboard exec vitest run --project dashboard-app-quality-settings app/components/__tests__/SettingsModal.test.tsx --silent=passed-only --reporter=dotpnpm --filter @fusion/dashboard test:browser-smokegit diff --checkBrowser Notes
agent-browserreached the local Vite server and the dashboard API health endpoint passed, but this linked-worktree HMR session did not mount the app because Vite returned 404s for existing app-root module paths such as/api/legacy.tsand/api-node.ts. The repository browser layout smoke passed in a real local browser.Summary by CodeRabbit
New Features
Improvements
Documentation & Tests
Chores