Skip to content

Post-#401 cleanup: 7 Low findings deferred from PR #477 blind round 2 #483

Description

@michael-wojcik

Context

Consolidated tracking for Low-severity findings from round-2 blind review of PR #477 (#401 teachback gate). Deferred as a batch because none are load-bearing and batching reduces GitHub issue count.

Round-2 reviewers: blind-architect-2, blind-test-engineer-2, blind-backend-coder-2, blind-security-engineer-2.

Deferred items

  • L1: bool-in-int inconsistency at teachback_idle_guard.py:215 — entry["count"] = int(entry.get("count", 0)) + 1 does not guard bool per PR feat(#414): persist source field on session_start events (R2) #416 pattern (not isinstance(x, bool)). Low risk since only this hook writes the field; inconsistent with the rest of the Codify variety-tiered blocking teachback using variety_scorer.py as backbone #401 surface. (blind-backend-coder-2)

  • L2: redundant variety compute at teachback_scan.py:238-244 + 324-328 — _is_carve_out_task computes variety_total for low-variety short-circuit; scan_teachback_state recomputes. Minor hot-path nit. Tighten by returning variety_total from _is_carve_out_task. (blind-backend-coder-2)

  • L3: isinstance guard asymmetry at teachback_gate.py:108 vs :112 — MCP-prefix check guards isinstance(tool_name, str); _BLOCKED_TOOLS membership check does not. Safe behavior (returns False on None) but defensively inconsistent. (blind-backend-coder-2)

  • L4: double fail-open without telemetry at teachback_gate.py:169-173 — validator exceptions swallowed; validators have own try/except; programmer errors silent twice. Add stderr warning before swallow. (blind-backend-coder-2)

  • Python↔markdown constant drift guard — test_shared_teachback_exports.py:53-65 pins Python constants to literals but no test reads docs/architecture/teachback-gate/INTERFACE-CONTRACTS.md:14-16,476 to confirm markdown parity. Silent drift risk. (blind-test-engineer-2)

    • NB: docs/architecture/ is gitignored/local-only per user directive. Only applicable if drift test is written against tracked markdown surfaces (e.g., TERMINOLOGY-LOCK.md equivalents in pact-plugin/).
  • Fail-open except coverage gap at teachback_idle_guard.py:178-179 (OSError in flock) + :348-349 (journal-append Exception). Both covered structurally by outer main() try/except; belt-and-suspenders direct testing missing. (blind-test-engineer-2)

  • Integration seam gap — TestCounterTestByRevertGate items 1+2 cover T1→T2→T4 via monkeypatch, not via real disk-backed task JSON + stdin event. Layers unit-tested; end-to-end gap. (blind-test-engineer-2)

Why deferred

  • Round 2 blind surfaced 1 Blocking (R2-A1 rubber-stamp bypass) + 2 convergent findings (C1 idle_guard mkdir hardening, C2 Unicode normalize bypass) that MUST land before merge. Those are being addressed in cycle-3 remediation.
  • These 7 Lows are defense-in-depth / style / test-quality improvements that don't block merge. Batching them as a cleanup issue keeps cycle-3 scope tight.

Related issues

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions