Single-source the shared helpers in the three transcript-walking instruments

[The01Geek]

Dependencies

Blocked by #1899 — that issue fixes a defect living inside two of the helpers this issue moves. Extracting first means fixing the same defect a second time in the extracted copy, and it risks levelling the correct implementation down to match the two broken ones.

Problem Statement

Three scripts read a run's transcript and measure how much context it used. Each carries its own private copy of the same five helpers. The copies drift, and the drift is not hypothetical: it produced a real defect in which two of the three instruments reported an unmeasured turn as a real zero while the third reported it as unknown. The rule of three is reached, so the shared code moves into one place that all three read.

Current Behavior

Five names are defined separately in each of scripts/create_issue_eval.py, scripts/implement-context-eval.py, and scripts/review-context-eval.py: _iter_session_files, _median, _context_tokens, the per-field usage reader, and the UNESTABLISHED sentinel constant. Compared with docstrings and comments stripped, _iter_session_files and _median are functionally identical across all three, differing only in one line wrap.

The three files are 2562, 928, and 554 lines. UNESTABLISHED is additionally defined in scripts/create_issue_benchmark.py, which is a fourth consumer that does not walk transcripts.

• Verified: git grep -ln "^UNESTABLISHED" -- scripts lib returns the three instruments plus scripts/create_issue_benchmark.py. Re-derive with that command.
• Verified: each of the three files defines _iter_session_files, _median, and _context_tokens. Re-derive with git grep -n "def _iter_session_files\|def _median\|def _context_tokens" -- scripts.
• Environment fact: the duplication happens regardless of environment. It is the shape of the three files.

Desired Behavior

Each of the five shared names has exactly one definition, in a module the three instruments import. All three instruments keep the strict behavior for an unmeasured turn, an empty population, and a non-finite number that issue #1899 establishes. Each instrument's own focused test keeps passing with its assertions about instrument output unchanged.

User Impact

A maintainer fixing one of these helpers fixes it once. A defect found in one instrument cannot silently survive in the other two.

Technical Context

Scope note: The files and details below are the known starting points, not the full list. Before implementing, trace the change through the codebase to find every affected call site, consumer, and layer — this issue maps the work, it does not bound it.

• Relevant Classes/Files — scripts/create_issue_eval.py, scripts/implement-context-eval.py, scripts/review-context-eval.py each define the five shared names. scripts/create_issue_benchmark.py defines the sentinel constant separately. scripts/create-issue-context-eval.py is a small compatibility shim that loads create_issue_eval through importlib; it is not a fourth instrument. lib/test/modules/coverage-map.json carries a row for each of the three instruments.
• Architecture Alignment — this repository's stated default is that a new fact lives in exactly one place and every other mention points at it. Physical copies held under a coupled-invariant test are recorded as backlog to collapse, not as a target state. So the shared code is extracted into a module, and no coupled-invariant test pinning three copies identical is written.
• Dependencies — the Python standard library only. The extracted module imports nothing outside it, so the instruments keep running against the tools lib/preflight.sh guarantees.
• Data/Schema Considerations — none. No stored record changes shape.
• Cross-layer Impact — the three instrument scripts, their three focused tests, and the coverage map that routes to those tests.
• Provenance: recorded as section C3 of umbrella issue Umbrella: findings and learnings from the #1850 / #1852 / #1443 shepherding session — to be split before implementation #1890.
• The extraction set named in that umbrella included _force_utf8_streams, and this issue drops it. That helper has 63 separate definitions across scripts/ and lib/, so it is a repository-wide idiom rather than a three-instrument duplication, and folding it in turns a bounded refactor into an unbounded one. — assumption, confirm before implementing

Acceptance Criteria

• Each of these five names has exactly one definition reachable by all three instruments: _iter_session_files, _median, _context_tokens, the per-field usage reader, and the UNESTABLISHED sentinel. That list is complete for this change.
• After the change, all three instruments return a non-numeric unestablished result for an empty usage object, raise on an empty median population, and return a non-numeric unestablished result for a non-finite token count. No instrument returns zero for any of the three.
• lib/test/test_create_issue_context_eval.py, lib/test/test_implement_context_eval.py, and lib/test/test_review_context_eval.py all pass, with their assertions about instrument output unchanged by this work.
• lib/test/modules/coverage-map.json names a covering test for the extracted module.
• _force_utf8_streams keeps its per-file definitions and is not moved into the extracted module.

Implementation Notes

