Repository navigation
refactor(coderd/x/chatd/chatdebug): replace run-summary Postgres subtests with literal rows - #30972
ibetitsmike wants to merge 3 commits into
Conversation
… with literal rows Move the post-query reduction of AggregateRunSummary into summarizeRunSteps and cover it with table tests over literal ChatDebugStep rows. Remove the Postgres subtests whose assertions the table covers.
|
@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. 🚀 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. Delightful! 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". |
Moves the post-query reduction in
chatdebug.(*Service).AggregateRunSummaryinto a privatesummarizeRunStepsmethod and tests it with table cases over literaldatabase.ChatDebugSteprows. This deletes the 11 Postgres-backed subtests ofTestService_AggregateRunSummarywhose assertions the table covers; each one built its own database.Why it is safe:
return result.AggregateRunSummarystill owns the nil run ID check, the chatd-authorized read and the error return.NilRunID,RecomputesHasErrorAndCompletedEndpointLabelandMultipleStepsSumTokensstay on Postgres here; test(coderd/x/chatd/chatdebug): check the run-summary query boundary #30973 replaces them.summary.gowithgo test -overlay:main's tests kill 22, this PR's tests kill 31, including all 22. The new kills cover JSON null and empty error payloads, error payloads on completed steps, malformed usage and attempts, empty paths, first-match selection and caller map mutation. Of the 11 survivors, 3 are query-boundary mutants thatmainalso misses (test(coderd/x/chatd/chatdebug): check the run-summary query boundary #30973 kills them) and 8 are equivalent or only change logging.Review focus:
summarizeRunStepsinsummary.go. The moved block should matchmainapart from the return statement.Validation:
go test -race -count=1 -p 4 -parallel 4 ./coderd/x/chatd/chatdebug/...passes.golangci-lint run --fix=false ./coderd/x/chatd/chatdebug/...reports 0 issues, and the pre-commit hook passed.Stack
This stack makes the debug run-summary reduction database-free and replaces its Postgres fixtures with literal rows.