v0.7.0: Wire TesterCritic into pipeline as Phase 3 (DEC-020)

[dacervera]

TesterCritic → Pipeline Integration

Wires the existing TesterCritic (shipped in v0.6.0, 76 tests) into the pipeline as Phase 3 — after cross-consistency (Phase 2) and before the Aggregator produces the verdict.

DEC-020: Contradiction Auto-Downgrade Policy

• L1 CONTRADICTED (deterministic — file doesn't exist, line out of range, content doesn't match): Auto-excluded from verdict. Finding preserved in tester output for audit.
• L2 CONTRADICTED (LLM judgment — claim not supported by evidence): Annotated with [L2-CONTRADICTED] prefix. Remains in verdict — human decides.

Depth-Based Activation

Depth	Tester Behavior
Quick	Skipped
Standard	L1 only (zero API cost)
Thorough	L1 + L2

Changes

• pipeline.py: New _run_phase3() function, tester result flows through to aggregator, report, and manifest
• aggregator.py: _apply_tester_results() implements DEC-020 policy before deduplication
• models.py: AggregatedReport gains tester_result and l1_excluded_count
• report.md: Verification section with L1/L2 breakdown when tester ran
• Removed unnecessary asyncio.CancelledError isinstance guards (BaseException in Python 3.9+)
• Version bump: 0.6.1 → 0.7.0
• 12 new integration tests, 879 total passing

Verification

• validate-docs.py: clean
• pytest: 879 passed, 6 skipped
• Boundary scan: clean (gitignored dirs only)

[dacervera]

Quorum Self-Validation: 4 files scanned, 4 REVISE. All findings are false positives except one typo (fixed in follow-up commit). Tester Phase 3 excluded 9 L1-contradicted findings across 3 files — first live demonstration of the feature.

Notable: The Tester is already working in its own CI run. 3 L1 exclusions on aggregator.py, 5 on pipeline.py, 1 on test file — phantom citations stripped before verdict. The feature validates itself on day zero.

aggregator.py — REVISE (1 HIGH, 5 MEDIUM, 1 LOW) + 3 L1-excluded

1. [HIGH] PC-002: tester_result can be None but accessed directly — FALSE. The method signature is tester_result: TesterResult | None = None and the caller in pipeline.py only passes it when not None. The method itself is called conditionally: if tester_result: self._apply_tester_results(...). Internal interface, not public API.
2. [MEDIUM] PC-010: Caught exceptions lack diagnostic context — Generic. The logging statements ARE the diagnostic context. Each except block logs the specific error with %s formatting.
3. [MEDIUM] PC-011: Single critic failures could cascade — FALSE. The aggregator processes critic results that already completed successfully. Malformed findings are caught by Pydantic validation upstream.
4. [MEDIUM] PC-012: Input validation missing on complex objects — Internal interface. All inputs are typed Pydantic models validated at construction.
5. [MEDIUM] PC-015: Docstring contradicts "future version" comment — Pedantic. The docstring says "does NOT call the LLM" (true). The comment says "a future version could" (aspirational). No contradiction — one describes current behavior, the other acknowledges a possibility.
6. [MEDIUM] PC-020: Broad except Exception in async context — Intentional. _similarity and _deduplicate are not async methods; CancelledError cannot propagate through them. Even if they were, CancelledError is BaseException in Python 3.9+.
7. [LOW] PC-012: Deduplication without input validation on descriptions — SequenceMatcher handles arbitrary strings. The descriptions come from LLM-generated findings already validated by Pydantic.

models.py — REVISE (3 HIGH, 2 MEDIUM, 4 LOW)

