fix: count WHOLE_FILE assignments with empty hunk_indices everywhere - #58
fix: count WHOLE_FILE assignments with empty hunk_indices everywhere#58vitali87 wants to merge 3 commits into
Conversation
|
Warning Review limit reachedNext included review available in 26 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (8)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Greptile SummaryThis change centralizes whole-file hunk expansion for planning and validation. A reproduced case shows that a WHOLE_FILE assignment can retain an invalid stored hunk index, causing conflict validation to reject a plan whose assignments do not overlap on any parsed hunk. Update the whole-file expansion before merging. Confidence Score: 4/5Not safe to merge until WHOLE_FILE coverage excludes nonexistent hunk indices. A focused executable reproducer exercised the full validation path on a one-hunk diff, observed the false overlap error, and confirmed that limiting whole-file coverage to parsed indices allows the same plan to pass. Files Needing Attention: pr_split/schemas.py needs the coverage calculation corrected; tests should cover validation with an out-of-range stored hunk index.
 T-Rex LogsWhat T-Rex did
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| was left empty; PARTIAL_HUNKS covers exactly what it lists. | ||
| """ | ||
| if self.assignment_type is AssignmentType.WHOLE_FILE: | ||
| return sorted(set(range(hunk_count)) | set(self.hunk_indices)) |
There was a problem hiding this comment.
covered_indices() unions every stored hunk_indices value into a WHOLE_FILE assignment, including indices that do not exist in the parsed file. A separate partial assignment containing the same invalid index then intersects it during conflict validation, and the plan is rejected despite sharing no real hunk. Return only the parsed file's index range for WHOLE_FILE assignments.
Artifacts
Narrow Python reproducer for the invalid WHOLE_FILE index conflict
- This executable source parses a one-hunk diff and invokes the full plan validator before and after substituting corrected WHOLE_FILE coverage semantics, with the takeaway that the scenario directly exercises the claimed path.
Current validator output with a false conflict from index 99
- Executed with `uv run` from `/home/user/repo`, this capture shows current WHOLE_FILE coverage includes nonexistent index 99 and validation raises an overlap error, with the takeaway that the claim reproduces on the current code.
Corrected WHOLE_FILE coverage output without the false conflict
- Executed with `uv run` from `/home/user/repo`, this comparison capture limits WHOLE_FILE coverage to parsed indices and validation passes, with the takeaway that ignoring stored invalid indices resolves the false conflict.
Captured source of the executed invalid-index reproducer
- This command capture contains the exact narrow reproducer source used for both executions, with the takeaway that the proof is traceable to the supplied in-process validation scenario.
d25f1f2 to
deb147e
Compare
There was a problem hiding this comment.
Greptile has paused reviews on this repository — it used its 100 free open-source review credits for this billing period. Reviews resume automatically on September 2. To continue before then, an organization admin can keep reviews running past the free credits — those bill as normal usage.
validate_coverage and merge_chain_assignments already treated a WHOLE_FILE assignment with an empty hunk_indices list as covering every hunk, but recompute_estimated_loc, assign_uncovered_hunks and validate_no_conflicts still iterated only hunk_indices. Such a group got estimated_loc=0 and the plan then failed validate_loc; the conflict check was blind to it. Centralise the expansion in GroupAssignment.covered_indices() and use it in all three. Also turn the zero-groups case in assign_uncovered_hunks into a PlanValidationError instead of a raw ValueError from max([]).
deb147e to
2b0b07f
Compare
…ignoring stale stored indices
|
Addressed the T-Rex P1 in e7d6223: |
1 participant
Summary
Commit
1984630taughtvalidate_coverageandmerge_chain_assignmentsto treat aWHOLE_FILEassignment with an emptyhunk_indiceslist as full coverage, but three other consumers still iterated onlyhunk_indices:recompute_estimated_loc→ such a group gotestimated_loc = 0, so the plan passed coverage and then failedvalidate_locwithTotal LOC 0 does not match diff LOC Nassign_uncovered_hunks→ the file's hunks were seen as unassigned and re-added asPARTIAL_HUNKSvalidate_no_conflicts→ overlaps with a whole-file assignment were invisibleThe expansion now lives in one place,
GroupAssignment.covered_indices(hunk_count), and all three use it (validate_no_conflictsgains an optionalhunk_countsargument, passed fromvalidate_plan).Also:
assign_uncovered_hunksraised a bareValueErrorfrommax([])when the LLM returned zero groups in chunked mode; it now raisesPlanValidationError(COVERAGE_GAP).Test plan
uv run ruff check/ruff format --checkcleanuv run pytest -q— 448 passed