|
| 1 | +# 2026-09-19 — The check disagreed with itself between runs. One `sorted()` fixed it. |
| 2 | + |
| 3 | +Round: `2026-09-19-partial-clone-blob-vs-absence-p0` |
| 4 | +Adopted proposal: `2026-09-19-partial-clone-blob-vs-absence#p0` — |
| 5 | +*"Sort the checker's findings so two runs can be compared."* |
| 6 | + |
| 7 | +## The premise, re-measured on today's `origin/main` |
| 8 | + |
| 9 | +Eight runs of the **unchanged** checker over the same range (`e53b2142^..e53b2142`), under |
| 10 | +`PYTHONHASHSEED=random`, hashing the output: |
| 11 | + |
| 12 | +``` |
| 13 | +6 16efef4228f50b1147cb57f8e1aced09 |
| 14 | +2 9b1105a77918f30b31c42c890815d10c |
| 15 | +``` |
| 16 | + |
| 17 | +Two orderings of the same six findings. The proposal is right, and it is right for the reason it |
| 18 | +gives: this cost a real round — a before/after diff of this checker looked like a regression until |
| 19 | +the *unchanged* version was shown to disagree with itself. |
| 20 | + |
| 21 | +## Where it came from, which is not quite where the proposal looked |
| 22 | + |
| 23 | +The proposal says *"Findings are accumulated in a set and printed in iteration order."* Close, but |
| 24 | +the print site was already fine: names are emitted through `sorted(theirs_symbols - …)`, so findings |
| 25 | +**within a path** were always ordered. What was unordered is the **path loop** — |
| 26 | +`for path in filter(None, changed)`, where `changed` is a `set` built from two `git diff --name-only` |
| 27 | +results. So the fix is on the outer loop, not at the print: |
| 28 | + |
| 29 | +```python |
| 30 | +for path in sorted(filter(None, changed)): |
| 31 | +``` |
| 32 | + |
| 33 | +**Changed: 1 file, +7/−1 lines** (one of them code, six a comment recording the measurement). |
| 34 | + |
| 35 | +## Axis — bidirectional, on the product script |
| 36 | + |
| 37 | +| form | 8 runs, `PYTHONHASHSEED=random` | |
| 38 | +|---|---| |
| 39 | +| that line reverted | **2 orderings** (6 + 2) | |
| 40 | +| with it | **1 ordering** (8 / 8) | |
| 41 | + |
| 42 | +Content is untouched in both directions: 6 findings, `rc=1`, and the same summary line |
| 43 | +`8 merge(s) in e53b2142^..e53b2142: 6 definition(s) dropped without a trailer`. |
| 44 | + |
| 45 | +**On the acceptance wording**: it asks for "revert it and get red". This change has no verdict to |
| 46 | +flip — it changes output *order*, not outcome, so `rc` is 1 before and after by design. The axis is |
| 47 | +therefore the ordering count above, measured in both directions on the real script. |
| 48 | + |
| 49 | +## What this costs |
| 50 | + |
| 51 | +- **The report no longer reflects traversal order.** Nothing reads it, which is exactly why this was |
| 52 | + safe — and also why it was never noticed. |
| 53 | +- **It makes the report comparable, not the check deterministic.** `examined` counts, which merges |
| 54 | + appear, and everything else that depends on repository state still vary with the repository. A |
| 55 | + future round that diffs two runs across *different* trees will still see real differences; this |
| 56 | + only removes the false ones. |
| 57 | +- **It fixes this checker only.** The sibling checks were not audited for the same shape in this |
| 58 | + round; `check-named-exception-classes-are-loadable.py` emits its missing list through `sorted(…)` |
| 59 | + and walks files through a sorted `rust_files()`, so it appeared safe, but that is an observation |
| 60 | + in passing rather than a measurement. |
| 61 | +- Runtime: **no significant change** — before 2.09 / 2.51 / 2.41 s, after 2.52 / 1.90 / 1.65 s |
| 62 | + (overlapping). Sorting a set of a few dozen paths is not where this check spends its time. |
| 63 | + |
| 64 | +## Effort |
| 65 | + |
| 66 | +The proposal estimated **S** and that was right: the change is one line. The round's actual work was |
| 67 | +the measurement — 8 runs × 2 directions — which is what turns "should be sorted" into "was unordered, |
| 68 | +here by how much". |
0 commit comments