gmickel/flow-next PR #327 — Coverage integrity & orphan detection — PR #327
gmickel/flow-next · pull request #327 ·
Transcript
PlainEnglish
Let's walk through pull request three twenty seven. This PR fixes two workflow integrity bugs in flow-next: one that blocked draft pull requests at the planning stage, and another that silently orphaned evidence commits when the git history got rewritten.
PlainEnglish
We're tackling two primary goals here. First, we need to expose undeclared coverage as a separate signal from uncovered coverage — they answer different questions. Second, we're fixing the abort condition to check whether any task claims a criterion, not whether any task has completed it. Along the way, we're detecting orphaned evidence commits via a batched reachability walk, rendering a three-state coverage table, and updating the make-pr logic. The public API for evidenced coverage stays untouched, so existing readers see no change.
PlainEnglish
Two distinct problems motivated this PR. On the left, issue three oh one: a fully-planned spec whose tasks were all marked todo reported zero percent coverage, and make-pr aborted telling the user to run the work command — but make-pr itself was blocking that step. The workflow was confusing two questions: is this criterion planned by any task, versus is this criterion done by a completed task. On the right, issue three oh two: after a history rewrite like a rebase or squash-merge, evidence commit SHAs recorded in task files remained in the export, and validate stayed green even though the links were now dead. One reporter measured fifteen out of fifteen orphaned on a spec after a mandatory rebase.
Architecture
Let's look at the first change. In the export command, we're now tracking coverage in two parallel sets. The original covered-rids set still looks only at done tasks — that's our evidenced coverage. The new declared-rids set looks at all tasks, regardless of status — that's our planned coverage. We expose both in the tasks summary payload. The uncovered-rids field stays unchanged, so existing readers see no difference. The new undeclared-rids field tells us which criteria no task ever claimed.
Architecture
The second big change is the new Evidence Reachability class. This classifies recorded evidence tokens into three states: reachable, orphaned, or ignored. The contract is strict — exactly two git spawns per validate invocation, regardless of how many specs or commits you have. One spawn is a batch-check over stdin to see which tokens are present in the git store. The other is a rev-list walk from HEAD that stops early once every candidate is found. If a token is present but not reachable, it's orphaned — that's a commit that survived in the object store but got cut out of the history by a rebase or squash.
Architecture
Here's how validate integrates the classifier. The command primes the reachability checker once, up front, with every token across all specs. That's where the two git spawns happen. Then, for each spec, validate-epic walks the recorded evidence commits and asks the classifier for the state of each token. If a token comes back as orphaned, we emit a warning — but we never rewrite the recorded SHA. A wrong remap is worse than a stale link, so the recorded value is left as-is.
Architecture
The abort condition is now keyed on undeclared R-IDs, not uncovered ones. This catches the one truly unrenderable state: no task ever claimed any criterion. A plan-gate spec — all tasks marked todo, but all criteria declared — is not an abort. It renders with a claimed-not-yet-evidenced marker. This unblocks the planning workflow.
Architecture
The coverage table now renders three distinct states. If a done task satisfies a criterion, you see commit links — that's evidenced. If an open task claims it, you see a hourglass marker — that's claimed but not yet evidenced. If no task claims it at all, you see a warning symbol — that's undeclared. The task column is always derived from the satisfies array, never filtered by status. And if validate found orphaned SHAs, those render as bare code with an annotation, never as commit links.
PlainEnglish
After this PR lands, flow-next will correctly distinguish planned coverage from evidenced coverage. The make-pr skill will open draft PRs at the planning gate instead of blocking them, rendering a three-state table that shows which criteria are evidenced, claimed-but-not-done, and entirely undeclared. The workflow will abort only when no task claims any criterion — the one state where the fix-advice is actionable. Validate will detect evidence commits orphaned by history rewrites and warn, with the recorded SHA left unchanged. And it'll cost exactly two batched git spawns per full validate invocation, regardless of spec or commit count.
CodeQuality
We found three things to keep an eye on. Number one: the batch-check method silently returns an empty dict on any git failure — non-zero return code, OS error, subprocess error. Paired with the default-to-ignored state, this degrades gracefully but offers no user signal that a git failure occurred. Number two: the reachable-oids walk catches the case where stdout is None after a successful Popen — should never happen, but it's caught for safety. And number three: the artifact-supersession rule in the workflow docs states omit the legacy section only when uncovered is empty, but the logic actually requires both uncovered and undeclared to be empty. The prose is explicit if you read carefully, but tightening the phrasing would help.
CodeQuality
Here's the silent-failure case in detail. If the git subprocess returns a non-zero code, or if we hit an OS error or subprocess error, the method returns an empty dict. The comment says this is deliberate — never crashes, never rewrites — but a logs-silent failure could mask a misconfigured repo. The tradeoff is validate stays green; the cost is you might not know git wasn't reachable. Worth noting for debugging.
PlainEnglish
So that's pull request three twenty seven. A focused fix that unblocks the planning workflow and closes the orphaned-evidence gap. The changes are solid, the tests are thorough, and the API stays backward compatible. I'd approve this with a couple of notes: confirm the spawn-budget test runs in CI on all platforms, consider logging a warning when batch-check degrades silently, and tighten the artifact-supersession prose to mention both uncovered and undeclared. Nice work.
How this was made
Lenzon read gmickel/flow-next at pull request #327 and generated this walkthrough automatically. The narration above is the transcript of what it says.
Explain a pull request from your own repo
Point Lenzon at a repo or a pull request and get a narrated walkthrough like this one.
Try it