Skip to content

fix(verifier): retain artifact and review-comment evidence destinations #3835

Open

Description

@stranske

Why

Current delivered Workflows source 49286cc28411f1efb5545866ed0f278e6c680fdb, scripts/langchain/pr_verifier.py in _required_evidence_channels (consumer lines 3886/4037), still loses three mandatory evidence destinations. Actual trip-planner #1885 head b28e40427fe68fb9848d0a988a9149463e10058c findings stranske/trip-planner#1885 (comment) and stranske/trip-planner#1885 (comment) reproduce: Attach evidence as an artifact returns no channel; Evidence must be posted in a PR review comment returns overall instead of comments. Independently reproduced - [ ] Validation output in a PR comment also 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.

Scope

One coherent shared destination-grammar repair, source/template parity, existing sync-manifest description, documentation and direct/destination-specific floor regressions. #3820/#3831 separately own capture/proof partitioning/native capacity; their implementation contract explicitly excludes unrelated grammar work. Keep this repair separate from their active branch and preserve all their commits; rebase/integrate through ordinary exact-head checks if shared files move.

Non-Goals

Do not edit generated consumers or resolve their review threads. Do not waive coverage, change providers/reserves, broaden into unlimited English recognition, alter #3831's capture partitioning, or claim campaign/native-provider acceptance. Scaffold-only or partial completion does not count: all three reproduced wrong-channel cases and destination-specific missing-evidence floors must become falsifiably green.

Tasks

  • Extend scripts/langchain/pr_verifier.py::_required_evidence_channels shared artifact operation/destination grammar to recognize active evidence delivery using as an artifact.
  • Extend scripts/langchain/pr_verifier.py::_required_evidence_channels shared review destination vocabulary for PR review comments and inline review comments.
  • Extend scripts/langchain/pr_verifier.py::_required_evidence_channels checklist noun handling using existing common-proof/polarity vocabulary so Validation output/results retain the named comment destination.
  • Add direct RED/GREEN cases to tests/scripts/test_pr_verifier_delivery_destinations.py for the three original examples and bounded positive/negative/optional/product/literal controls.
  • Add destination-specific coverage-floor regressions to tests/scripts/test_pr_verifier_prompt_coverage.py with body present but comments absent/unavailable, and body present but artifacts absent/unavailable.
  • Synchronize templates/consumer-repo/scripts/langchain/pr_verifier.py with the source fix.
  • Update the existing verifier entry in .github/sync-manifest.yml and the boundary contract in docs/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 -q passes 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 artifact and Upload the coverage report as an artifact require artifacts, and missing/unavailable artifacts block a model PASS even with a present PR body.
  • Evidence must be posted in a PR review comment and the inline-review-comment variant require comments; absent/unavailable comments cannot be masked by a present PR body.
  • - [ ] Validation output in a PR comment requires comments; negative, optional, quoted-literal and product-capability controls remain nongating unless a separate mandatory delivery exists.
  • Source and templates/consumer-repo/scripts/langchain/pr_verifier.py are byte-identical for the changed verifier, with manifest/docs coverage and retained RED/GREEN commands/results in the ready implementation PR.

Implementation Notes

Receiving owner: existing Workflows ordinary auto-pilot source lane, no second writer on #3831's branch. Priority P1, high confidence in authenticated direct reproduction; estimate 1–2 active hours plus regression/CI. Due owner follow-up 2026-10-11T20:25Z or earlier PR birth. Existing campaign worktree evidence: evidence/20261011-checklist-proof.py supports validation-output-noun, active-as-artifact and review-comment-channel; each exits 1 on actual full immutable consumer snapshots. Trip-planner and Travel-Plan-Permission bind identical blob 5a50c9f29e76263a2f38965b8d3d76cb20c85d92, SHA256 892d992e8fe614a05373219eb5aaa9173e5ba8b92105774b62f070e2979264e5, length239799. Original fixed lock/package/Test-evidence findings have separate completed independent assessments, not acceptance of these defects. Postmerge Maint 68 generation/qualification/promotion and Maint 71 reviewer/reconciliation/sealing/merge remain campaign follow-through outside this implementation checklist.

Implementation receipt