• Approach — create one shared module under scripts/ holding the five names, and have each of the three instruments import from it instead of defining its own copy. Take the post-Report an unmeasured turn as unestablished in the create-issue and implement context instruments #1899 implementation as the single definition, which is the strict one. Leave scripts/create_issue_benchmark.py's own sentinel constant alone unless importing the shared one is a clean drop-in.
• Relevant files — this touches scripts/create_issue_eval.py, scripts/implement-context-eval.py, scripts/review-context-eval.py, a new shared module beside them, and lib/test/modules/coverage-map.json. It plausibly touches scripts/create_issue_benchmark.py and scripts/create-issue-context-eval.py, the importlib shim, whose loading path must keep working.
• Code Patterns — the repository already ships shared Python helpers under scripts/ that other scripts import directly, so no new packaging pattern is needed. Every new Python file carries the standard licence header.
• Testing Strategy — the three existing focused tests are the primary evidence, and they run unchanged. Add one test on the extracted module itself covering the three strict behaviors: an empty usage object, an empty median population, and a non-finite token count. Add one test that loads scripts/create-issue-context-eval.py's shim path and asserts it still resolves, since that indirection is the one call site an import change can quietly break.
• Documentation Needed — none. The three instruments are maintainer tools documented in their own module docstrings, which the change updates in place.
• Potential Gotchas — the strict implementation is the one to keep. Taking the shortest copy, the most-used copy, and equally the first copy read will level the correct behavior down to the broken one, which is the exact defect Report an unmeasured turn as unestablished in the create-issue and implement context instruments #1899 exists to remove. The importlib shim at scripts/create-issue-context-eval.py loads a sibling by file path rather than by module name, so an import added to the loaded file must resolve under that loading style.

[The01Geek]

/prflow:implement

[github-actions]

PRFlow Workpad — Issue #1900

Status: 🎉 Complete
Branch: issue-1900-single-source-the-shared-helpers-in-the-three
Run: View run
PR: #1926
Last updated: 2026-08-25 01:46 UTC

Progress