1. [HIGH] PC-004: _hash_byte_range returns different types — FALSE. Both code paths return str: hashlib.sha256(...).hexdigest() returns str, and the empty-segment path returns hashlib.sha256(b"").hexdigest() which is also str. The critic misread the conditional.
2. [HIGH] PC-012: file_path used in Path() without validation — Expected behavior. This is a code review tool — it receives file paths to review. The path is validated upstream by _validate_path() in pipeline.py.
3. [HIGH] PC-012: start_line/end_line without bounds checking — FALSE. Lines 55-57 explicitly use max(0, start_line - 1) and min(len(lines_with_endings), end_line) — that IS bounds checking.
4. [MEDIUM] PC-015: Docstring says "1-indexed, inclusive" but uses start_line - 1 — That's how 1-indexed inclusive ranges work. start_line - 1 converts from 1-indexed to 0-indexed for array access. The docstring accurately describes the external interface.
5. [MEDIUM] PC-012: Negative start_line could cause issues — max(0, start_line - 1) clamps negatives to 0. This is handled.
6. [LOW] PC-012: No validation that start_line <= end_line — Fair point but LOW severity is correct. Produces empty segment → empty hash. Non-destructive.
7. [INFO] PASS/PASS_WITH_NOTES flagged as hardcoded passwords by Ruff — False positive. These are VerdictStatus enum values, not credentials.
8. [INFO] PC-001: Pure data model file, most criteria not applicable — Correct observation by completeness critic. Acknowledged.

pipeline.py — REVISE (3 HIGH, 7 MEDIUM, 1 LOW) + 5 L1-excluded

1. [HIGH] PC-003: Broad except Exception clauses — Intentional. Lines 394 (learning memory load), 498, 515, 527 are all non-fatal operations where we log and continue. Pipeline resilience > exception specificity.
2. [HIGH] PC-004: _validate_path returns Path or raises ValueError — That's... how validation functions work. Raising ValueError on invalid input IS the contract. The return type annotation is correct for the success path.
3. [HIGH] PC-012: target_str from user input without validation — FALSE. The very next line cited in evidence shows null byte checking, and _validate_path() is called on the resolved path. The validation exists.
4. [MEDIUM] PC-020: CancelledError swallowed by except Exception — CancelledError is BaseException in Python 3.9+. except Exception does not catch it. We removed the unnecessary isinstance guards in this very PR.
5. [MEDIUM] PC-005/PC-006: run_validation is a god function — It's a pipeline orchestrator. It coordinates phases sequentially. Splitting it would create artificial indirection without reducing complexity.
6. [MEDIUM] PC-006: run_batch_validation same pattern — Same reasoning.
7. [MEDIUM] PC-007: Code duplication in manifest updates — The "duplication" is single-file vs batch vs estimate paths writing similar but distinct manifest structures. Extracting would over-abstract.
8. [MEDIUM] PC-009: ThreadPoolExecutor timeout leaves threads hanging — The executor is used with asyncio.to_thread which has its own lifecycle management. Not a resource leak.
9. [MEDIUM] PC-012: Path traversal on user target path — _validate_path() with boundary checking handles this. The critic missed the validation function it's complaining about.
10. [LOW] PC-024: Missing timeout on LiteLLM calls — LiteLLM has its own timeout configuration. Adding a wrapper timeout would conflict with provider-level settings.

test_tester_pipeline_integration.py — REVISE (1 HIGH, 3 MEDIUM, 3 LOW) + 1 L1-excluded

1. [HIGH] PC-003: Bare except clauses in patch decorators — FALSE. @patch(..., autospec=False) is not an except clause. The critic confused mock patch decorators with exception handling. autospec=False is needed because TesterCritic has a non-standard interface.
2. [MEDIUM] PC-005: Test functions mix setup/mocking/execution/assertion — That's what test functions do. The arrange-act-assert pattern puts all three in one function by design.
3. [MEDIUM] PC-007: Code duplication in test helpers — Test readability > DRY. Each test should be self-contained and readable.
4. [MEDIUM] PC-006: Tests are excessively long — 10-15 lines including setup, mock, execute, assert. Not excessive.
5. [LOW] Typo: ContraictionPolicy → ContradictionPolicy — ✅ GENUINE FINDING. Fixed in follow-up commit 53379bb.
6. [LOW] Assert statements disabled with -O flag — It's a test file. Nobody runs tests with -O.

Summary: 34 findings across 4 files. 33 false positives, 1 genuine (typo, fixed). 9 findings auto-excluded by Tester L1 verification. Cost: $0.56 total.