Ready implementation PR: #3836 at 85f990cfcac9d44d537d35a4e14eadeda5183a42. Local two-module gate: 92,822 passed; original focused reproduction: 14 failures; fixed focused gate: 28 passes; three independent rule reversals: 3 failures then exact-byte-restored 3 passes each. Commands, raw logs/JUnit, hashes and replay are retained in docs/evidence/issue-3835-evidence-destinations/. The completed checkboxes describe that local implementation contract. Hosted CI/review, post-merge comparison and downstream promotion remain keepalive/closer/campaign-owned; no native-provider or whole-campaign acceptance claim.

Post-push review checkpoint

Two active review findings remain on PR3836: shared noun-form negative governors (No need/No requirement) must remain nongating for the new as-artifact match, and the local replay XML parser needs a hardened-parser or justified-input-boundary disposition. Both exact findings and independent counterexamples were handed to the existing active keepalive worker; a same-PR review-recovery request is queued. The previous local92,822PASS and three-rule proof remain snapshot evidence, not acceptance of the new counterexamples. Hosted/review acceptance is pending.

Activity

  1. agents-workflows-bot commented on Oct 11, 2026

    @agents-workflows-bot
    Contributor

    🤖 Auto-pilot step 1: Starting issue formatting...

    Running formatter inline (not via label trigger).

  2. agents-workflows-bot commented on Oct 11, 2026

    @agents-workflows-bot
    Contributor

    🤖 Auto-pilot step 1: Analyzing issue for improvements...

    Running optimizer inline (not via label trigger).

  3. agents-workflows-bot commented on Oct 11, 2026

    @agents-workflows-bot
    Contributor

    Issue Optimization Suggestions

    Review the suggestions below. If you want the agent to apply them, add the
    agents:apply-suggestions label to this issue.

    Task splitting

    • Extend scripts/langchain/pr_verifier.py::_required_evidence_channels checklist noun handling using existing common-proof/polarity vocabulary so Validation output/results retain the named comment destination. (This task bundles two things: recognizing 'Validation output/results' as an evidence noun and keeping the comment destination through the existing polarity handling. It also adds the guardrail that negative, optional, quoted-literal and product-capability controls stay nongating. Each part can fail on its own.). Suggested split: Add 'validation output' and 'validation results' to the common-proof noun vocabulary used by _required_evidence_channels checklist handling., Update _required_evidence_channels so a checklist item such as 'Validation output in a PR comment' returns the comments channel., Verify that the checklist noun change leaves negative, optional, quoted-literal and product-capability phrasings nongating..
    • Add direct RED/GREEN cases to tests/scripts/test_pr_verifier_delivery_destinations.py for the three original examples and bounded positive/negative/optional/product/literal controls. (This combines three reproduced-defect tests with four categories of control tests in a single task. The RED/GREEN demonstration adds a further step.). Suggested split: Add tests in test_pr_verifier_delivery_destinations.py asserting that 'Attach evidence as an artifact', 'Evidence must be posted in a PR review comment' and '- [ ] Validation output in a PR comment' return the artifacts and comments channels., Add negative, optional, quoted-literal and product-capability control tests to test_pr_verifier_delivery_destinations.py asserting that no channel is required., Write a RED run of the new tests against the unfixed source and record the failing output before applying the fix..
    • Add destination-specific coverage-floor regressions to tests/scripts/test_pr_verifier_prompt_coverage.py with body present but comments absent/unavailable, and body present but artifacts absent/unavailable. (This covers two destinations, each with two states (absent and unavailable), so there are four scenarios. They are easier to review and debug as separate pieces.). Suggested split: Add coverage-floor regression tests in test_pr_verifier_prompt_coverage.py where the PR body is present but comments are absent or unavailable, asserting a model PASS is blocked., Add coverage-floor regression tests in test_pr_verifier_prompt_coverage.py where the PR body is present but artifacts are absent or unavailable, asserting a model PASS is blocked..
    • Update the existing verifier entry in .github/sync-manifest.yml and the boundary contract in docs/ops/CONSUMER_REPO_MAINTENANCE.md. (This task touches two unrelated files with different purposes: a manifest description and a documentation contract.). Suggested split: Update the existing pr_verifier.py entry description in .github/sync-manifest.yml to reflect the expanded destination grammar., Update the boundary contract section in docs/ops/CONSUMER_REPO_MAINTENANCE.md to describe the artifact, review-comment and validation-output destination handling..

    Blocked tasks

    • Acceptance: deliberate restoration of each original grammar defect makes its named regression fail, followed by byte-identical restoration and GREEN. (The agent can edit files and run tests locally. It cannot retry CI pipelines, so any CI-level mutation-and-restore proof is out of reach. Reverting and restoring the defect also carries a risk of non-identical restoration unless it is done carefully. | Have the agent run the mutation check locally with git stash/checkout and record the commands and outputs in the PR description. Leave any CI re-runs to a human.)
    • Acceptance: 'retained RED/GREEN commands/results in the ready implementation PR'. (The agent can write the PR description text but cannot retry CI or guarantee that CI results are retained. Marking the PR 'ready' is a human workflow decision. | Ask the agent to include local RED/GREEN command output in the PR body. A human decides when to mark the PR ready and re-runs CI if needed.)
    • Implementation Notes: rebase/integrate with fix(verifier): derive capture obligation manifest from context coverage #3831's branch through exact-head checks, plus the owner follow-up deadline and the Maint 68/71 campaign follow-through. (Coordinating with another active branch, tracking due dates and running postmerge campaign steps are outside the agent's capabilities. They are also not tasks on the checklist. | Keep these items as human-owned notes. Do not turn them into agent tasks.)
    • Changes under .github/workflows/*.yml if any prove necessary for the sync or CI. (None of the listed tasks require workflow edits, but the sync-manifest change could trigger them. Workflow files are protected. | Limit the change to .github/sync-manifest.yml, which is not a workflow file. Escalate to a human if a workflow edit turns out to be needed.)

    Objective acceptance criteria

    • Negative, optional, quoted-literal and product-capability controls remain nongating unless a separate mandatory delivery exists. ('Controls' and 'bounded' are not enumerated. The exact phrasings are left to the implementer, so the test is unverifiable and could pass with weak controls. | List at least one concrete example per category. For instance: 'Do not attach evidence as an artifact' (negative), 'Optionally upload the report as an artifact' (optional), '"Attach evidence as an artifact"' in quotes (literal), and 'The app can export reports as an artifact' (product). State that each returns an empty required-channel set.)
    • Attach evidence as an artifact and Upload the coverage report as an artifact require artifacts, and missing/unavailable artifacts block a model PASS even with a present PR body. ('Block a model PASS' is only partly specified. It does not say what the floor result or verdict is. | State the expected return value, e.g. '_required_evidence_channels returns a set containing "artifacts", and the coverage floor downgrades the verdict from PASS to the existing non-PASS status/reason when the artifacts source is absent or unavailable.')
    • Source and template pr_verifier.py are byte-identical for the changed verifier, with manifest/docs coverage. ('Manifest/docs coverage' is vague. The byte-identity part is objective. | Replace with 'diff scripts/langchain/pr_verifier.py templates/consumer-repo/scripts/langchain/pr_verifier.py exits 0; .github/sync-manifest.yml verifier entry mentions artifact, review-comment and validation-output destinations; CONSUMER_REPO_MAINTENANCE.md contains the updated boundary contract text.')
    • This is a current deterministic completeness defect... all three reproduced wrong-channel cases ... must become falsifiably green. ('Falsifiably green' is a jargon-heavy phrase, not a pass/fail condition. | Replace with 'each of the three reproduced inputs has a test that fails on the pre-fix code and passes on the fixed code.')
    • Do not broaden into unlimited English recognition. (This non-goal has no measurable boundary. | Limit the grammar change to the specific phrases: 'as an artifact', 'PR review comment', 'inline review comment', and 'Validation output/results'. Any other phrasing is out of scope.)

    Missing sections

    • None

    Formatting issues

    • The 'Original Issue'
      Details block repeats the entire body verbatim. It adds length and risks the copies drifting apart. It should be removed or kept collapsed and clearly marked as a historical copy.
    • The Implementation Notes section is a dense run-on paragraph that mixes ownership, priority, estimate, due date, evidence file names, blob hashes and campaign follow-through. It should be split into bullets or subheadings: Owner/Priority, Evidence, Out-of-scope follow-through.
    • Formatting slips in the notes: 'length239799' is missing a space, and the blob hash and SHA256 are bare in one copy and in backticks in the other.
    • Acceptance criteria are checkboxes but several are compound, bundling code behavior, process steps (restore, rerun) and PR content requirements. Each should be one verifiable statement.
    • Several Tasks use the same long prefix ('Extend scripts/langchain/pr_verifier.py::_required_evidence_channels ...'). A parent heading or a shared preamble would be more readable.
    • The Why section includes long commit SHAs, line numbers and discussion URLs inline. A short 'References' list would be easier to scan.
    • Test and docs tasks have no stated ordering, such as writing RED tests before the grammar fixes, even though the acceptance criteria require a RED then GREEN sequence.

    Notes

    • All required sections (Why, Scope, Non-Goals, Tasks, Acceptance Criteria, Implementation Notes) are present, so none are missing. The issue is technically precise and scoped. Its main weakness for agent use is the heavy process and campaign context mixed into Implementation Notes and some acceptance criteria. The agent cannot retry CI, coordinate with fix(verifier): derive capture obligation manifest from context coverage #3831's branch, or run the Maint 68/71 follow-through. Several tasks bundle multiple code and test changes and would work better split by file and behavior. A good ordering would be: (1) write the RED tests, (2) implement the three grammar fixes, (3) sync the template, (4) update the manifest and docs, (5) capture the local RED/GREEN output for the PR body. Sharpen the control-case criteria with concrete example strings, and replace 'falsifiably green' with explicit fail-before/pass-after tests.
  4. stranske-keepalive commented on Oct 11, 2026

    @stranske-keepalive
    Contributor

    🤖 Auto-pilot step 1: Applying optimization suggestions...

    Running apply inline (not via label trigger).

  5. github-actions commented on Oct 11, 2026

    @github-actions
    Contributor

    ⚠️ Workflow failed

    The agents-issue-optimizer workflow encountered an error.

    Details: View workflow run

  6. github-actions commented on Oct 11, 2026

    @github-actions
    Contributor

    This issue is not yet agent-processable. See docs/AGENT_ISSUE_FORMAT.md.

    • Tasks item must name a concrete file, symbol, path, config key, job, or command: Add 'validation output' (verify: confirm completion in repo).

    Advisory:

    • 17 cited path(s) do not exist yet: evidence/20261011-checklist-proof.py, operation/destination, common-proof/polarity, output/results, RED/GREEN, positive/negative/optional/product/literal, … — expected when a task creates them; check for typos otherwise.

    Dispatched the Agents Issue Optimizer format phase (attempt 1 of 3).

  7. agents-workflows-bot commented on Oct 11, 2026

    @agents-workflows-bot
    Contributor

    🤖 Auto-pilot step 1: Issue prepared! Assigning to agent...

    Adding agent:codex label. The capability check will run automatically.

    ⏳ Agent will create a PR shortly.

  8. 6 remaining items

  9. added
    agent:retryAdd to trigger agent retry after rate limit or pause
    and removed
    agent:codexAgent-created issues from Codex
    on Oct 11, 2026
  10. stranske-keepalive commented on Oct 11, 2026

    @stranske-keepalive
    Contributor

    ⚠️ Auto-pilot paused during formatting.

    Formatter output does not satisfy the canonical issue-format contract after one retry.

    Ownership remains with automation; no human authority boundary was inferred.
    The issue claim was released. Any existing codex/issue-<n> branch is preserved for safe reuse rather than deleted without proof that it has no unique work.

    Auto-pilot counts: dispatched=0 formatted=0 paused=1 handed-to-human=0.

    A bounded automation wakeup will clear agents:auto-pilot-pause and re-dispatch the format step after a 30-minute backoff unless the issue state changes first.

  11. stranske commented on Oct 11, 2026

    @stranske
    OwnerAuthor

    Opener has taken the released #3835 implementation lane on the existing codex/issue-3835 branch lineage, separately from #3831. All three grammar fixes and source/template parity are implemented. The original focused reproduction is RED (14 failures); the fixed focused gate passes all 28 cases. Independently reversing each repaired production rule makes its direct regression plus absent/unavailable destination-floor tests fail (3 failures per rule), and exact byte restoration passes those same tests (3 passes per rule). The complete two-module suite contains 92,822 cases and is running with four local pytest workers. The ready implementation PR will retain raw logs, JUnit, hashes, commands and the replay script. No native-provider or campaign acceptance is inferred from this bounded grammar proof.

  12. stranske commented on Oct 12, 2026

    @stranske
    OwnerAuthor

    Post-merge consumer validation: REJECT remains, not campaign acceptance.

    PR #3836 merged as fe15da5 after all authoritative exact-head checks (hosted Python matrices: 103216 PASS each) and source review gates. Full canary run38188135401 regenerated plan sha256:f01fca89fc41201b2cddca9b26fb5ed98983a5b439ea7f47b275245507beefae. Actual consumer bytes still reproduce two bounded missing destinations:

    1. Travel-Plan-Permission#1647 head1a8142e55e57744fd0e61dc232df260c258c6d9d: _required_evidence_channels('- [ ] PR comment with test evidence') returns empty, and _required_evidence_options returns [set()]. Adding an explicit Add verb returns comments, so the destination-first noun checklist itself remains uncovered. Original18c24879... is RED and regenerated head is still RED. Authenticated full bytes/blob/SHA256/length packet: 20261012-Travel-Plan-Permission-finding-offline.json SHA256 3bcc9e2df7a7bef6a74f6a80bd6816e364a4f9816af857362e78d3420be29181. Reviewer independently confirms chore: sync workflow templates Travel-Plan-Permission#1647 (comment) after Maint71 exact-head request4243886861.
    2. trip-planner#1885 headf49f49d8cf17fbed74a53f7854f6437e423e2409: originating reviewer explicit REJECT chore: sync workflow templates trip-planner#1885 (comment) for bare Evidence must be posted in a review comment returning overall instead of comments. PR-prefixed/inline variants and as-an-artifact examples are GREEN on exact consumer bytes; that does not accept the separate valid bare review-comment counterexample.

    Adopt both substantive findings. Do not resolve generated threads, claim complete grammar acceptance, promote this plan, or waive staging Gate failures. Source owner remains canonical root Workflows#3835, not a new issue/reset. Further source repair must update source/template, finite regression matrix, manifest/docs, then regenerate via Maint68/71 with exact-head review. All six campaign source admissions and 240 reserved minutes are consumed; no live permits remain. Resume engineering only with explicit user-authorized capacity extension, not timed retries or another lane. Existing independent delivery/reviewer work continues without borrowing a source permit.

    Actual postmerge compare run38188175059 is NON_PASS with truncated/omitted proof artifacts and incomplete comment retrieval; workflow SUCCESS is not provider PASS. Retain original source evidence and current consumer REJECT separately. This issue is reopened because delivery acceptance is demonstrably incomplete, not because source merge was undone.

  13. stranske commented on Oct 12, 2026

    @stranske
    OwnerAuthor

    Third current-head canary finding is adopted under this same canonical repair root (no new budget/reset): Portable-Alpha-Extension-Model#2332 at6abdcfa2ac8202a93b36fdecf91fadf44207761f, originating Codex finding stranske/Portable-Alpha-Extension-Model#2332 (comment) . Exact authenticated consumer-byte AST execution confirms _required_evidence_channels('Copy the command output into a PR comment') returns empty and options [set()]; expected comments. Preserve copy/copied/copying delivery aliases with the shared polarity/modality controls in any admitted repair, not a positive regex alone. Full-byte/head/blob/SHA256/length evidence20261012-pae2332-copy-offline.json SHA2561904f8b14a114e17966306da349834201192f9f0ed0adf4562b5df7dcf16b8db.

    All three canaries now have substantive source-owned review blockers. Latest Maint71 run38189073408 adopted the independent artifact proof in its proof-application step: at00:43:20Z logged Resolved proof-bound review thread PRRT_kwDOOzvyds6rQZYh using https://github.com/stranske/Workflows/pull/3836; live readback confirms resolved. Later collection stages reuse the same input and report thread_not_resolvable because it is already resolved; this is not an acceptance failure or permission to retry. Independent assessmentc0d2af31... actual PASS is finding-specific only; trip's bare-review-comment REJECT and original broader comment thread remain open. Do not claim originating acceptance or whole-PR PASS.

    Full-plan f01fca89fc41 is NOT promoted; no fleet authorization or post-promotion Health83 evidence exists. Source owner must repair the coherent three-example boundary matrix through explicit campaign capacity authorization, then exact-head CI/review and Maint68/71 regeneration. All six admissions/240minutes consumed and no live permits. No available safe promotion/seal/merge action remains on the rejected canaries. Preserve weekly service through Luna rather than restarting source engineering on a timer.

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

Metadata

Metadata

Assignees

No one assigned

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions