Roles model reshuffle#832
Open
ajonkisz wants to merge 45 commits into
Open
Conversation
…ve hang Co-authored-by: bobbit-ai <bobbit@bobbit.ai>
Add browser-fixture coverage for the Roles model reshuffle against the agreed design hooks (role-row-model-control, roles-model-section, data-model-state): - List view: inherited rows resolve the per-role default via the review/session allowlist heuristic and render '<model> · default' with data-model-state= inherited and no clear/Test; inline model pick + thinking auto-saves, persists across reload, and flips to data-model-state=override with clear/Test; clearing an override reverts model + thinking to inherited; no navigation on interaction. - Detail editor: Model is its own section between Accessory and the tab bar; tab bar is Prompt + Tool Access only (no roles-tab-model); model edit is draft-based and persists via Save. - Save-hang regression: editing a field + Save returns the button from 'Saving…' to a disabled 'Save' without navigating. Gated on the success-path refetch and an rAF flush so it is red against the pre-fix code and green after, robust to renderApp()'s requestAnimationFrame coalescing. - Update the old fixture test that clicked roles-tab-model to assert the Model section instead. - Enrich the fixture's /api/models payload (input/contextWindow/maxTokens/cost) so the shared ModelSelector dialog renders selectable rows. Verified red-before/green-after by temporarily applying the design implementation (list inline control, Model section, loadRolePageData prefs fetch, handleSave renderApp fix) locally, then reverting — no production files changed in this commit. Tests are currently red pending the implementation task landing the hooks. Co-authored-by: bobbit-ai <bobbit@bobbit.ai>
…-hang, list hooks) Co-authored-by: bobbit-ai <bobbit@bobbit.ai>
Co-authored-by: bobbit-ai <bobbit@bobbit.ai>
Co-authored-by: bobbit-ai <bobbit@bobbit.ai>
Expose per-field source hierarchy (current override / inherited / default) and editability for role model + thinkingLevel via a new ConfigCascade.resolveRoleModelResolution() helper, emitted as the backwards-compatible modelResolution field on /api/roles and /api/roles/:name. Lets the Roles UI render accurate source badges without re-deriving the cascade order. Co-authored-by: bobbit-ai <bobbit@bobbit.ai>
… thinking-only, pack read-only, no-overflow) Add/update fixture + browser E2E tests for the polished Roles list model controls per the design doc. These are test-first and currently red against the two in-flight implementation branches; they encode the agreed hook contract: - role-row-model-source / role-row-thinking-source compact source badges (Role override, Inherited role override · <src>, Session/Review/Auto default) - data-model-state ∈ inherited | override | thinking-override | readonly - role-row-thinking-clear-btn to reset thinking independently - read-only pack rows show 'Managed by pack <name>' + effective model, no edits - E2E bounding-box checks: sub-controls + select chevrons stay within bounds (catches the current chevron-overflow bug), control stays compact - narrow-viewport: edit/delete actions remain reachable and unclipped Co-authored-by: bobbit-ai <bobbit@bobbit.ai>
Co-authored-by: bobbit-ai <bobbit@bobbit.ai>
…'origin/goal/a0f95537/test-engineer-4090' into goal/a0f95537/coder-9a0f
- Use server modelResolution metadata for source badges (Role override, Inherited role override · <source>, Session/Review/Auto default); degrade gracefully to the default-pref heuristic when metadata is absent. - Add independent role-row-thinking-clear-btn; model clear resets the whole override, thinking clear resets thinking only. - First-class thinking-override state; read-only pack rows show effective values + 'Managed by pack <name>' with no pickers. Co-authored-by: bobbit-ai <bobbit@bobbit.ai>
- Inline model clear/reset preserves an existing thinking override (no thinkingLevel:"" write); thinking is reset only by its own control. - Inherited-role rows surface the effective model/thinking from modelResolution even when top-level role fields are absent (no misleading "(use default)"); row state follows current-scope overrides. - Thinking-only edits no longer promote an inherited model into a fresh current-scope model override. - resolveRoleFieldSource respects PackResolver precedence: a winning market/global-user/project role field shadows lower server/builtin fields with the same role name in the metadata. Adds pinning tests for all four findings. Co-authored-by: bobbit-ai <bobbit@bobbit.ai>
Finding #1: resolveRoleFieldSource now reports the actual PackResolver whole-role winner whenever it supplies model/thinkingLevel, instead of falling through to the plain server/builtin walk for winners that sit below the editable band but above server/builtin (server-market, global-user, ancestor-project). The winnerRank>=editableRank guard only governs the role vs inherited-role label now, not whether the winner's value is used. Adds cascade tests for server-market, global-user (project scope), and project-scoped winners shadowing server/builtin. Finding #2: inline list-row model/thinking saves are no longer fire-and-forget. A per-role pending draft + serialized, coalescing save loop builds each PUT from the merged draft rather than a stale row snapshot, so picking a model then quickly changing thinking before the refetch no longer clobbers the just-selected model. Adds a deterministic save-race fixture test (slow PUT) that fails on the pre-fix code. Co-authored-by: bobbit-ai <bobbit@bobbit.ai>
Raise the fixture default per-provider timeout from 1000ms to 8000ms so ordinary fast providers are not killed by worker startup/contention during the full suite. Relax the timeout-path elapsed assertion from <1000ms to <4000ms so it still proves dispatch returns well before the 5000ms hang and records a timeout, without flaking on loaded CI. Intent preserved: the slow provider keeps a bounded 200ms budget and still times out. Co-authored-by: bobbit-ai <bobbit@bobbit.ai>
…orrect edit affordances Address Roles model-control review findings: - #1 (HIGH): feed only current-scope override values into the inline list picker; inherited/default effective values are shown read-only in the source line. Prevents renderModelRow's render-time thinking clamp from auto-saving a spurious local override just by rendering the list. - #2: gate model-clear / thinking-reset on source === 'role' (current-scope override) only; inherited-role fields no longer show no-op clear controls. - #3: in config-cascade, a field winner ABOVE the editable band (e.g. global-user while editing the server layer) is reported editable:false, since the server-layer PUT cannot override it. Pinning tests: inherited unsupported thinking does not clamp/persist on render; inherited-role values expose no clear/reset; system-scope global-user winner is read-only at the server save target. Co-authored-by: bobbit-ai <bobbit@bobbit.ai>
…ution metadata Co-authored-by: bobbit-ai <bobbit@bobbit.ai>
…model-re-4fd93412
… chevron overflow Co-authored-by: bobbit-ai <bobbit@bobbit.ai>
Adds design/roles-model-row-polish-options.html with three layout options (A recommended) addressing the model/thinking row feedback: no ellipsis on key info, readable Built-in/Custom/Pack origins, flat informational source captions (not fake buttons), and symmetric in-box reset for model + thinking. Theme tokens only; light/dark verified; narrow-width wrap demonstrated. Co-authored-by: bobbit-ai <bobbit@bobbit.ai>
… resets) Reshuffle the inline model/thinking control on the Roles list per the approved mockup (design/roles-model-row-polish-options.html, Option A): - Widen the list and move the origin badge out of the ellipsising slug into a .role-meta row so Built-in/Custom/pack labels are never reduced to '...'. - Replace the button-like .rrm-badge source pills with a flat, informational source caption: inherited fields show the resolved value + source, override fields show a quiet accent '* Role override' marker (no chip/hover). - Move the thinking reset INTO the control box beside the model reset (renderModelRow gains an opt-in onThinkingClear -> model-thinking-clear-btn), so model and thinking reset from the same place (no split-brain UX). - Let model names + source text use the available width (no aggressive truncation); theme tokens only. Settings model rows are unchanged (the new behaviour is opt-in via the helper parameter). Fixture test updated for the relocated thinking-clear hook. Co-authored-by: bobbit-ai <bobbit@bobbit.ai>
…t labels Option A copy/affordance polish (no layout redesign): - Inherited list captions now read as a sentence: "Model inherits from session default: <value>" / "<scope> role override: <value>" instead of a cryptic "Session default"/"Review default" pill, with hover titles spelling out what session vs review default means. - Drop the leftwards arrow marker that could be mistaken for a reset button; the caption is plain non-interactive text. - Name the in-box override resets precisely via opt-in renderModelRow params: "Reset model override" / "Reset thinking override" (with aria-labels); Settings rows keep "Reset model to auto"/"Reset thinking to default". - Fixture source-caption regexes relaxed to the new wording. Co-authored-by: bobbit-ai <bobbit@bobbit.ai>
Collaborator
Author
|
Reviewed for malicious intent and verifiable bugs. No malicious intent found. I did not see suspicious dependency changes, secret/token handling, shell execution, auth bypasses, or production Verifiable issues to fix before merge:
Minor: |
Co-authored-by: bobbit-ai <bobbit@bobbit.ai>
Co-authored-by: bobbit-ai <bobbit@bobbit.ai>
Co-authored-by: bobbit-ai <bobbit@bobbit.ai>
Co-authored-by: bobbit-ai <bobbit@bobbit.ai>
Co-authored-by: bobbit-ai <bobbit@bobbit.ai>
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
Saving…when saving on the current edit route.Tests
npm run checknpm run test:unitnpm run test:e2e:run -- tests/e2e/ui/roles-model-reshuffle.spec.ts --project=browsernpm run test:e2e:run -- tests/e2e/ui/marketplace.spec.ts --project=browser -g "install a pack → entities resolve with pack origin chip"🤖 Generated with Bobbit