document responses contract additions#316
document responses contract additions#316ndycode wants to merge 2 commits intofeat/openai-parity-pr7from
Conversation
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
📝 WalkthroughWalkthroughtwo documentation files updated with new configuration and api contract information. Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
✨ Simplify code
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 |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/reference/public-api.md`:
- Around line 79-84: Update the docs entry that lists the SSE compatibility
fields to state they are conditionally synthesized: change the description in
public-api.md so it notes that output_text and reasoning_summary_text are only
emitted when outputText and reasoningSummaryText contain content (see the
outputText.length and reasoningSummaryText.length checks in
lib/request/response-handler.ts), and that commentary_text, final_answer_text
and phase_text are only added when state.phaseText has entries (see
state.phaseText.size check); add a brief sentence such as "these fields are
synthesized only when the corresponding content is present in the response
stream."
- Line 72: Combine and rephrase the three repetitive sentences that begin with
"is preserved" into a single clearer sentence: state that the plugin preserves
previous_response_id when explicitly provided, maintains text.format when
verbosity defaults are applied, and honors prompt_cache_retention from the
request body while falling back to providerOptions.openai or
provider.openai.options user config defaults; update the sentence that mentions
previous_response_id, text.format, and prompt_cache_retention accordingly to
improve readability.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: f8b26e02-871e-48e5-99a9-ba866b179e96
📒 Files selected for processing (2)
docs/development/CONFIG_FIELDS.mddocs/reference/public-api.md
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Greptile Review
🧰 Additional context used
📓 Path-based instructions (1)
docs/**
⚙️ CodeRabbit configuration file
keep README, SECURITY, and docs consistent with actual CLI flags and workflows. whenever behavior changes, require updated upgrade notes and mention new npm scripts.
Files:
docs/development/CONFIG_FIELDS.mddocs/reference/public-api.md
🪛 LanguageTool
docs/reference/public-api.md
[style] ~72-~72: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...h untouched. - prompt_cache_retention is preserved from the request body and can...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
🔇 Additional comments (5)
docs/development/CONFIG_FIELDS.md (2)
34-34: lgtm - promptCacheRetention documented correctly.the entry matches the guideline spec and aligns with
lib/request/request-transformer.ts:206-216fallback resolution logic. the common values5m|1h|24h|7dare appropriate server-side cache duration strings.
78-78: lgtm - responseContinuation documented correctly.the default
falsematcheslib/config.ts:925-937wherenormalizeBooleanfalls back to false, and the schema inlib/schemas.ts:50-54confirms it'sz.boolean().optional().docs/reference/public-api.md (3)
73-78: lgtm - hosted tool support documented correctly.the enumeration of supported hosted tools matches the implementation in
lib/request/request-transformer.ts:307-368:
tool_searchfiltered when!capabilities.toolSearchcomputer/computer_use_previewfiltered when!capabilities.computerUsenamespacebundles with recursive nested tool filteringthe filtering behavior is accurately described.
71-71: verify the text.format preservation claim.the documentation states
text.formatis preserved when verbosity defaults are applied, but the provided context snippets don't show this preservation logic. we need evidence that the request transformer handlestext.formatcorrectly.run the following script to find text.format handling:
#!/bin/bash # description: search for text.format preservation logic in request transformation # search for text.format handling rg -nP --type=ts -C5 'text\.format|text\[.format.\]' lib/request/ # search for verbosity default application rg -nP --type=ts -C5 'textVerbosity|text.*verbosity' lib/request/
70-70: documentation claims are accurate – all verified in code.the auto-fill behavior at
index.ts:1354-1359correctly implements the documented statement:previous_response_idgets auto-filled from stored session state when bothgetResponseContinuation(pluginConfig)returns true and the field isn't already set in the request body. test case attest/index.test.ts:1323-1380confirms the follow-up request receives the stored value.text.format preservation at
request-transformer.ts:989happens through the spread operator (...body.text) which leaves any existingformatfield untouched. prompt_cache_retention fallback correctly chainsbody.prompt_cache_retention→body.providerOptions?.openai?.promptCacheRetention→modelConfig.promptCacheRetentionatrequest-transformer.ts:210-214. all four hosted tool types (tool_search, mcp, computer/computer_use_preview, namespace) have type definitions inlib/types.tsand filtering logic inrequest-transformer.ts:305-352. all five sse synthesis fields (output_text, reasoning_summary_text, commentary_text, final_answer_text, phase_text) are assigned inlib/request/response-handler.ts:199-322.the section is ready as written.
|
@coderabbitai review |
|
✅ Actions performedReview triggered.
|
All review threads are resolved and later commits addressed this stale automated change request.
Summary
Stack
#315