Repository navigation
fix(verifier): retain artifact and review-comment evidence destinations - #3836
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Warning Review limit reachedYour organization has reached its usage spending cap. Adjust your spending cap in the billing tab. Next included review available in 8 minutes. View limit details
📝 Summary
Pre-merge checks |
|
|
Runner dispatch state for codex on PR #3836. Do not edit. |
🤖 Keepalive Loop StatusPR #3836 | Agent: Codex | Iteration 3/12 Current State
Agent Delegation (auto mode)
🔍 Failure Classification| Error type | infrastructure | 🧠 Task Analysis| Provider | 🔶 Regex (fallback) |
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 85f990cfca
ℹ️ 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".
|
Post-push review checkpoint on unchanged head
Keeper owns this coherent review-recovery pass, current CI and thread resolution; closer requires fresh complete expected topology, unchanged head, zero active threads, elapsed420s floor and actual compare before acceptance. No human/product hold or CodeRabbit-cap blocker is asserted. |
Keepalive Work Log (click to expand)
|
|
Runner dispatch state for codex on PR #3836. Do not edit. |
|
Runner dispatch state for codex on PR #3836. Do not edit. |
✅ Codex Completion CheckpointIteration: 1 Tasks Completed
Acceptance Criteria Met
About this commentThis comment is automatically generated to track task completions. |
|
Runner dispatch state for codex on PR #3836. Do not edit. |
|
Runner dispatch state for codex on PR #3836. Do not edit. |
|
Recovered campaign checkpoint for the existing source owner, not a second worker request: keepalive run 38170641097 is already actively implementing on current head 430d1b4. Adopt current Codex finding discussion_r4242906490 (No need/no requirement to attach evidence as an artifact) and explicitly disposition Semgrep discussion_r4242907764 on the new XML proof parser; do not waive the failing Semgrep check. New generated TPP finding stranske/Travel-Plan-Permission#1647 (comment) adds the exact destination-first regression - [ ] PR comment with test evidence. Ensure direct and absent/unavailable-comments floor controls cover it, then request originating exact-head disposition after the fix. Generated TPP head remains 18c24879e80bdddc9bc45cf00ba79283a994366c; Maint71 alone owns its eventual disposition after accepted source regeneration. Keep #3831 native capture/proof recovery separate. No additional writer, general review or generated mutation is dispatched by this comment. |
|
Runner dispatch state for codex on PR #3836. Do not edit. |
|
Opener drain audit on current head
Current JavaScript Tests/github scripts tests/test-quality/Semgrep failures also require the actual failing logs and focused validation. Preserve root/template parity and original raw proof history. After recovery, closer rechecks the full current-head topology, all active threads and a fresh seven-minute review floor before merge/actual comparison. No merge is armed by this audit. |
|
Runner dispatch state for codex on PR #3836. Do not edit. |
|
Runner dispatch state for codex on PR #3836. Do not edit. |
Automated Status SummaryHead SHA: fe15da5
Coverage Overview
Coverage Trend
Top Coverage Hotspots (lowest coverage)
Low Coverage Files (<50.0%)
Updated automatically; will refresh on subsequent CI/Docker completions. Keepalive checklistScopeCurrent delivered Workflows source Context for AgentRelated Issues/PRsTasks
Acceptance criteria
|
|
@codex review Please review exact head 99f4fdd. The active nominal negative-governor finding is repaired through the shared governor and body predicate classifier, not an artifact-only exception; source/template are identical. Original regressions: 34 failed; fixed focused gate: 220 passed; full relevant suite: 93,014 passed. The replay security finding is repaired with Expat handlers rejecting DTD/entity declarations; ordinary JUnit plus internal/external declaration controls pass (4 tests). Original three-rule reversal/restoration replay still gives 3 RED assertion failures and 3 GREEN passes per rule. See committed docs/evidence/issue-3835-evidence-destinations/follow-up-20261011.json and README for scope and proof bindings. Preserve the runtime-file cleanup. No generated PR mutation or merge authority is requested. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 99f4fddd22
ℹ️ 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".
|
@codex review Review exact head e252bbc. Both new findings were adopted: shared nominal exists-negation and review-comment UI/API/renderer product subjects. Deliberate RED 45 failures / 225 tests; focused GREEN 285; broader two-module GREEN 93,075. Immutable source/test/proof hashes are in docs/evidence/issue-3835-evidence-destinations/review-follow-up-20261011.json; source/template parity preserved, docs/manifest updated. Original finding-only assessment at prior head 99f4fdd was completed PASS but explicitly not whole-PR acceptance, and cannot authorize this new head. No edits or merge actions requested. |
Provider Comparison ReportProvider Summary
📋 Full Provider Details (click to expand)openai
anthropic
Agreement
DisagreementNo major disagreements detected. Unique Insights
🔍 LangSmith Traces |
Tasks
Acceptance Criteria
The campaign lane owns this local follow-up to #3835. The original implementation and evidence follow below. Post-merge regeneration and campaign reconciliation are coordination, not implementation tasks.
Closes #3835.
The verifier discarded mandatory artifact delivery, PR/inline review-comment destinations, and validation-output checklist evidence, allowing a model PASS despite missing named evidence. This change retains those destinations through the existing polarity/product/literal and destination-OR grammar, with the source and consumer template kept byte-identical.
Tasks
scripts/langchain/pr_verifier.py::_required_evidence_channelsshared artifact operation/destination grammar to recognize active evidence delivery usingas an artifact.scripts/langchain/pr_verifier.py::_required_evidence_channelsshared review destination vocabulary for PR review comments and inline review comments.scripts/langchain/pr_verifier.py::_required_evidence_channelschecklist noun handling using existing common-proof/polarity vocabulary so Validation output/results retain the named comment destination.tests/scripts/test_pr_verifier_delivery_destinations.pyfor the three original examples and bounded positive/negative/optional/product/literal controls.tests/scripts/test_pr_verifier_prompt_coverage.pywith body present but comments absent/unavailable, and body present but artifacts absent/unavailable.templates/consumer-repo/scripts/langchain/pr_verifier.pywith the source fix..github/sync-manifest.ymland the boundary contract indocs/ops/CONSUMER_REPO_MAINTENANCE.md.Acceptance Criteria
python -m pytest tests/scripts/test_pr_verifier_delivery_destinations.py tests/scripts/test_pr_verifier_prompt_coverage.py -qpasses on the fixed bytes; deliberate restoration of each original grammar defect makes its named regression fail, followed by byte-identical restoration and GREEN.Attach evidence as an artifactandUpload the coverage report as an artifactrequire artifacts, and missing/unavailable artifacts block a model PASS even with a present PR body.Evidence must be posted in a PR review commentand the inline-review-comment variant require comments; absent/unavailable comments cannot be masked by a present PR body.- [x] Validation output in a PR commentrequires comments; negative, optional, quoted-literal and product-capability controls remain nongating unless a separate mandatory delivery exists.templates/consumer-repo/scripts/langchain/pr_verifier.pyare byte-identical for the changed verifier, with manifest/docs coverage and retained RED/GREEN commands/results in the ready implementation PR.Validation
python -m pytest tests/scripts/test_pr_verifier_delivery_destinations.py tests/scripts/test_pr_verifier_prompt_coverage.py -q -o addopts= -n 4: 92,822 passed, zero failures/errors/skips.a743cbfed0bdd653a2a33ee3061650b75175d84ded1031604908343c12fc9c53.docs/evidence/issue-3835-evidence-destinations/retains the replay, exact commands/process receipts, full raw logs/JUnit and SHA256 manifest; all 28 archive-member hashes and five current input hashes verified.This proves the bounded local grammar and coverage-floor contract. Native-provider acceptance, #3820/#3831 capture partitioning and downstream promotion/reconciliation remain separate work. The initial serial run was intentionally interrupted and then replaced by the complete four-worker run; its partial logs are retained without a full-gate claim.
Closes #3835
Automated Status Summary
Scope
Current delivered Workflows source
49286cc28411f1efb5545866ed0f278e6c680fdb,scripts/langchain/pr_verifier.pyin_required_evidence_channels(consumer lines 3886/4037), still loses three mandatory evidence destinations. Actual trip-planner #1885 headb28e40427fe68fb9848d0a988a9149463e10058cfindings stranske/trip-planner#1885 (comment) and stranske/trip-planner#1885 (comment) reproduce:Attach evidence as an artifactreturns no channel;Evidence must be posted in a PR review commentreturns overall instead of comments. Independently reproduced- [ ] Validation output in a PR commentalso returns no channel. This is a current deterministic completeness defect, not optional stylistic coverage. Required evidence can be missing while a model PASS survives the floor.Context for Agent
Related Issues/PRs
Tasks
scripts/langchain/pr_verifier.py::_required_evidence_channelsshared artifact operation/destination grammar to recognize active evidence delivery usingas an artifact.scripts/langchain/pr_verifier.py::_required_evidence_channelsshared review destination vocabulary for PR review comments and inline review comments.scripts/langchain/pr_verifier.py::_required_evidence_channelschecklist noun handling using existing common-proof/polarity vocabulary so Validation output/results retain the named comment destination.tests/scripts/test_pr_verifier_delivery_destinations.pyfor the three original examples and bounded positive/negative/optional/product/literal controls.tests/scripts/test_pr_verifier_prompt_coverage.pywith body present but comments absent/unavailable, and body present but artifacts absent/unavailable.templates/consumer-repo/scripts/langchain/pr_verifier.pywith the source fix..github/sync-manifest.ymland the boundary contract indocs/ops/CONSUMER_REPO_MAINTENANCE.md.Acceptance criteria
python -m pytest tests/scripts/test_pr_verifier_delivery_destinations.py tests/scripts/test_pr_verifier_prompt_coverage.py -qpasses on the fixed bytes; deliberate restoration of each original grammar defect makes its named regression fail, followed by byte-identical restoration and GREEN.Attach evidence as an artifactandUpload the coverage report as an artifactrequire artifacts, and missing/unavailable artifacts block a model PASS even with a present PR body.Evidence must be posted in a PR review commentand the inline-review-comment variant require comments; absent/unavailable comments cannot be masked by a present PR body.- [ ] Validation output in a PR commentrequires comments; negative, optional, quoted-literal and product-capability controls remain nongating unless a separate mandatory delivery exists.templates/consumer-repo/scripts/langchain/pr_verifier.pyare byte-identical for the changed verifier, with manifest/docs coverage and retained RED/GREEN commands/results in the ready implementation PR.Head SHA: e252bbc
Latest Runs: ✅ success — Gate
Required: gate: ✅ success