Skip to content

Normalize Codex GPT-5 answer footer metadata#400

Draft
sepo-agent-app[bot] wants to merge 2 commits into
mainfrom
agent/implement-issue-399/codex-26856410300
Draft

Normalize Codex GPT-5 answer footer metadata#400
sepo-agent-app[bot] wants to merge 2 commits into
mainfrom
agent/implement-issue-399/codex-26856410300

Conversation

@sepo-agent-app

@sepo-agent-app sepo-agent-app Bot commented Jun 3, 2026

Copy link
Copy Markdown

Summary

  • Move footer model display logic into a testable helper.
  • Split Codex GPT-5 /reasoning and [reasoning] suffixes into the reasoning footer column.
  • Preserve plain fallback model display and non-Codex behavior.

Tests

  • npm run build
  • node --test dist/__tests__/model-display.test.js

Closes #399

@sepo-agent-app

This comment has been minimized.

@sepo-agent-app

This comment has been minimized.

Comment thread .agent/src/run.ts
Comment thread .agent/src/model-display.ts
@sepo-agent-app

This comment has been minimized.

@sepo-agent-app

This comment has been minimized.

@sepo-agent-app

sepo-agent-app Bot commented Jun 3, 2026

Copy link
Copy Markdown
Author

Sepo pushed fixes for this PR. Branch: agent/implement-issue-399/codex-26856410300. Requested by @sepo-agent.

  • Updated envelope.test.ts to validate the moved model-display.ts helper contract instead of stale run.ts symbols.
  • Adjusted Codex suffixed-model footer normalization so non-GPT-5 suffixed Codex models suppress separate env reasoning, matching adapter semantics.
  • Verified with npm --prefix .agent run build, node --test dist/__tests__/envelope.test.js, and node --test dist/__tests__/model-display.test.js.

codex | gpt-5.5/xhigh | Macmini-runner-1

@sepo-agent-app

sepo-agent-app Bot commented Jun 3, 2026

Copy link
Copy Markdown
Author

Sepo is dispatching follow-up automation.

Source Next Target Round Status
fix-pr review PR #400 5 / 40 Dispatched

Reason: agent planner selected review: The fix-pr action reported success on open PR #400, so one automated review is warranted to verify the requested corrections before stopping or leaving the PR to a human.

@sepo-agent-app

sepo-agent-app Bot commented Jun 3, 2026

Copy link
Copy Markdown
Author

Rubrics Review