• Setup — branch & workpad
  • prompt extension resolved: implement
  • 23:16:01 — /prflow:implement run started
  • 23:16:44 — Claude job setup complete; invoking agent
  • 23:18:50 — agent entered Phase 1 setup; workpad triage passed
  • 23:19:06 — agent initialized; Phase 1 workpad hydrated
  • 23:19:30 — resume-kind: fresh
  • 23:19:30 — classification: non-bug — Deduplication/refactor of maintainer transcript-walking tooling; no malfunction described and no bug label
  • 23:19:48 — Phase 1.3.5: early dependency preflight passed (PROCEED; Report an unmeasured turn as unestablished in the create-issue and implement context instruments #1899 landed)
  • 23:21:58 — issue-claim audit (projection): every Desired Behavior obligation is represented in the resolved acceptance criteria - pass complete
  • 23:22:02 — issue-claim audit (negative-scope): issue excluded '_force_utf8_streams (63 repo-wide definitions) from the extracted module' and 'no coupled-invariant test pinning three copies identical'; trace confirms no impact - coverage-map.json currently carries one row per instrument with no shared-module row, and no such coupled-invariant test exists
  • 23:22:04 — issue-claim audit (policy): no policy-referencing AC claims found - pass complete
  • 23:22:06 — issue-claim audit (execution-capability): no workflow-resident acceptance criteria found - pass complete
  • 23:22:08 — issue-claim audit (verified-premise): re-checked 2 Verified: bullet(s) at HEAD - 0 hold (both unestablished by the automated grader: bullet 1 cites a path with no quotation, bullet 2 is a command handle never auto-executed), 0 unestablished... wait
  • 23:22:14 — issue-claim audit (verified-premise) correction: re-checked 2 Verified: bullet(s) at HEAD via check-verified-premises.py - 0 hold, 0 refuted, 2 unestablished by the automated grader (bullet 1: cited path carries no quotation to re-derive from; bullet 2: command handle reported but not auto-executed); independently manually re-ran both cited git grep commands and both bullets' claims hold exactly at HEAD - no premise refuted
• Implement
  • code + sweeps
  • 23:23:07 — entered Phase 2
  • 23:29:11 — Path A (simple): mechanical extraction, clear solution in issue. 2.2.4 reuse: single-sourcing reuses existing byte-identical create_issue_eval.py bodies (the fullest-documented copy); no new logic authored. Altitude: shared module beside the instruments is the right depth. Import mechanism reuses the sys.path.insert + noqa:E402 convention from scripts/check-verified-premises.py (proven under importlib-by-path loading). RESIDENCY_KEYS moves as the dependency of the shared _context_tokens (single-sourced, not duplicated); imported only by create_issue_eval.py which uses it in _residency_spend.
  • 23:29:31 — Criterion-lifecycle accounting (all Testable, RED/GREEN): AC1 five-names-one-definition → identity assertions (each instrument's name IS shared object) in test_context_eval_shared.py; AC2 strict behaviors/no-zero → direct shared-module tests (empty usage→None, empty median→raises, non-finite→None) + existing focused tests; AC3 three focused tests pass unchanged → run them; AC4 coverage-map names covering test → new test asserts the coverage-map row present + coverage_map_guard; AC5 _force_utf8_streams keeps per-file defs → new test asserts each instrument still defines it and shared module does not. §2.2.7 coupled sites (from grep): 3 instruments (import), 3 focused tests (attribute access), coverage-map.json, run.sh block.
  • 23:29:31 — test-first: lib/test/test_context_eval_shared.py written first, run RED (ImportError: no scripts/context_eval_shared.py) before creating the module, then GREEN after extraction.
  • 23:41:13 — Sweep selection (diff shape: deletes helper defs from 3 instruments; relocates 5 symbols + RESIDENCY_KEYS into new scripts/context_eval_shared.py; adds import at 3 peer sites; adds 2 members to _ALLOWED_REFERENCES in 2 tests; adds coverage-map row + run.sh block + config allowed_tools grant). Ran: 2.3.0 changed-contract/relocation — grep for surviving citations of the five names at the old location / imports from the instruments returned 0 matches; consumers use module-attribute access which still resolves via re-export. 2.3.0a peer-checkpoint — sys.path.insert+import present at all 3 instruments; _ALLOWED_REFERENCES updated in BOTH review and implement tests. 2.3.0b enum-enumeration — the 2 new files added to _ALLOWED_REFERENCES in both tests, coverage-map row added, config grant added. 2.3.1 orphaned-setup — removed now-unused import math from all 3 instruments and local RESIDENCY_KEYS from implement/review; ruff clean confirms no orphans. 2.3.2 stranded-dependents — no external caller imports the moved symbols (grep 0). 2.3.3 convention — SPDX headers on new .py, ruff clean, stdlib-only, portable. 2.3.4 boundary-assumption — import mechanism verified by RED->GREEN and by the shim resolution check (shim loads; its _context_tokens IS the shared object). 2.3.4a self-authored-claim — docstring/comment claims verified against code via the identity test; import comments name the wrong change (removing the sys.path insert). 2.3.4b coverage-claim — stale-prose-lint clean after scoping two universals (Every helper -> the three residency/median helpers; any entry -> an entry). 2.3.5 simplification — the change IS the dedup; no added complexity. 2.3.6 error-handling — no new error sites; moved code retains its tally+breadcrumb walk discipline.
  • 23:41:13 — test-first GREEN: new test 10 pass; three focused tests pass (review 38, implement 66, create-issue 169) with instrument-output assertions unchanged; ruff/coverage-map guard/regenerate-artifacts all clean.
• Review
  • prompt extension resolved: review engine
  • prompt extension resolved: fix loop
  • prompt extension resolved: code-review reception
  • Classify diff (Phase 0.5)
  • Generate verification checklist (Phase 1)
  • Verify checklist (Phase 2)
  • Review agents (Phase 3)
  • Aggregate & verdict (Phase 4)
  • Run complete — everything this run owed
  • /simplify
  • review-and-fix
  • acceptance-criteria gate
  • 23:48:26 — /simplify: reuse/efficiency clean; applied one finding (trimmed a redundant RESIDENCY_KEYS sentence in the shared module docstring + fixed a mid-word wrap). Skipped 2 with AC rationale: (a) moving _median_or_unestablished/_max_or_unestablished into the shared module — would violate AC1's 'That list is complete for this change' (exactly the five named helpers), so out of scope; (b) dropping the _usage_value re-export from implement/review — AC1 requires the five names 'reachable by all three instruments', and the single-source identity test asserts all five per instrument, so the re-export is intentional (noqa:F401).
  • 23:48:48 — Phase 3.3: review-and-fix invoked in PR mode with draft PR Single-source the shared helpers in the three transcript-walking instruments #1926 (bare token) + --push-each-iteration --issue 1900; pre-loop iter-*.json baseline empty (fresh run)
  • 23:48:48 — mid-phase resume point recorded
  • 00:08:46 — review coverage recorded (coverage=full, dispatch=attempted, roster=complete, checklist=complete)
  • 00:09:06 — Phase 3.3 review-and-fix (driven inline, PR mode Single-source the shared helpers in the three transcript-walking instruments #1926): iter-1 engine (checklist-generator + code-reviewer/silent-failure-hunter/comment-analyzer/pr-test-analyzer/requesting-code-review) = APPROVE with 5 Suggestion findings; 4 applied (run.sh comment cap, drop brittle source-substring pin, add RESIDENCY_KEYS identity assertion, add direct _iter_session_files behavioral+symlink-escape tests), 1 pushed back (blanket noqa:F401 is intentional for the re-export identity contract). Fix-delta gate: clean. engine_self_modifying early shadow (5 blinded reviewers, full expected roster): APPROVE, no new findings, coverage=full over the final tree HEAD 68429bb. Loop verdict: CLEAN-FULL / APPROVE. Extension state: review-and-fix + receiving-code-review loaded content-present via the ladder in the loop turn; review-engine extension loaded when the engine executed its phases (reached by file read within this turn, no separate Skill return to observe).
  • 00:13:31 — verified (two-verifier reconciled satisfied); evidence and dispositions in this run's AC dispositions note
• Documentation
  • 00:18:02 — Phase 4.1: no documentation changes to commit (docs subagent ran clean / made no changes; issue Documentation Needed = none; changeset covers release notes)
  • 00:20:32 — 4.2 claim audit (behavioral): all body claims traced to the shipped code and accurate; no rewrite needed
  • 00:20:32 — 4.2 claim audit (verification): every Test Plan row bound to a named test in the diff (test_context_eval_shared.py 13 methods + the three pre-existing focused tests); focused-test row phrased 'pass unchanged' rather than transcribing tallies; no unsupported coverage claim
  • 00:20:32 — 4.2 claim audit (artifact-existence): one artifact claim (the changeset), present in the diff; body asserts no follow-up issue (none deferred), no dangling identifier
  • 00:20:32 — 4.2: PR body updated from placeholder (confirmed); pr-description extension content-present and honored; no code-level fix made
  • 00:20:53 — checkpoint 4: observed token UP_TO_DATE — clean, proceeding to the publish decision
  • 00:23:53 — batched-regeneration: run (--with-floors, all rows clean incl. exact-module-floors; no drift)
  • 01:22:20 — trigger accepted; existing workpad adopted
  • 01:23:01 — Claude job setup complete; invoking agent
  • 01:26:16 — Phase 4.3 (resume): checked out feature branch on fresh runner (was on main); checkpoint 4 observed UPDATED 4 — merged origin/main and pushed (new HEAD 708ad9e). Proceeding to whole-suite completion-evidence flight over the merged tree.
  • 01:29:12 — batched-regeneration: run (--with-floors, all rows clean incl. exact-module-floors after the base merge; no drift)
  • 01:46:28 — completion verification recorded (flight be567d4be836…, validated)
• PR marked ready
  • 01:46:50 — /prflow:implement run finished, PR published (gh pr ready): Single-source the shared helpers in the three transcript-walking instruments #1926

Plan

• Create scripts/context_eval_shared.py (stdlib-only, SPDX header) holding the single definition of the five shared names — _iter_session_files, _median, _context_tokens, _usage_value (per-field usage reader), UNESTABLISHED — taking the strict post-Report an unmeasured turn as unestablished in the create-issue and implement context instruments #1899 bodies (reuse: the existing byte-identical scripts/create_issue_eval.py definitions, the fullest-documented copy). RESIDENCY_KEYS moves too as the dependency _context_tokens ranges over.
• Edit scripts/create_issue_eval.py: delete its local _median/_usage_value/_context_tokens/_iter_session_files/UNESTABLISHED/RESIDENCY_KEYS; import them from context_eval_shared via the proven sys.path.insert(0, dirname(abspath(__file__))) + from … import (…) # noqa: E402,F401 convention (matches scripts/check-verified-premises.py). Keep _usage_field/_residency_spend/_median_or_unestablished/_sum_or_unestablished/_is_numeric local (reference the imported names).
• Edit scripts/implement-context-eval.py: same removal + import (no RESIDENCY_KEYS, unused there). Keep _median_or_unestablished/_sum_or_unestablished/_gap_median local.
• Edit scripts/review-context-eval.py: same removal + import (no RESIDENCY_KEYS).
• Add lib/test/test_context_eval_shared.py: (a) the three strict behaviors on the shared module directly — empty usage → None, empty median population → raises, non-finite token count → None, plus no-zero assertions; (b) load all three instruments + the create-issue-context-eval.py shim via importlib and assert each re-exports the SAME (identity) shared objects, proving AC1 single-sourcing and that the shim still resolves.
• Add a coverage-map.json row naming lib/test/test_context_eval_shared.py as the covering test for scripts/context_eval_shared.py (AC4).
• Register the new test in lib/test/run.sh as a serial block modeled on the three existing context-eval blocks.
• Add .changeset/issue-1900-*.md (patch) after the draft PR exists (Phase 3).
• Run the three existing focused tests + the new one + ruff; confirm all green with the existing assertions unchanged (AC3).

Acceptance Criteria

• Each of these five names has exactly one definition reachable by all three instruments: _iter_session_files, _median, _context_tokens, the per-field usage reader, and the UNESTABLISHED sentinel. That list is complete for this change.
• After the change, all three instruments return a non-numeric unestablished result for an empty usage object, raise on an empty median population, and return a non-numeric unestablished result for a non-finite token count. No instrument returns zero for any of the three.
• lib/test/test_create_issue_context_eval.py, lib/test/test_implement_context_eval.py, and lib/test/test_review_context_eval.py all pass, with their assertions about instrument output unchanged by this work.
• lib/test/modules/coverage-map.json names a covering test for the extracted module.
• _force_utf8_streams keeps its per-file definitions and is not moved into the extracted module.

Devflow Reflection

Devflow Reflection (click to expand)

ℹ️ Notes

• ℹ️ skill-directory anchor resolved from the runner-reported base directory
• ℹ️ resume-precheck: both open-PR queries ran and returned none for this issue; workpad Branch line absent; no prior attempt to adopt
• 📝 Issue accuracy: issue-claim audit (count): claimed line counts '2562, 928, 554' for create_issue_eval.py/implement-context-eval.py/review-context-eval.py, verified '2642, 944, 554' at HEAD - using the verified counts; the '63 definitions of _force_utf8_streams' claim was verified exactly (git grep -c/-l both return 63)
• ℹ️ Phase 3.4 acceptance-criteria gate — two-verifier reconciliation (all satisfied, no undischarged slots). Flight fc23adde…, checkout_verified against HEAD 68429bb, suite_summary exit 0. - AC1 (five names one definition reachable by all three): satisfied (both). Evidence: context_eval_shared.py defines the five once; each instrument imports them, no surviving local def; identity test passes. Evidence slots: type-decided=yes, command-run=no (source+identity test), single-flight=no, evidence-recorded=yes. Claim slots: claim-traced=yes, command-source-read=no, evidence-recorded=yes. - AC2 (strict behaviors, never zero): satisfied (both). Evidence: test_context_eval_shared.py UnmeasuredTurnContract passed in-env (13 OK); source confirms None/raise never 0. Evidence slots all yes (command-run yes, single-flight yes). Claim slots yes/no/yes. - AC3 (three focused tests pass, assertions unchanged): satisfied (both). Evidence: create-issue 169 OK, implement 66 OK, review 38 OK in-env under one flight; only NoAutoInvocation allowlist adds, no output-assertion change. Evidence slots all yes. Claim slots: claim-traced=yes, command-source-read=yes, evidence-recorded=yes. - AC4 (coverage-map names covering test): satisfied (both). Evidence: coverage-map row -> lib/test/test_context_eval_shared.py; coverage_map_guard.py exit 0. Evidence slots all yes. Claim slots yes/no/yes. - AC5 (_force_utf8_streams kept per-file, not moved): satisfied (both). Evidence: defined in each instrument (create 2426, implement 802, review 424); absent from shared module. Evidence slots yes/no/no/yes. Claim slots yes/no/yes. undischarged_slots: none on any criterion. all_satisfied: true; blocking: none.
• ℹ️ Phase 4.3: final-tree whole-suite completion evidence — lib/test/run-parallel.sh over merged HEAD 708ad9e aggregate CLEAN (20899 passed, 0 failed, 0 skipped, 5/5 shards); flight satisfies_verification=true, checkout_verified=true.

[prflow-implementer]

DevFlow stall backstop — this cloud run ended while the workpad Status was still in-progress (interim 🚀 Documenting). Auto-resume attempt 1 of 2:

Resume note: invoke bundled helpers as .prflow/vendor/prflow/scripts/… (and .prflow/vendor/prflow/lib/…) with that path as the leading token — never an absolute path, never repo-root scripts/…, never behind a VAR= prefix or bash <path> wrapper (those forms are silently denied).

Headless note: this is a headless run — ending the turn ends the process, with no re-invocation. Never end the turn while any dispatched agent has not returned; poll to keep the turn alive; treat ScheduleWakeup and any future task-notification as unavailable.

/prflow:implement 1900