Skip to content

[Follow-up] Update docs/verification-concerns-1307.md: Ensure (PR #1316) #1322

Description

@stranske

Address Remaining Verification Gaps from PR #1316

Why

PR #1316 addressed issue #1315, but verification returned a CONCERNS verdict due to remaining gaps: high-severity concerns are not fully traceable to concrete resolutions/decisions with repo-local evidence, LLM invocation paths still lack tests that assert config/metadata propagation (and logging on fallback), model classification logic can misclassify names, and some workflows still reference older actions/upload-artifact versions. This follow-up issue closes those gaps with explicit documentation, evidence, tests, and minimal safe refactors.

Source

Tasks

Documentation & Traceability

  • Add resolution links and summaries to each high-severity concern in docs/verification-concerns-1307.md that was resolved by code changes
  • Add explicit DECISIONS.md references to each high-severity concern in docs/verification-concerns-1307.md that was resolved by architectural decision
  • Add per-concern evidence links pointing to repo-local verification artifacts for each high-severity concern in docs/verification-concerns-1307.md
  • Add dated entry to DECISIONS.md for the "missing artifacts" concern including decision statement, rationale, date (ISO format), and evidence link

Re-verification Evidence Artifacts

  • Create docs/reverification/README.md explaining how evidence files are produced and what they contain
  • Create a dated re-verification evidence markdown file under docs/reverification/ with clearly labeled sections for each high-severity concern
  • Update docs/verification-concerns-1307.md to link to the dated re-verification evidence file for each high-severity concern

LLM Invocation Testing

  • Add unit tests in tests/test_pr_verifier.py that mock client.invoke and assert config/metadata propagation for all LLM invocation call sites
  • Add unit tests in tests/test_followup_issue_generator.py that mock client.invoke and assert config/metadata propagation for all LLM invocation call sites
  • Add test in tests/test_followup_issue_generator.py that forces the TypeError path and asserts warning/error log emission with exception info and fallback phrase

Model Classification Refactoring

  • Refactor tools/langchain_client._is_reasoning_model to use a start-anchored regex pattern that matches only o followed by digits
  • Add unit tests in tests/test_langchain_client.py covering both intended model names (o1, o3, o123) and edge cases (o-mini, gpt-4o, foo, O1) for _is_reasoning_model classification

Logging Improvements

  • Update followup_issue_generator.py _invoke_llm to log warning/error when TypeError is raised, including exception text and explicit phrase indicating config/metadata fallback

Deferred Tasks (Requires Human)

  • Manual: Update all .github/workflows/*.yml files to use actions/upload-artifact@v7 instead of older versions (v3/v4/v5/v6) and adjust any related inputs for v7 compatibility

Acceptance Criteria

Documentation Traceability

  • In docs/verification-concerns-1307.md, every high-severity concern entry includes a Resolution: field with at least one GitHub PR URL (containing /pull/) OR commit URL (containing /commit/) plus a non-empty Summary: line
  • In docs/verification-concerns-1307.md, every high-severity concern entry that was resolved by architectural decision includes a Decision: field that references a DECISIONS.md entry by anchor or heading text
  • In docs/verification-concerns-1307.md, every high-severity concern entry contains an Evidence: field with a relative link to a repo-local file path under docs/reverification/
  • DECISIONS.md contains a dated entry for the "missing artifacts" concern with all of:
    • A one-sentence Decision statement
    • A Rationale section with at least one bullet or paragraph
    • A Date: field in ISO format YYYY-MM-DD
    • An Evidence: link to a repo-local file under docs/reverification/

Re-verification Evidence

  • A repo-local re-verification evidence file exists at docs/reverification/<YYYY-MM-DD>-*.md where the filename contains a date prefix in YYYY-MM-DD format
  • docs/reverification/README.md exists and describes how the evidence file is produced or what it contains
  • docs/verification-concerns-1307.md links to the dated docs/reverification evidence file and each high-severity concern's Evidence: link points to a specific section within that file (markdown anchor) or to a per-concern evidence file under docs/reverification/

LLM Invocation Tests - pr_verifier.py

  • tests/test_pr_verifier.py contains at least one test that patches/mocks the underlying LLM client's invoke method used by pr_verifier.py
  • The test asserts that the invocation receives the constructed config/metadata argument (not just that _build_llm_config returns it)
  • The test covers every call site in pr_verifier.py that triggers an LLM invocation

LLM Invocation Tests - followup_issue_generator.py

  • tests/test_followup_issue_generator.py contains at least one test that patches/mocks the underlying LLM client's invoke method used by followup_issue_generator.py
  • The test asserts that _invoke_llm passes the constructed config/metadata argument through to invoke
  • The test covers every call site in followup_issue_generator.py that triggers an LLM invocation
  • tests/test_followup_issue_generator.py includes a test that forces the TypeError path by mocking invoke to raise TypeError when called with config/metadata
  • The TypeError test asserts a warning/error log record is emitted containing:
    • The exception text or repr
    • An explicit phrase indicating config/metadata fallback occurred
  • The TypeError test verifies that a second invocation without config/metadata occurs (fallback behavior preserved)

Model Classification

  • tools/langchain_client._is_reasoning_model uses a regex anchored at the start that matches only model names beginning with o followed immediately by one or more digits (e.g., ^o\d+)
  • tools/langchain_client._is_reasoning_model does not match names like o-mini
  • tests/test_langchain_client.py includes explicit cases asserting _is_reasoning_model returns True for: o1, o3, o123
  • tests/test_langchain_client.py includes explicit cases asserting _is_reasoning_model returns False for: o-mini, gpt-4o, foo, O1

Logging

  • followup_issue_generator.py _invoke_llm logs a warning or error when a TypeError is raised due to passing config/metadata to a client that does not accept it
  • The log message includes both:
    • The exception text (or repr)
    • An explicit phrase indicating config/metadata was not passed due to fallback

Test Suite

  • All newly added tests in tests/test_pr_verifier.py, tests/test_followup_issue_generator.py, and tests/test_langchain_client.py pass individually when run with pytest
  • No new test failures are introduced in existing test files when running the full test suite

Implementation Notes

Documentation traceability

  • In docs/verification-concerns-1307.md, standardize each high-severity concern to include fields like Resolution: (PR/commit link) + Summary: or Decision: (link to DECISIONS.md entry), plus Evidence: pointing to a repo-local docs/ path.
  • Prefer relative links (e.g., docs/reverification/2026-02-07-...md#concern-x) so they work in GitHub UI and remain stable.

Re-verification evidence artifact

  • Add docs/reverification/README.md explaining what is captured (commands run, environment assumptions, how to regenerate).
  • Add at least one dated evidence markdown file whose filename begins with YYYY-MM-DD and contains clearly labeled sections per high-severity concern to support anchored links.

LLM invocation tests

  • Ensure tests validate real invocation paths: patch/mock the actual client.invoke used by each module, call the public/entry methods that trigger LLM calls, and assert the invoke args include the constructed config/metadata.
  • Cover all LLM invocation call sites in pr_verifier.py and followup_issue_generator.py, not only helper builders.
  • For the fallback behavior in followup_issue_generator.py, use caplog (or equivalent) to assert a warning/error log is emitted, and assert the invoke call happens twice (first with config → TypeError, second without config succeeds).

Model classification

  • Update _is_reasoning_model to a start-anchored regex such as r"^o\d+" and add explicit tests including O1 to lock down case-sensitivity expectations.

Workflow changes (manual)

  • Keep workflow edits scoped: only bump actions/upload-artifact to @v7 and adjust inputs solely as required for v7 compatibility.
  • After manual updates, validate via ripgrep: rg "actions/upload-artifact@v[3-6]" .github/workflows/ should return no matches.
Background (previous attempt context)
  • Lack of explicit re-verification evidence for high-severity concerns:

    • Earlier approach referenced workflow artifacts without ensuring repository documentation provided direct evidence.
    • Instead: incorporate embedded test results/artifact files in the repo, or dated repo-local evidence links in docs/verification-concerns-1307.md and DECISIONS.md.
  • Insufficient tests for verifying metadata propagation in LLM invocation:

    • Prior iterations only tested the configuration builder (_build_llm_config) but not the use of this configuration in actual client.invoke calls.
    • Instead: mock/intercept client.invoke and assert constructed config/metadata is passed for each invocation path; include coverage for fallback behavior.

Critical Rules

  1. Do NOT include "Remaining Unchecked Items" or "Iteration Details" sections unless they contain specific, useful failure context
  2. Tasks should be concrete actions, not verification concerns restated
  3. Acceptance criteria must be testable (not "all concerns addressed")
  4. Keep the main body focused - hide background/history in the collapsible section
  5. Do NOT include the entire analysis object - only include specific failure contexts from blockers_to_avoid

Activity

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

Metadata

Metadata

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions