Conversation
teknium1
left a comment
There was a problem hiding this comment.
Thanks for preserving the earlier implementation and adding config, transport, preflight, cache, and documentation coverage. The feature remains unimplemented on current main: agent/codex_responses_adapter.py:935-941 still rejects top-level text.
Problems
agent/text_verbosity.py:38treatsis_codex_backendas sufficient for the Codex branch. Current main sets that flag for anyprovider == "openai-codex"inagent/chat_completion_helpers.py:1176-1181, whilehermes_cli/runtime_provider.py:1506-1524accepts an arbitrary explicit base URL for that provider. A custom host can therefore receivetext.verbosity, contrary to the stated fail-closed routing contract.
Suggested changes
- Require the canonical ChatGPT Codex route for the Codex branch, or otherwise validate route identity, and add a custom-host regression test.
- During salvage, move the default-config hunk to
hermes_cli/config_defaults.py; current main extractedDEFAULT_CONFIGthere.
This is an automated hermes-sweeper review.
25f1e60 to
76bfc67
Compare
76bfc67 to
83d3108
Compare
d512ec7 to
9bbfb9f
Compare
|
@teknium1 This branch has been rebased onto current The prior review findings are resolved: Current-head validation: 662 focused/impact tests passed, focused Ruff passed, all three commits are SSH-signature verified, and range/worktree diff checks passed. |
aa8f7ae to
6c27090
Compare
|
Rebased onto current upstream Validation on head
@teknium1 @alt-glitch please review the current head. |
6c27090 to
dad7d45
Compare
|
Re-review requested. Rebased onto current Validation: 675 focused/impact tests passed; focused Ruff, diff checks, and all four SSH signatures passed. @teknium1 @alt-glitch please review the current head. |
dad7d45 to
f4103ab
Compare
|
Rebased the fail-closed Responses verbosity feature onto current Validation: 675 focused/impact tests passed; focused Ruff, @coderabbitai review @teknium1 @alt-glitch please re-review the current head. |
4300f7d to
6c369e5
Compare
|
@Enough1122 Final disposition on all six points for head
Validation: 675 focused/impact tests passed, focused Ruff and both diff checks passed, attribution passed, and all six upstream-range SSH signatures verified. |
|
@alt-glitch Final disposition on the competing implementations: #72638 remains the preferred consolidation. It consolidates #63543 under the current Diaspar4u-owned branch and adds fail-closed exact-host/Codex-route gating, malformed-config handling, override precedence, cache invalidation, documentation, and impact coverage beyond #20258, #29574, and #59844. No additional code change is required for this comparison. Current head: 966a279013ff2d66c4ff3aa94125b7f597b6d0ad. |
1e5b97c to
b597644
Compare
The confirmation that no other consumer relied on the looser |
b597644 to
966a279
Compare
|
@Enough1122 Final disposition: this is informational confirmation, so no code change is required. It confirms the prior routing and validation concerns are resolved; current head 966a279013ff2d66c4ff3aa94125b7f597b6d0ad preserves the same validated patch with normalized public commit identity. |
Understood — thanks for confirming. This closes the loop on the routing and validation concerns; no new issues from this side. |
966a279 to
b597644
Compare
|
@Enough1122 Final disposition: informational confirmation; no code change required. Restored canonical-identity head @teknium1 @alt-glitch Please review this exact head and approve its two pending Actions suites. |
|
@teknium1 CI run #5519 on head |
b597644 to
0307ce6
Compare
|
Rebuilt the feature on current Validation on head @teknium1 @alt-glitch Please review this exact head and approve its Actions workflows. |
0307ce6 to
8346b58
Compare
|
Resolved the final review gap on the current head: Chat Completions now strips only dict-shaped Validation on head @teknium1 @alt-glitch Please review this exact head and approve its Actions workflows. |
94e9d5f to
94aa19a
Compare
|
Resolved the current-main compatibility conflicts: the Responses request now preserves upstream wire-normalized reasoning configuration while adding Validation on head @teknium1 @alt-glitch please review this exact head. A write-access maintainer: please approve its pending Actions workflows. |
94aa19a to
936948c
Compare
|
Rebased #72638 onto current upstream Validation on head @teknium1 @alt-glitch please review this exact head. A write-access maintainer: please approve its pending Actions workflows. |
936948c to
89611ce
Compare
|
Rebased onto current upstream main at head 89611ce. The fail-closed GPT-5 Responses verbosity boundary now composes with the current Astra request sanitizer and budget-warning config without changing unsupported provider, host, model, or Chat Completions behavior. The current Windows footgun gate is satisfied with explicit UTF-8 fixture I/O. Validation: 731 focused tests passed with 4 platform skips; focused Ruff, Windows-footgun, plugin-compat, attribution, and diff checks passed. All competing implementations remain open and no equivalent behavior is merged on main. @teknium1 @alt-glitch please review this exact head. A write-access maintainer: please approve its Actions workflows. |
5d3cf93 to
a6720f2
Compare
a6720f2 to
eec08ed
Compare
|
Resolved upstream by #115880, merged as |
What does this PR do?
Adds first-class
agent.text_verbosityconfiguration for OpenAI Responses requests. Valid values arelow,medium, andhigh; empty, unset, or malformed values preserve the provider default.The setting is emitted as top-level
text.verbosityonly for supported GPT-5 requests through the canonical ChatGPT Codex route or exactapi.openai.comhost. It is stripped from Chat Completions, xAI/Grok, GitHub Models, custom endpoints, non-GPT models, and unsupported effective-model overrides.No merged upstream implementation provides this behavior. This remains the corrected consolidation of #63543 for #20203; #20258, #29574, #59844, and #63543 are still open competing implementations.
Related Issue
Fixes #20203
Type of Change
Changes Made
agent.text_verbosityonce at agent construction and invalidates cached gateway agents when it changes.text,extra_body, and service-tier fields, and keeps explicit supported overrides authoritative.turn_api_callowner, then rejects malformed explicit values during final Responses preflight without stripping unrelated scalar/listtextextensions from Chat Completions.How to Test
Current result on head
70b4a9056d4dagainst upstream7dc796463d54: 731 passed, 4 skipped. Focused Ruff, Windows-footgun, plugin-compat,git diff --check, and attribution audits pass.Checklist
Code
pytest tests/ -q— focused and impact suites above passDocumentation & Housekeeping
cli-config.yaml.exampleis updatedCONTRIBUTING.md/AGENTS.md— N/AScreenshots / Logs
N/A — request construction and configuration change only.