Skip to content

fix: floor the auto-derived min_loc at 1 - #72

Merged
vitali87 merged 5 commits into
mainfrom
fix/derived-min-loc-floor
Aug 31, 2026
Merged

fix: floor the auto-derived min_loc at 1#72
vitali87 merged 5 commits into
mainfrom
fix/derived-min-loc-floor

Conversation

@vitali87

@vitali87 vitali87 commented Aug 30, 2026

Copy link
Copy Markdown
Owner

Problem

Settings.auto_derive_min_loc sets min_loc = max_loc // DEFAULT_MIN_LOC_RATIO when refinement is enabled and min_loc is unset. For max_loc < 4 that is 0, which the explicit field constraint (min_loc: int | None = Field(ge=1)) would have rejected. Settings(max_loc=3, max_refinement_iterations=1).min_loc == 0 then feeds into detect_loc_bound_violations / score_plan as a real bound.

Fix

max(1, max_loc // DEFAULT_MIN_LOC_RATIO). max_loc=1 with refinement now fails the existing min_loc < max_loc check with the normal MIN_LOC_GE_MAX_LOC message (the CLI already reports ValidationError/ValueError from Settings cleanly) instead of running with min_loc=0.

Tests: max_loc 2 and 3 derive min_loc == 1; max_loc=1 is rejected; max_loc=400 still derives 100. 448 tests pass, ruff clean.

Summary by CodeRabbit

  • Bug Fixes

    • Ensured automatically calculated minimum location values never fall below the supported minimum of 1.
    • Invalid configurations with an incompatible maximum location value now continue to be rejected clearly.
  • Tests

    • Added coverage for small and typical configuration values, including validation of invalid inputs.

@coderabbitai

coderabbitai Bot commented Aug 30, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: ad0436f2-85c4-44e7-8b03-5a54f44b1e6b

📥 Commits

Reviewing files that changed from the base of the PR and between 2e946a3 and d9d6573.

📒 Files selected for processing (2)
  • pr_split/config.py
  • tests/test_config.py

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

The change floors automatically derived min_loc values at 1 when refinement is enabled. Tests cover small, invalid, and normal max_loc values.

Changes

Derived min_loc validation

Layer / File(s) Summary
Derive and validate min_loc
pr_split/config.py, tests/test_config.py
auto_derive_min_loc now floors the derived value at 1. Tests cover max_loc values 2, 3, 1, and 400.

Estimated code review effort: 1 (Trivial) | ~5 minutes

Merge Risk: ⚪ Minimal · up to d9d65

The change prevents invalid automatically derived bounds while preserving the existing rejection for incompatible values. No actionable merge-blocking risk remains after normal checks and review.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 2 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: flooring the automatically derived min_loc value at 1.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/derived-min-loc-floor

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

With refinement enabled and min_loc unset, min_loc = max_loc // 4 gave 0
for max_loc < 4, bypassing the ge=1 constraint on the explicit field.
Floor it at 1; max_loc=1 now fails the min_loc < max_loc check instead
of silently running with min_loc=0.
@vitali87
vitali87 force-pushed the fix/derived-min-loc-floor branch from 6c60102 to 6bb9a38 Compare August 30, 2026 13:05
@vitali87
vitali87 merged commit fc6ad35 into main Aug 31, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant