fix(core): preserve explicit Codex final answers - #794
Conversation
|
@codex review |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
📜 Recent review details⏰ Context from checks skipped due to timeout. (2)
🔇 Additional comments (4)
📝 WalkthroughWalkthroughAdds authoritative handling for Codex ChangesCodex final-answer authority
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant CodexJSONL
participant FinalAnswerDetector
participant SessionImporter
participant PreparedSourceRanges
participant IncrementalIndexer
CodexJSONL->>FinalAnswerDetector: provide assistant item
FinalAnswerDetector->>SessionImporter: identify explicit final_answer
SessionImporter->>PreparedSourceRanges: persist authoritative final-message reason code
IncrementalIndexer->>SessionImporter: import appended content
SessionImporter->>IncrementalIndexer: merge with authoritative final precedence
Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
evaOS review status: completedPR: #794 - fix(core): preserve explicit Codex final answers evaOS review completed for this PR head. Automation note: agents should wait for this comment to reach PR URL: #794 Review URL: #794 (review) |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@packages/core/src/index.ts`:
- Around line 5367-5375: Update existingCodexSessionHasRangeReasonCode so the
reasonCode value is escaped for SQL LIKE metacharacters, including underscores,
percent signs, and the escape character, before constructing the JSON substring
pattern. Add the corresponding LIKE escape declaration to the query while
preserving the existing quoted reason-code matching behavior.
In `@tests/codex-index.test.ts`:
- Around line 240-258: Update the test fixture in the final-answer detection
test so the event_msg and response_item shapes are exercised independently, with
each case containing only its own final-bearing item. Parameterize or split the
cases while preserving the existing assertions for both shapes, ensuring
event_msg.phase regressions cannot be masked by a response_item final.
🪄 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: a1673c8f-ab65-4f38-a12c-476be4ee62fc
📒 Files selected for processing (2)
packages/core/src/index.tstests/codex-index.test.ts
📜 Review details
⏰ Context from checks skipped due to timeout. (2)
- GitHub Check: CodeQL (javascript-typescript)
- GitHub Check: test
🔇 Additional comments (3)
packages/core/src/index.ts (2)
3062-3065: LGTM!Also applies to: 5336-5339, 5476-5479, 5496-5501, 19462-19465
19542-19572: LGTM!Also applies to: 19587-19599, 19965-20012, 20043-20051
tests/codex-index.test.ts (1)
2-2: LGTM!Also applies to: 282-324
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 796b384c15
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Walkthrough
PR: #794 - fix(core): preserve explicit Codex final answers
Head: 796b384c158239e4e41d8b935b295edf6b3a0e8d into main. Review event: COMMENT.
Provider: GLM/Z.ai through ZCode (zcode-glm, zcode, model GLM-5.2).
Estimated review effort: 1/5 (~14 min)
Changed Files
| File | Status | Churn | Purpose | Risk |
|---|---|---|---|---|
packages/core/src/index.ts |
modified | +42/-3 | Changed file | Moderate: validated P3 finding |
tests/codex-index.test.ts |
modified | +93/-1 | Test coverage | Low |
Review Signal
Validated inline findings: 3 (P0: 0, P1: 0, P2: 0, P3: 3).
Dropped findings before posting: 0. High-severity findings: 0.
Risk Taxonomy
- API compatibility: 1
- Auth: 1
- Runtime correctness: 1
Validation and Proof
1 required validation/proof recommendation(s) selected from changed files.
- required: TypeScript/web build or CI proof - Runtime TypeScript/web files or package/config files changed. Proof: npm run build; typecheck; focused Vitest; green GitHub check.
Proof status: sufficient - PR metadata mentions acceptable proof for each required validation recommendation.
Profile validation hints: Call out evidence leakage, replay/collision risks, duplicate side effects, and brittle sanitizer logic.
Profile proof expectations: Look for focused sanitizer, signature, orchestration, or fixture proof.
Related Context
Related issues/PRs: #785, #793, #791.
Suggested labels: tests.
Suggested reviewers: none from current metadata.
Review Settings Preview
- Profile: assertive
- Enabled sections: Review summary (inline_review); Walkthrough (inline_review); Changed-files table (walkthrough); Effort estimate (walkthrough); Related issues/PRs (walkthrough); Suggested labels (suggestion_only); Review status comment (sticky_status)
- Path instructions: none
- Label suggestions: orchestration, security, regression-hardening
- Reviewer suggestions: none
- Suggestion behavior: suggestions only; labels and reviewers are not auto-applied.
- Roadmap-only settings: auto-apply labels; auto-request reviewers; required status checks
Pre-merge checklist
- Inline comments target current RIGHT-side diff lines.
- No secret-like content survived into posted inline comments.
- REQUEST_CHANGES is only used when eligible P0/P1 findings survive validation.
- Required behavior proof is present or not applicable.
- Labels and reviewers are suggestions only; the bot did not auto-apply them.
evaOS review status: stale headPR: #794 - fix(core): preserve explicit Codex final answers evaOS review stopped because this queued head is no longer the live PR head. Automation note: agents should wait for this comment to reach PR URL: #794 |
evaOS review status: completedPR: #794 - fix(core): preserve explicit Codex final answers evaOS review completed for this PR head. Automation note: agents should wait for this comment to reach PR URL: #794 Review URL: #794 (review) |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f70fad8522
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Walkthrough
PR: #794 - fix(core): preserve explicit Codex final answers
Head: f70fad852267bc1755f46ce14e693170fcae64de into main. Review event: COMMENT.
Provider: GLM/Z.ai through ZCode (zcode-glm, zcode, model GLM-5.2).
Estimated review effort: 2/5 (~32 min)
Changed Files
| File | Status | Churn | Purpose | Risk |
|---|---|---|---|---|
packages/core/src/index.ts |
modified | +45/-6 | Changed file | Moderate: validated P3 finding |
tests/codex-index.test.ts |
modified | +104/-1 | Test coverage | Moderate: validated P3 finding |
tests/index-fast-skip.test.ts |
modified | +70/-1 | Test coverage | Low |
tests/prepared-cards.test.ts |
modified | +1/-1 | Test coverage | Low |
tests/prepared-source-ranges.test.ts |
modified | +1/-1 | Test coverage | Low |
tests/session-diff.test.ts |
modified | +3/-3 | Test coverage | Low |
Review Signal
Validated inline findings: 3 (P0: 0, P1: 0, P2: 0, P3: 3).
Dropped findings before posting: 0. High-severity findings: 0.
Risk Taxonomy
- Data loss: 1
- Flaky test risk: 1
- Runtime correctness: 1
Validation and Proof
1 required validation/proof recommendation(s) selected from changed files.
- required: TypeScript/web build or CI proof - Runtime TypeScript/web files or package/config files changed. Proof: npm run build; typecheck; focused Vitest; green GitHub check.
Proof status: sufficient - PR metadata mentions acceptable proof for each required validation recommendation.
Profile validation hints: Call out evidence leakage, replay/collision risks, duplicate side effects, and brittle sanitizer logic.
Profile proof expectations: Look for focused sanitizer, signature, orchestration, or fixture proof.
Related Context
Related issues/PRs: #785, #793, #791.
Suggested labels: tests.
Suggested reviewers: none from current metadata.
Review Settings Preview
- Profile: assertive
- Enabled sections: Review summary (inline_review); Walkthrough (inline_review); Changed-files table (walkthrough); Effort estimate (walkthrough); Related issues/PRs (walkthrough); Suggested labels (suggestion_only); Review status comment (sticky_status)
- Path instructions: none
- Label suggestions: orchestration, security, regression-hardening
- Reviewer suggestions: none
- Suggestion behavior: suggestions only; labels and reviewers are not auto-applied.
- Roadmap-only settings: auto-apply labels; auto-request reviewers; required status checks
Pre-merge checklist
- Inline comments target current RIGHT-side diff lines.
- No secret-like content survived into posted inline comments.
- REQUEST_CHANGES is only used when eligible P0/P1 findings survive validation.
- Required behavior proof is present or not applicable.
- Labels and reviewers are suggestions only; the bot did not auto-apply them.
evaOS review status: completedPR: #794 - fix(core): preserve explicit Codex final answers evaOS review completed for this PR head. Automation note: agents should wait for this comment to reach PR URL: #794 Review URL: #794 (review) |
There was a problem hiding this comment.
Walkthrough
PR: #794 - fix(core): preserve explicit Codex final answers
Head: 94f4583160a30bac4f252bdd9aacda230d55cc43 into main. Review event: COMMENT.
Provider: GLM/Z.ai through ZCode (zcode-glm, zcode, model GLM-5.2).
Estimated review effort: 1/5 (~16 min)
Changed Files
| File | Status | Churn | Purpose | Risk |
|---|---|---|---|---|
packages/core/src/index.ts |
modified | +43/-4 | Changed file | Low |
tests/codex-index.test.ts |
modified | +104/-1 | Test coverage | Low |
tests/index-fast-skip.test.ts |
modified | +73/-1 | Test coverage | Low |
Review Signal
No validated inline findings.
Dropped findings before posting: 0. High-severity findings: 0.
Risk Taxonomy
No finding categories.
Validation and Proof
1 required validation/proof recommendation(s) selected from changed files.
- required: TypeScript/web build or CI proof - Runtime TypeScript/web files or package/config files changed. Proof: npm run build; typecheck; focused Vitest; green GitHub check.
Proof status: sufficient - PR metadata mentions acceptable proof for each required validation recommendation.
Profile validation hints: Call out evidence leakage, replay/collision risks, duplicate side effects, and brittle sanitizer logic.
Profile proof expectations: Look for focused sanitizer, signature, orchestration, or fixture proof.
Related Context
Related issues/PRs: #785, #793, #791.
Suggested labels: tests.
Suggested reviewers: none from current metadata.
Review Settings Preview
- Profile: assertive
- Enabled sections: Review summary (inline_review); Walkthrough (inline_review); Changed-files table (walkthrough); Effort estimate (walkthrough); Related issues/PRs (walkthrough); Suggested labels (suggestion_only); Review status comment (sticky_status)
- Path instructions: none
- Label suggestions: orchestration, security, regression-hardening
- Reviewer suggestions: none
- Suggestion behavior: suggestions only; labels and reviewers are not auto-applied.
- Roadmap-only settings: auto-apply labels; auto-request reviewers; required status checks
Pre-merge checklist
- Inline comments target current RIGHT-side diff lines.
- No secret-like content survived into posted inline comments.
- REQUEST_CHANGES is only used when eligible P0/P1 findings survive validation.
- Required behavior proof is present or not applicable.
- Labels and reviewers are suggestions only; the bot did not auto-apply them.
Outcome
Preserve Codex's explicit
phase: "final_answer"as the authoritative finalmessage even when later assistant commentary is appended to the same task.
This unblocks the Hermes-first LCO return leg used by Eva to observe a Codex
acknowledgment.
What changed
event_msgandresponse_itemshapes.retains the same precedence.
Validation
node --test --import tsx --test-name-pattern='explicit Codex final_answer|append indexing preserves' tests/codex-index.test.ts— 2 passed.
node --test --import tsx tests/codex-index.test.ts— 35 passed.
node --test --import tsx tests/index-fast-skip.test.ts tests/session-diff.test.ts— 70 passed.
npm run build— passed.acknowledgment marker found through exact and recent final extraction.
npm run typecheckremains red on the same pre-existing test-only errors onclean
main@48b3d9928498c1bc5a04987e82657a2a0f824e70; repository-wide validationis delegated to the canonical CI workflow.
Safety and proof boundary
credential, environment value, or copied runtime configuration is included.
rerun after the exact merged candidate is packaged.
fix(qa): accept documented post-create diagnostics #785, or claim customer readiness.
Agent-authored PR.
Closes #793
Related: #791
Agent provenance
fix/1.6-final-answer-extraction/Volumes/LEXAR/Codex/evidence/lco-hermes-first/2026-07-28/Summary by CodeRabbit