Repository navigation
Scope golden liveness errors to the exact response request - #178
Merged
Merged
Conversation
📝 WalkthroughWalkthroughChangesObserver error scoping
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Observer
participant GoldenValidation
participant ResponseConnection
Observer->>GoldenValidation: provide recorded transport errors
GoldenValidation->>GoldenValidation: filter errors by request ID and response endpoint
GoldenValidation->>ResponseConnection: validate response liveness
GoldenValidation->>GoldenValidation: aggregate response errors across observers
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@cmd/subrouter-transport-observer/golden.go`:
- Around line 3026-3043: Update observerResponseErrorCount and its
turn-validation call sites around the validated response flow to scope proxy
errors by request IDs from validatedGoldenResponseRequests, while retaining
recording_error handling. Pass the validated request-ID set into
observerScopedErrorCount for turn validation, but preserve endpoint-wide
aggregation for session summaries. Add a regression test covering two Responses
request IDs on one observer, ensuring an error for the unrelated ID does not
fail the validated request.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: ca26a2bc-9063-4e56-9bd0-2a30b4eaa7a1
📒 Files selected for processing (4)
cmd/subrouter-transport-observer/golden.gocmd/subrouter-transport-observer/golden_acceptance_regression_test.gocmd/subrouter-transport-observer/golden_migration_transition.gocmd/subrouter-transport-observer/main.go
Comment on lines
+3026
to
+3043
| func observerScopedErrorCount(stats *observerStats, matchesProxyError func(transportEvent) bool) int { | ||
| if stats == nil { | ||
| return 1 | ||
| } | ||
| count := 0 | ||
| for _, event := range stats.errorSnapshot() { | ||
| if event.Kind == "recording_error" || | ||
| (event.Kind == "proxy_error" && matchesProxyError != nil && matchesProxyError(event)) { | ||
| count++ | ||
| } | ||
| } | ||
| return count | ||
| } | ||
|
|
||
| func observerResponseErrorCount(stats *observerStats) int { | ||
| return observerScopedErrorCount(stats, func(event transportEvent) bool { | ||
| return event.Path == "/v1/responses" || event.Path == "/responses" | ||
| }) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Scope response-turn errors to validated request IDs.
observerResponseErrorCount filters only by Path. If the observer records a proxy_error for a different /v1/responses request, Line 3261 fails the healthy validated request.
Build a request-ID set from validatedGoldenResponseRequests. Pass that set to observerScopedErrorCount for turn validation. Keep endpoint-wide aggregation for session summaries.
Add a regression test with two Responses request IDs on one observer. The unrelated request error must not fail the validated request.
Also applies to: 3260-3261
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@cmd/subrouter-transport-observer/golden.go` around lines 3026 - 3043, Update
observerResponseErrorCount and its turn-validation call sites around the
validated response flow to scope proxy errors by request IDs from
validatedGoldenResponseRequests, while retaining recording_error handling. Pass
the validated request-ID set into observerScopedErrorCount for turn validation,
but preserve endpoint-wide aggregation for session summaries. Add a regression
test covering two Responses request IDs on one observer, ensuring an error for
the unrelated ID does not fail the validated request.
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.
The staging golden gate observed a healthy accepted Responses connection emitting paced chunks every 500 ms, but an unrelated Codex metadata GET returned 502 on another connection. The aggregate observer error count incorrectly invalidated exact-connection liveness and caused a safe rollback.\n\nThis records observer error identities and scopes liveness, turn validation, and summaries to Responses requests while retaining fail-closed behavior for the exact request and all evidence-recording failures.\n\nVerification:\n- regression test fails on the test-only commit and passes with the fix\n- focused observer package tests pass\n- both timing-sensitive failures from the first concurrent full run pass in isolation\n- serialized full suite is running
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.Summary by cubic
Scopes golden observer liveness, validation, and closure to the exact /v1/responses request/connection so unrelated proxy errors don’t trigger false failures. Keeps fail-closed behavior for the affected request and for evidence recording errors.
observerStatsand adderrorSnapshot().observerScopedErrorCount()andobserverResponseErrorCount()to filter by path/connection/request.waitGoldenObserverRequestConnectionsClosed(still fails on response errors),waitGoldenConnectionResponseChunkAfter,validateObserverTurns, andsummarizeGoldenSession.recording_error.Written for commit 6f973e6. Summary will update on new commits.
Summary by CodeRabbit
Bug Fixes
Tests