Conversation
…), fixing CI on PR #600 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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
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.
What
board_from_cacheinscripts/statusline.pycomputed astatefield (unknown/partial/measured) that nothing downstream ever read --_board_field, the renderer, readsprs/issues/issues_external/checksdirectly and renders?per missing value regardless of whatstatesaid. #595 had just added a rule tostate(a cache withissuesbut noissues_externalispartial) that, for the same reason, could never affect anything a maintainer sees.#597 named two directions and deliberately chose neither: give
statea reader (the render), or delete it. This takes the delete path.Why delete rather than wire a reader
The render is the most-visible field on the line and a design change there (a
partialboard rendering differently from ameasuredone) needs its own argument about what it costs in width and reader attention -- and, on inspection,statewas never even a complete summary of what the render already shows: it only ever consideredprs/issues/issues_external, neverchecks. A reader added for it would have inherited that gap. Each of the three countsstatesummarized already carries its own missing/present distinction (Nonevs an int), which is exactly what_board_fieldalready renders as?vs a number. A summary that can disagree with the values it summarizes -- and, in this case, was already known to be inert -- is a second copy of the same fact rather than new information.Verified the premise
Grepped
board_from_cacheand["state"]/.get("state")acrossscripts/,commands/,hooks/: the only production caller ofboard_from_cacheisgather()(scripts/statusline.py:1479), which never readsboard["state"]; the board dict only reaches_board_field, which reads the four other keys via.get(). No other caller anywhere in the repo reads this field. Confirmed by both spawned reviews independently.Tests
Red:
tests/test_statusline_state_deleted_597.py::test_board_from_cache_carries_no_state_keyfailed before the fix (statewas present). Green after. Positive control in the same file (test_the_per_field_questionmarks_still_distinguish_every_case_state_used_to_name) proves the per-field?/int distinction alone still recovers every casestateused to name, including the must-fire zero-vs-missing control forissues_external. Three existing test files (test_statusline_479.py,test_statusline_board_external_595.py) had theirboard["state"]assertions removed as a deliberate contract change, not an oversight --stateis deleted, so there is no contract left to assert.Full suite: 3766 passed, 7 skipped (pre-existing, unrelated), coverage 89.34%.
Docs
README.mddoes not document the board field format at all (confirmed, matches the issue's own note) -- no change needed..oss/statusline.py, the owned vendored copy, is rendered fromscripts/statusline.pyat scaffold time (scaffold.py::_owned_statusline), not hand-maintained -- it will carry this fix at the next re-scaffold, noted in the changelog fragment.Closes #597