Conversation
Works through the four candidates the :path()-retirement fix deliberately left untouched. Verified safety the same way that fix proved once already: check whether a pointer, when present, actually reduces each partition to at most one candidate before trusting a count-based deferral. RESULTS' instance-level :all()+accessor check (both literal-root and traversal) is confirmed structurally settled, not a count-dependent candidate at all -- left unchanged, permanently. RESULTS' _star_group_and_reduce() GROUP-mode content-accessor check is converted: each partition's own pointer application guarantees at most one selected run, and _results_for_run()'s own internal guards guarantee at most one result per run when an accessor is present, so more than one result can only mean more than one group's own already-legitimate winner -- the same shape already proven safe to defer. FILES' _query_star_traversal() unconditional :manifest() rejection is converted the same way, plus the literal-root ':all()'/':groups()' grouping + :manifest() rejection is narrowed. Found two real, previously-undetected bugs while verifying this live, not just missing conversions: both the literal-root and traversal FILES checks were also rejecting :manifest() combined with a chained field accessor (e.g. :manifest():uuid()), even though field accessors are exempt from Rule 1 entirely and resolve_kind's own priority means :manifest() is never actually read as a whole resource in that shape. Fixed at all three affected sites, which all shared the same incomplete has_manifest computation. Two new regression tests for the field-accessor bug; four existing '*'-traversal tests rewritten from unconditional raises to either "now works" (POOL mode, never actually ambiguous) or "query succeeds/resolve raises" pairs (GROUP mode); two RESULTS tests rewritten the same way. Full suite: 1612 passed (up from 1609). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Gdf2DfxEeGeUxweybvNhH7
This branch has not been deployed
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.
Summary
The first "fully specified" item from the bucket list — the
'*'-traversal content-accessor guards re-audit left over from the:path()-retirement fix. This one turned out to have real teeth, so it's worth a careful read.Worked through all four candidates the earlier fix deliberately left untouched, verifying safety the same way that fix proved once already: check whether a pointer, when present, actually reduces each partition to at most one candidate before trusting a count-based deferral (the trap already caught once there — CSVPATHS'
:all():last():manifest(), several groups each already reduced to their own one candidate, must NOT raise even though the final count is > 1).:all()+accessor check — confirmed structurally settled, not actually a count-dependent candidate at all (instance-level:all()pools multiple statement files with no pointer concept to reduce them). Left unchanged, permanently, both literal-root and traversal._star_group_and_reduce()GROUP-mode content-accessor check — converted. Traced the exact guarantee before removing anything: each partition's own pointer application returns at most one run, and_results_for_run()'s own internal guards independently guarantee at most one result per run when an accessor is present._query_star_traversal()unconditional:manifest()rejection — converted the same way, plus the literal-root':all()'/':groups()' grouping + :manifest()rejection narrowed.Two real, previously-undetected bugs caught while verifying #3 live, not just missing conversions: both the literal-root and
'*'-traversal FILES checks were also incorrectly rejecting:manifest()combined with a chained field accessor (e.g.:manifest():uuid()) — even though field accessors are exempt from Rule 1 entirely, andresolve_kind's own priority means:manifest()is never actually read as a whole resource in that shape. All three affected call sites shared the same incompletehas_manifestcomputation (it never checked whether a field accessor was also present). Confirmed live with a probe script before fixing, not assumed.Two new regression tests for the field-accessor bug; four existing
'*'-traversal tests rewritten from unconditional raises to either "now works" (POOL mode, never actually ambiguous) or "query succeeds/resolve raises" pairs (GROUP mode); two RESULTS tests rewritten the same way.Test plan
CSVPATH_CONFIG_PATH=.../config.ini pytest tests/references/ -q→ 1612 passed (up from 1609)Co-Authored-By: Claude Sonnet 5 noreply@anthropic.com
https://claude.ai/code/session_01Gdf2DfxEeGeUxweybvNhH7