refactor(ready): absorb annotation->readiness interpretation into ClassifyReadiness (links-va-001-readiness-f90.1)#202
Merged
brandon-fryslie merged 1 commit intoJun 10, 2026
Conversation
…ssifyReadiness (links-va-001-readiness-f90.1) Annotations are neutral facts; readiness is an interpretation. That interpretation previously lived at six-plus sites that each re-walked annotation.Kind: isReadyBlocked, printBlockedSummary's HasAny loop over readyBlockingKinds, printRankInversions' Kind == check, the Orphaned HasAny checks in inProgressSuffix and runOrphaned, and the raw OpenDependency walks in dependencyIDs/buildUnblocksMap and backlog's nonDependencyBlockingReasons. ClassifyReadiness (internal/cli/readiness.go) is now the single annotation->policy enforcer producing a sealed IssueReadiness whose fields mirror the three interpretation families: membership (BlockingReasons), staleness (IsOrphaned), rank hygiene (RankInversions). Every consumer reads the typed result; the per-site Kind walks are deleted. IsReady() is len(blocking)==0 by construction, so emptiness of the raw annotation slice can never become a readiness proxy. FocusPath deliberately stays OUTSIDE the classifier: it is an ordering interpretation (sortByFocusPath), not a membership one, and routing it through readiness would re-tangle the two concerns the focus design keeps apart. Recorded at both sites. Deviations from the epic sketch, per [LAW:types-are-the-program]: no issueID field (no consumer reads it; rows carry their own identity), orphaned bool instead of *InProgressAge (display derives age from UpdatedAt, not from annotations), and no BlockingSummary() string (the two display surfaces want different formats; formatting stays at the consumers, the classifier owns only which facts block). Verified: contract test covers every annotation kind plus the empty and composite cases; go test ./... clean; old-vs-new binary output on the live store is byte-identical for ready/backlog/queue.
There was a problem hiding this comment.
Z.ai Coding Agent Review
Clean refactoring that consolidates scattered annotation→readiness interpretation into a single ClassifyReadiness classifier producing a typed IssueReadiness struct. No must-change items.
The PR correctly:
- Removes
readyBlockingKinds,isReadyBlocked,dependencyIDsand replaces them with one source of truth (blockingKinds+ClassifyReadiness). - Inverts all test assertions from
isReadyBlocked(true=blocked) toIsReady()(true=ready) semantics. - Leaves
annotation.HasAnyfor FocusPath in place with explicit documentation — FocusPath is ordering, not readiness.
Pre-existing note: ClassifyReadiness is called redundantly in several display paths (printInlineDeps, inProgressSuffix, printRankInversions) and inside the sort comparator in sortByBlockingAnnotations (O(n log n) calls). The function is pure and lightweight, so this isn't a LAW violation, but if annotation lists ever grow large, callers could compute readiness once and thread it through.
✅ Approved
brandon-fryslie
added a commit
that referenced
this pull request
Jun 10, 2026
…recedence.First enforcer (links-va-001-precedence-jg6.1) (#207) Three surviving helper variants (templates.firstNonEmpty, cli.firstNonEmptySyncBranch, cli.firstNonEmptySyncRemote — the latter two byte-identical) collapse into internal/precedence.First. The epic's proposed trim bool is deliberately not implemented: every sync candidate is already trimmed where it is produced, and template content must never be trimmed — so trim is a property of the value at its production boundary (the PathSpec precedent), not a mode of resolution. [LAW:no-mode-explosion] [LAW:single-enforcer] Helper-specific sync test becomes the precedence package test matrix. The other three filed sites (config.first, workspace.firstNonEmptyTrimmed, error_output.firstNonEmptyString) were already absorbed by #202-#204.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Implements links-va-001-readiness-f90.1 (epic links-va-001-readiness-f90, F-2 Tier 1).
What
Annotations are neutral facts; readiness is an interpretation a consumer applies over them. That interpretation previously lived at six-plus sites that each re-walked
annotation.Kind. This PR addsinternal/cli/readiness.gowithClassifyReadinessas the single annotation→policy enforcer producing a sealedIssueReadinesswhose fields mirror the three interpretation families:BlockingReasons()/IsReady()(len(blocking)==0 by construction; raw-slice emptiness can never become a readiness proxy)IsOrphaned()RankInversions()Sites migrated (old per-site Kind walks deleted)
isReadyBlocked+readyBlockingKinds(deleted; kinds list now private to the classifier)printBlockedSummaryHasAny/seen/count loop → consumes[]IssueReadiness+blockingKindCountsprintRankInversionsKind == checkinProgressSuffixandrunOrphaneddependencyIDs/buildUnblocksMapraw OpenDependency walks →DependencyIDs()nonDependencyBlockingReasons→ formats classifiedBlockingReasonsfilterPullable(queue),pickFirstReady,sortByBlockingAnnotations, ready partitionDeliberate decision: FocusPath stays outside
sortByFocusPathkeeps its directHasAny(FocusPath)read. Focus is an ordering interpretation, readiness a membership one; routing the ordering fact through the readiness classifier would re-tangle the two concerns the focus design (#200) keeps apart. Recorded in comments at both sites.Deviations from the epic sketch
Per [LAW:types-are-the-program] (no unread fields, strongest true theorem): no
issueIDfield,orphaned boolinstead of*InProgressAge(display derives age fromUpdatedAt, not annotations), noBlockingSummary() string(the two display surfaces want different formats; formatting stays at consumers).Verification
go test ./...cleanready,backlog,queue