Review follow-up to #62: docstring indentation and invisible U+202F spaces #63

alee · 2026-09-09T02:03:26Z

alee commented

2026-09-09 02:03:26 +00:00

Review follow-up to #62, which merged while the review was in flight. No behaviour change — docstring text only, full suite still 1806.

Broken list indentation

Item 2. of recompute_category's numbered list sat at column 0 while items 1. and 3. sit at 4, with its continuation lines at 8 instead of 7.

Invisible characters in the formula

Four U+202F NARROW NO-BREAK SPACE characters had been inserted around the operators:

Σ‍outcome_samples‍×‍outcome_score‍/‍Σ‍outcome_samples

They render identically to a plain space, so they are invisible in review, in a diff, and in the rendered docstring. I found them only by enumerating non-ASCII codepoints in the changed file:

0x202f  NARROW NO-BREAK SPACE   x4   <- removed
0x3a3   GREEK CAPITAL SIGMA     x4   <- kept
0xd7    MULTIPLICATION SIGN     x2   <- kept
0x2013/0x2014  EN/EM DASH       x3   <- kept

Sigma, the multiplication sign and the dashes stay — those are deliberate and legible. The distinction is the point: a character you can see is a choice; a character you cannot see is a hazard. This repo has just spent a session on that exact failure mode, and plans/text-integrity-audit.md converted its own test fixtures to \u escapes for the same reason.

Review notes on #62 itself, for the record

The change it made is correct and I verified it independently rather than taking the report:

No division by zero — outcome_rows is filtered on outcome_samples > 0, so Σn ≥ 1.
peer_rate = None is handled — expected_success_rate guards it (proficiency.py:171,174), and the new else branch matches the pre-existing empty case.
The test discriminates — reverting the change fails it with assert 0.905 == 0.812, so it would catch a regression rather than restating the implementation.
It also silently fixed a second bug that went unmentioned: the old unweighted form filtered None scores out of the numerator but divided by the unfiltered len(outcome_rows), so any None dragged the peer rate toward zero.

Review follow-up to #62, which merged while the review was in flight. No behaviour change — docstring text only, full suite still 1806. ## Broken list indentation Item `2.` of `recompute_category`'s numbered list sat at column 0 while items `1.` and `3.` sit at 4, with its continuation lines at 8 instead of 7. ## Invisible characters in the formula Four **U+202F NARROW NO-BREAK SPACE** characters had been inserted around the operators: ``` Σ‍outcome_samples‍×‍outcome_score‍/‍Σ‍outcome_samples ``` They render identically to a plain space, so they are invisible in review, in a diff, and in the rendered docstring. I found them only by enumerating non-ASCII codepoints in the changed file: ``` 0x202f NARROW NO-BREAK SPACE x4 <- removed 0x3a3 GREEK CAPITAL SIGMA x4 <- kept 0xd7 MULTIPLICATION SIGN x2 <- kept 0x2013/0x2014 EN/EM DASH x3 <- kept ``` Sigma, the multiplication sign and the dashes stay — those are deliberate and legible. The distinction is the point: **a character you can see is a choice; a character you cannot see is a hazard.** This repo has just spent a session on that exact failure mode, and `plans/text-integrity-audit.md` converted its own test fixtures to `\u` escapes for the same reason. ## Review notes on #62 itself, for the record The change it made is correct and I verified it independently rather than taking the report: - **No division by zero** — `outcome_rows` is filtered on `outcome_samples > 0`, so `Σn ≥ 1`. - **`peer_rate = None` is handled** — `expected_success_rate` guards it (`proficiency.py:171,174`), and the new `else` branch matches the pre-existing empty case. - **The test discriminates** — reverting the change fails it with `assert 0.905 == 0.812`, so it would catch a regression rather than restating the implementation. - It also silently fixed a second bug that went unmentioned: the old unweighted form filtered `None` scores out of the numerator but divided by the unfiltered `len(outcome_rows)`, so any `None` dragged the peer rate toward zero.

alee added 1 commit 2026-09-09 02:03:26 +00:00

docs(proficiency): fix the peer_rate docstring indentation and invisible spaces 79210ed762

Review fixes on 9c7119a. No behaviour change -- docstring text only, full
suite still 1806.

Item 2 of the numbered list had lost its indentation, sitting at column 0
while items 1 and 3 sit at 4, with continuation lines at 8 instead of 7.

More worth having: four U+202F NARROW NO-BREAK SPACE characters had been
inserted around the operators in the formula. They render identically to a
plain space, so they are invisible in review and in any diff -- found only
by enumerating non-ASCII codepoints in the file. Replaced with plain spaces.

Sigma, the multiplication sign and the dashes are kept: those are deliberate
and legible. The distinction is the point -- a character you can see is a
choice, a character you cannot see is a hazard, and this repo has just spent
a session on exactly that failure mode (see plans/text-integrity-audit.md,
whose fixtures were converted to \u escapes for the same reason).

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VRQXz5SYZYVWscxS1QqF6U

alee merged commit 05b772c573 into main

2026-09-09 02:05:44 +00:00

alee referenced this issue from a commit

2026-09-09 02:05:45 +00:00

Merge pull request 'Review follow-up to #62: docstring indentation and invisible U+202F spaces' (#63) from fix/peer-rate-docstring into main

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: alee/6krrt#63