fix(cache): stabilize compression session feedback - #663
Conversation
|
Companion GoModel Pro epoch/compression change: https://github.com/ENTERPILOT/GoModel-pro/pull/16 |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
💤 Files with no reviewable changes (1)
📝 WalkthroughWalkthroughThe PR adds optional response-feedback observers and filters, integrates feedback collection into chat and Responses API handlers, and canonicalizes JSON segments used for automatic session identifiers. ChangesResponse feedback
Session anchor canonicalization
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant RequestRewriter
participant TranslatedInferenceService
participant Provider
participant ResponseFeedbackObserver
RequestRewriter->>TranslatedInferenceService: register eligible observers
TranslatedInferenceService->>Provider: request with usage reporting enabled
Provider-->>TranslatedInferenceService: completed or streaming response
TranslatedInferenceService->>ResponseFeedbackObserver: notify response metadata and usage
Possibly related PRs
Poem
🚥 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: 3
🤖 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 `@ext/ext.go`:
- Around line 84-116: Add user-facing extension documentation for
ResponseFeedbackObserver and ResponseFeedbackFilter alongside the existing
extension API contract. Document callback timing after successful rewritten
requests, the meaning of usageObserved, the requirement that observers be
concurrency-safe and return promptly, and that WantsResponseFeedback controls
per-request registration while observers without the filter receive all
successful responses.
In `@internal/server/response_feedback.go`:
- Around line 64-67: Update the non-nil resp.Usage branch in the response
feedback handling to set usage.observed = true after cacheUsageFromCore
processes the usage object, preserving confirmed zero values. Add a regression
test covering a zero-valued Responses API usage object and verify it remains
observed.
In `@internal/session/detect_test.go`:
- Around line 199-228: Add direct unit tests for canonicalSegment covering
malformed JSON and concatenated JSON values, and assert that each returns the
exact original raw segment unchanged. Keep the existing canonicalization tests
intact and target the raw fallback behavior specifically, including both
decode-error and trailing-value cases.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: fe2b0761-909f-42df-bb33-dd554fbbe51f
📒 Files selected for processing (8)
ext/ext.gointernal/server/request_rewrite.gointernal/server/request_rewrite_test.gointernal/server/response_feedback.gointernal/server/response_feedback_test.gointernal/server/translated_inference_service.gointernal/session/detect.gointernal/session/detect_test.go
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
|
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
Confidence Score: 4/5Not safe to merge until optional response-feedback filters can fail without aborting inference requests. The failure was reproduced through the real server path with an otherwise identical successful control request, directly demonstrating that the filter panic prevents provider dispatch. Files Needing Attention:
 T-Rex LogsWhat T-Rex did
Comments Outside Diff (1)
Reviews (1): Last reviewed commit: "fix(cache): stabilize compression sessio..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?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 `@internal/server/request_rewrite.go`:
- Around line 107-110: Update the panic recovery logging in the request rewrite
flow to remove the recovered value from the slog.Warn attributes, retaining only
the rewriter name or a non-sensitive panic type. Ensure data originating from
ext.Input.Body or configured credentials cannot be emitted by this log.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 00ed9f4c-e1ef-499d-a1b9-94e4565da2e1
📒 Files selected for processing (6)
ext/ext.gointernal/server/request_rewrite.gointernal/server/request_rewrite_test.gointernal/server/response_feedback.gointernal/server/response_feedback_test.gointernal/session/detect_test.go
|
Addressed all review and CI findings in
The branch already contains the latest |
3 participants
Description
This supplies the cache-domain and provider-cache evidence consumed by the companion GoModel Pro compression change.
Verification
go test ./...go test -race ./internal/server ./internal/sessionmake lintmake test-race,make fix-check, hot-path guards, and lintAI Generated (optional)
Implemented and verified with Codex.
Summary by CodeRabbit
New Features
Improvements