perf: make table width fitting width-aware - #401
codeforester wants to merge 2 commits into
Conversation
| self.assertIn("…", output) | ||
| self.assertLessEqual(max(len(line) for line in output.splitlines()), 20) | ||
|
|
||
| def test_table_width_fitting_handles_large_widths_without_per_unit_loop(self) -> None: |
There was a problem hiding this comment.
Test gap: this test's name implies it verifies the fix avoids a per-unit loop, but it only asserts final output values (fitted == [28, 28, 29, 29]), not iteration count or execution time. If a future refactor reintroduced an O(magnitude) per-unit decrement loop that still produced the same final widths, this test would keep passing — the regression would only surface as CI slowness, with no hard guard on the perf property this PR exists to fix.
| while position < len(order) and result[order[position]] == level: | ||
| active_count += 1 | ||
| position += 1 | ||
| while remaining > 0 and active_count: |
There was a problem hiding this comment.
Cleanup: this replacement algorithm (level/position/active_count group bookkeeping) is substantially more intricate than the original 8-line greedy loop but has no comment explaining its invariants — e.g. that order is a snapshot taken before mutation, or why next_level is floored to 1 only when position runs out. I fuzz-tested this against a reimplementation of the old algorithm across ~27,000 random/adversarial cases with zero mismatches, so the logic itself is sound — but a future contributor adjusting width-fitting (a per-column floor, different tie-break order) is likely to break an invariant that's only implicit here.
This branch has not been deployed
1 participant
Fixes #390