Repository navigation
refactor(coderd/x/chatd): delete the test-only advisor runtime getters - #30952
ibetitsmike wants to merge 2 commits into
Conversation
chatadvisor.Runtime.MaxOutputTokens and Runtime.ProviderOptions had no production callers. The two TestNewAdvisorRuntime subtests that used them now check the request the advisor sends to the provider: max_output_tokens, user, and reasoning effort. Delete TestNewRuntimeDeepClonesOpenAIResponsesProviderOptions. It is a one-call copy of TestAdvisorRunDisablesStoreAndIsConsistentAcrossCalls.
|
@codex review |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Codex Review: Didn't find any major issues. Already looking forward to the next diff. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
@codex review
|
|
Codex Review: Didn't find any major issues. More of your lovely PRs please. ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
@codex review
|
|
Codex Review: Didn't find any major issues. 👍 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Deletes
chatadvisor.Runtime.MaxOutputTokensandRuntime.ProviderOptions, which only tests called. The twoTestNewAdvisorRuntimesubtests that read them now run one advisor call against a provider that rejects it, and check the request body:max_output_tokens,user, andreasoning.effort. The PR also deletesTestNewRuntimeDeepClonesOpenAIResponsesProviderOptions, a one-call copy ofTestAdvisorRunDisablesStoreAndIsConsistentAcrossCalls.Why it is safe
git grepfinds no other caller of the getters. The rewritten subtests read what the provider receives instead of the runtime's stored config, so they catch more:maindefaultAdvisorMaxOutputTokensNewRuntimedrops the call template'sMaxOutputTokensReasoningEffortUserThe deleted clone test checked that
NewRuntimesucceeds, that one call returns advice, and that the parent'sStorestaystrue.TestAdvisorRunDisablesStoreAndIsConsistentAcrossCallsmakes the same checks over two calls. WithcloneProviderOptionssharing theResponsesProviderOptionspointer instead of copying it, both tests fail onmain, and the remaining one fails on this branch.Review focus
The subtests decode the OpenAI Responses request shape. The old helpers accepted both OpenAI option types, so if the transport moved this fixture to Chat Completions, these subtests would fail on the renamed fields instead of passing.
Validation
go test ./coderd/x/chatd/ -race -count=1 -run '^TestNewAdvisorRuntime$': pass.go test ./coderd/x/chatd/chatadvisor/ -race -count=1: pass, with one test fewer on this branch.go test -overlay): results as listed. A copy with a syntax error fails to build, so the overlay applies.make pre-commit) passed.