Total Score Verdict Rubrics Scored
97 PASS 5
Dimension Rubric Result Score Evidence
coding_workflow / generic Read docs and linked context first pass 6/6 PR #400 closes issue #399 and implements the approved normalization plan: split Codex GPT-5 /reasoning and [reasoning] suffixes, prefer ACP-reported encoded reasoning for display, preserve fallback/non-Codex behavior.
coding_workflow / generic Reuse existing code partial 7/8 run.ts now delegates the existing footer logic to model-display.ts. Minor drift risk remains because Codex suffix parsing mirrors adapter semantics rather than sharing an exported parser.
coding_workflow / generic Make surgical changes pass 8/8 Diff is limited to run.ts, one new helper, one focused test file, and the envelope guard update; no workflow or unrelated refactor churn.
coding_style / generic Prefer simple implementations pass 7/7 The implementation is a small parser/normalizer with direct string trimming and suffix handling in model-display.ts.
coding_workflow / generic Keep docs in sync pass 7/7 Existing docs already define the footer order as `provider

Notes

  • Verified current PR head 1ca755094549b68a565fd98fa82e6cc27d93d230.
  • Local verification passed: npm --prefix .agent run build, node --test dist/__tests__/model-display.test.js, and node --test dist/__tests__/envelope.test.js.
  • Focused tests cover slash suffix, bracket suffix, resumed/no-session-model fallback, non-GPT-5 Codex suffix suppression, and non-Codex behavior.
  • Smallest useful follow-up: consider sharing Codex suffix parsing with ACP model selection if this logic grows beyond display normalization.

Findings

  • INFO: Under reuse-existing-code, the PR still duplicates Codex suffix recognition concepts near adapter behavior. This is acceptable for the current surgical change, but it is the only rubric-specific maintainability caveat.

Final Rubric Verdict

PASS


codex | gpt-5.5/xhigh | Macmini-runner-5

@sepo-agent-app

sepo-agent-app Bot commented Jun 3, 2026

Copy link
Copy Markdown
Author

AI Review Synthesis

Dual-agent review by Claude and Codex.
Requested by @lolipopshock.

Summary of PR/Issue

  • PR Normalize Codex GPT-5 answer footer metadata #400 closes issue #399, which investigated inconsistent answer-route footer metadata between fresh and resumed Codex runs.
  • The approved plan was to keep the same answer template while normalizing Codex GPT-5 encoded reasoning into a stable footer shape.
  • The PR moves footer model display logic into .agent/src/model-display.ts, updates run.ts to delegate to it, and adds focused tests for encoded GPT-5, fallback, non-GPT-5 Codex suffixes, and non-Codex behavior.
  • Current head is 1ca755094549b68a565fd98fa82e6cc27d93d230.

Review

  • The current reviewer artifacts agree that the prior blocking test failure and Codex suffix semantics warning are fixed.
  • Claude and rubrics note a deferrable INFO-level duplication risk around Codex suffix parsing; Codex reports no findings. No unresolved branch-change work remains.
Issue Severity Description
No actionable issues remain INFO Current review artifacts and PR diff show the earlier blockers are resolved; remaining notes are optional follow-up, not required PR work.

Progress

  • Current diff confirms envelope.test.ts now checks the model-display.ts helper contract.
  • Current diff confirms non-GPT-5 Codex suffixed models now suppress separate env reasoning, matching adapter semantics.
  • Reviewer artifacts report passing build, targeted tests, and full .agent suite coverage.
  • Resolved two existing same-agent inline threads: .agent/src/run.ts:63 and .agent/src/model-display.ts:52.
  • No new inline comments were posted, no replies were added, and no comments were minimized.

Recommended Next Step

NO_AUTOMATED_ACTION: no unresolved actionable branch changes remain; humans can decide draft or merge readiness outside this synthesis.

Final Verdict

SHIP

Action Items

  • No required branch-change work remains.

codex | gpt-5.5/xhigh | Macmini-runner-6

@lolipopshock

Copy link
Copy Markdown
Contributor

Is the gpt5 prefix etc overly hacky? Is it a generic enough implementation

@sepo-agent-app

sepo-agent-app Bot commented Jun 3, 2026

Copy link
Copy Markdown
Author

I would not call it overly hacky for this PR. It is provider-specific, but it is scoped to the provider behavior that already exists in ACPX model selection: Codex GPT-5 reasoning is encoded into model IDs like gpt-5.5/xhigh, while other providers keep reasoning separate.

The gpt-5 prefix check is reasonable as a guard, not a generic abstraction. It prevents the display formatter from splitting arbitrary slash-looking model names while still handling the known Codex GPT-5 shape: model-display.ts#L1-L27. That lines up with the runtime adapter’s existing semantics: acpx-adapter.ts#L232-L275.

So my read is:

  • Generic enough at the display boundary: buildModelDisplay still takes agent, model, reasoningEffort, and runnerName, then normalizes only when needed: model-display.ts#L44-L78.
  • Intentionally not provider-generic inside the Codex branch. That is fine because this is not a universal model parser; it is formatting ACPX/Codex effective model metadata.
  • The one real maintainability caveat is duplicated Codex suffix knowledge between acpx-adapter.ts and model-display.ts. I would not block on that now, but if this grows, the right follow-up is to extract a shared codex-model helper for parse/compose semantics.

The tests cover the important anti-hacky cases: GPT-5 slash/bracket split, resumed fallback, non-GPT-5 Codex suffix suppression, and non-Codex passthrough: model-display.test.ts#L6-L59. So I’d treat this as acceptable and surgical, with a small future refactor only if Codex model encoding keeps expanding.


codex | gpt-5.5/xhigh | Macmini-runner-6

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

agent Handled by the agent

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Investigate answer-route footer/template metadata inconsistency

2 participants