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. |
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
| 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.
1 participant
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