Fix Context Builder model authority hydration - #565
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
| updateDynamicModelPolling( | ||
| startCursorPolling: false, | ||
| allowUnvalidatedDynamicPolling: false | ||
| ) |
There was a problem hiding this comment.
Bug: After provider validation, handleAgentProviderAvailabilityChanged calls updateDynamicModelPolling with startCursorPolling: false, preventing the dynamic model subscription for Cursor from starting.
Severity: MEDIUM
Suggested Fix
In handleAgentProviderAvailabilityChanged, ensure the call to updateDynamicModelPolling passes startCursorPolling: true when the validation for the Cursor provider completes successfully. This will allow the guard condition in updateCursorModelPolling to pass and initiate the dynamic model subscription.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location:
Sources/RepoPrompt/Features/ContextBuilder/ViewModels/ContextBuilderAgentViewModel.swift#L1105-L1108
Potential issue: When agent provider validation completes, the
`handleAgentProviderAvailabilityChanged` function calls `updateDynamicModelPolling` with
the `startCursorPolling` parameter set to `false`. This causes a guard condition within
the downstream `updateCursorModelPolling` function to fail, preventing the
`startCursorModelsSubscriptionIfNeeded()` method from being called. As a result, for a
user with the Cursor agent selected, the list of dynamic models will not be updated upon
validation completion and will only refresh after a subsequent user action.
Did we get this right? 👍 / 👎 to inform future reviews.
| selectedModel = AgentModel.resolvedModel(forRaw: selectedModelRaw, agentKind: selectedAgent) ?? .defaultModel | ||
| isRestoringState = false | ||
| } | ||
| updateDynamicModelPolling() | ||
| updateDynamicModelPolling(allowUnvalidatedDynamicPolling: true) |
There was a problem hiding this comment.
Bug: In selectedAgent.didSet, dynamic model polling is started but then immediately cancelled by a subsequent settings persistence flow that runs on the next run loop.
Severity: MEDIUM
Suggested Fix
Refactor the logic to prevent the immediate cancellation of polling. One option is to delay the persistAgentModelToEffectiveProfile() call or modify the applyEffectiveAgentModel function to not unconditionally stop polling. Alternatively, the state management could be adjusted so that the settings application does not override the intent to start polling.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location:
Sources/RepoPrompt/Features/ContextBuilder/ViewModels/ContextBuilderAgentViewModel.swift#L529-L532
Potential issue: In the `didSet` observer for `selectedAgent`, a call to
`updateDynamicModelPolling(allowUnvalidatedDynamicPolling: true)` is made to start
polling for dynamic models. However, this is immediately followed by
`persistAgentModelToEffectiveProfile()`, which triggers a settings change. This change
schedules `applyEffectiveAgentModel()` on the next main run loop, which in turn calls
`updateDynamicModelPolling` with `allowUnvalidatedDynamicPolling: false`. This second
call cancels the polling that was just initiated, rendering the initial
`allowUnvalidatedDynamicPolling: true` flag ineffective.
Did we get this right? 👍 / 👎 to inform future reviews.
baron
left a comment
There was a problem hiding this comment.
Blocking on exact head 12c2f5ddfb1d1d89745c7033e91f3b8044a6c304.
The acknowledged Cursor dynamic-model polling defect remains. Validation/persisted-profile application calls updateDynamicModelPolling(startCursorPolling: false), leaving restored Cursor selections without polling; the separate selection flow can start polling and then cancel it through the subsequent profile application. The related review threads remain unresolved, and making the PR ready did not change the head.
Please establish one authoritative polling transition, add deterministic restored-Cursor and selection-order regressions, resolve the threads, and rerun exact-head CI.
Audit disposition — known blocker before leaving draft (2026-08-14)The early authority hydration is the right direction, but the restored Cursor path still reconciles with Start the appropriate poll when validation makes the restored provider usable, cleanly retire any pre-validation subscription, and add a cold-launch regression for a persisted Cursor selection. Then refresh the branch and rerun exact-head CI. This is a completion request, not a closure recommendation. |
Deep-review assessment — 2026-08-14Disposition: block on the acknowledged Cursor restoration bug. Early authority hydration without persisting fallback state is the right direction, and stopping dynamic pollers during pre-validation restoration is sensible. However, the validation-completion path reconciles with Start the appropriate Cursor poller once provider validation completes, while retaining the pre-validation no-side-effect rule, and add a cold-launch regression for a persisted Cursor selection with no manual interaction. This is the remaining branch-specific blocker already identified in the PR body; resolve it before marking ready. |
Summary
Root cause
The global profile was authoritative, but PromptViewModel and ContextBuilderAgentViewModel left their cached defaults at Claude until provider validation completed. That allowed the popover and runtime cache to diverge from persisted Codex state.
Early hydration then exposed a second lifecycle issue: restored provider selection could start dynamic-model subscribers during window construction. The follow-up gates polling until validation or explicit user selection and cleans stale subscriptions during restoration.
Validation
The full pr-ready suites were attempted. They stopped on unrelated nondeterministic tests outside this diff; three failures passed immediately in isolation, and the branch-specific subscriber failure was investigated and fixed.
Known review finding
Draft status is intentional. The final autoreview reported one remaining P2: the provider-validation completion path currently calls Cursor reconciliation with startCursorPolling false, so a restored Cursor selection may not start polling until a later user action. This should be resolved before marking ready.
Closes #561