Test: release Go-proxy RESTful contract tests verified passing in Go mode - #17468
Test: release Go-proxy RESTful contract tests verified passing in Go mode#17468euvre wants to merge 11 commits into
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe non-stream chat completion response now includes pipeline-provided audio, prompt, and creation-time metadata, with tests covering final-event metadata and answer accumulation. Go proxy REST contract-test skip groups are updated for route support and document parsing timeout behavior. ChangesChat response metadata
Go proxy REST test coverage
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
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 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #17468 +/- ##
=======================================
Coverage 90.65% 90.65%
=======================================
Files 10 10
Lines 717 717
Branches 118 118
=======================================
Hits 650 650
Misses 39 39
Partials 28 28 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
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/service/chat_session.go`:
- Around line 1695-1703: Update the metadata assignments in the non-stream
response path to run only when result.Final is true, matching the stream path’s
use of result.Final; assign AudioBinary, Prompt, and CreatedAt directly from
that final result, including empty or zero values. Add a regression test
covering conflicting intermediate and final metadata to verify only the final
event’s values are returned.
🪄 Autofix (Beta)
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: CHILL
Plan: Pro Plus
Run ID: 5add3f57-8114-424a-8e69-3cae04375875
📒 Files selected for processing (2)
internal/service/chat_session.gotest/testcases/restful_api/conftest.py
💤 Files with no reviewable changes (1)
- test/testcases/restful_api/conftest.py
- chat completions: return business error codes (101/102/109) with HTTP 200 for validation, not-found, and authorization failures instead of HTTP 500 - related_questions: validate missing question with the contract message instead of the Gin validator format; beta auth failures return code 102 "Authorization is not valid!" mirroring login_required(auth_types=AUTH_BETA) - dataset update: validate dataset_id as UUID (code 101); report nonexistent datasets as "lacks permission" so existence is not revealed (IDOR) - system tokens: accept an empty request body when creating a token - documents parse/stop: reject empty document_ids with code 102
Summary
Aligns Go error codes/messages with the established (Python) RESTful API contract where the Python side is the reasonable/established behavior, and releases the contract tests that now pass in Go-proxy mode (
API_PROXY_SCHEME=go).Go fixes:
/chat/completions(non-stream): expected domain failures (missingmessages, missingchat_id, unknown session/chat, unauthorized chat) now return HTTP 200 with business codes 101/102/109 via a newcommon.CodedError, instead of a generic HTTP 500. The non-stream response also includesaudio_binary/prompt/created_atto match Python'sdecorate_answershape, with metadata captured only from the final pipeline event./searchbots/related_questions: a missingquestionis reported with the contract message instead of the Gin validator format; beta-auth failures return code 102Authorization is not valid!, mirroring Python'slogin_required(auth_types=AUTH_BETA).PUT /datasets/{id}:dataset_idis validated as a UUID (code 101Invalid UUID format); nonexistent datasets reportlacks permission for dataset(code 102) so existence is not revealed (IDOR), matching Python.POST /system/tokens: accepts an empty request body (all fields optional) instead of HTTP 400.POST /datasets/{id}/documents/parse|stop: emptydocument_idsreturns code 102\document_ids` is required`.Released from
GO_ONLY_SKIPS(all verified passing against the Go server):test_chat_completion_validation_errors,test_chat_completion_nonstream_with_session,test_chat_completion_nonstream_with_chat_without_session,test_chat_completion_nonstream_without_chat,test_chat_completion_stream_eventstest_related_questions_contracttest_system_tokens_auth_and_crudtest_dataset_update_identifier_validation_contracttest_documents_list_default_concurrent_and_filters_contract,test_documents_download_requires_auth_and_invalid_id_contract,test_documents_parse_and_stop,test_documents_stop_parse_requires_authTests that still fail in Go mode (unimplemented routes, response-shape mismatches, and ingestion-pipeline parse timeouts) remain skipped.