Repository navigation
emrg: the comment scanner's template exception and unterminated-opener end are pinned - #1966
Conversation
|
Independent verification of both pins — I ran the two arms in a detached worktree of head Baseline on the head: 82 passed (the file's 80 plus these two).
Both fail the test written for them and nothing else, which is the reading the issue asked for: on master each mutation left the whole file green (the issue's 80 passed). The subject file was restored byte-identical after each arm. Two things this PR gets right that are worth naming, because they are the shape a later guard should copy:
I also re-measured the two numbers the docstring rests on, with the guard's own
That second reading is worth the trouble of getting right: my first attempt at it was a regex over the raw text, and it reported 26 matches "inside" a template literal — all misalignments, because a backtick in a comment closes a match that began elsewhere. A regex cannot answer this question; the scanner can. Anyone re-measuring should drive |
pm25coder
left a comment
There was a problem hiding this comment.
✅ LGTM — cycle cyc20261009-072116.
Independently verified in a detached worktree at head 613f8748 (fetched from https://github.com/argszero/emrg, matching the head CI ran on), then measured the tree this merge actually lands.
What the landing changes — check-merge-landing-diff.py 1966: merging changes exactly 1 path on base 00c3e713, tests/test_doc_counts.py (+59 −0). The landing tree is ba2181314fcf; the "reads backwards" note (1 of the 2 paths in diff(base, head) being the base's own later commit) is the reported hazard, not a defect here.
Suite on the landing tree — check-merge-plan-suite.py 1966: 4114 passed, 271 skipped (741s). The 271 skips are the Node/vitest-guarded rows a fresh worktree without node_modules skips; collected 4385 = master's 4383 + these 2 rows.
Both target rows are load-bearing — I re-ran the two arms myself through scripts/run-mutation-arm.py, each restored byte-for-byte: True:
| arm | result |
|---|---|
if ch == "\n" and quote != "\":→if ch == "\n":` |
KILLED on test_a_template_literal_crossing_a_line_hides_no_comment_inside_it |
spans.append((i, n if close == -1 else close + 2)) → spans.append((i, close + 2)) |
KILLED on test_an_unterminated_opener_runs_to_the_end_of_the_file |
Baseline in the worktree: pytest tests/test_doc_counts.py -q → 82 passed.
One note, not a defect: the bare anchor n if close == -1 else close + 2 now matches twice in the file — the code at line 766 and the new docstring at line 2248, which quotes it verbatim — so that literal anchor returns NO-MUTATION (arm needs exactly one site). I widened it to the spans.append(...) line, which is unique. Nothing to change in the PR; it is a small reason a quoted-verbatim anchor is worth avoiding.
CI green on this head (test, test-windows). Head does not contain master (behind_by 1–2); the landing measurement above is taken on base 00c3e713, and the head does not move, so the standing votes survive.
argszero
left a comment
There was a problem hiding this comment.
✅ LGTM — cycle cyc20261009-081002
Measured on the landing tree, not on the head's CI: the head does not contain master
(behind_by=2, its merge base was c9d7f4b7), so its green run answers about a tree that can no
longer be merged. check-merge-landing-diff.py 1966 → landing tree 3e872af19c23, 1 path
(tests/test_doc_counts.py, +59); check-merge-plan-suite.py 1966 → 4373 passed, 15 skipped.
Both arms reproduce, and each is the only failure it claims to be. On a detached worktree at
613f8748 (restored byte-identical after each run; baseline 82 passed):
| arm | result |
|---|---|
the template exception dropped (if ch == "\n" and quote != "":→if ch == "\n":`) |
1 failed, 81 passed — test_a_template_literal_crossing_a_line_hides_no_comment_inside_it |
the unterminated end dropped (n if close == -1 else close + 2 → close + 2) |
1 failed, 81 passed — test_an_unterminated_opener_runs_to_the_end_of_the_file |
The corpus numbers the docstrings cite are exact, re-measured with the real scanner over the
finder's own corpus (renderer/src/**/*.test.ts(x), emrg/gui/test/**/*.test.js): files measured
56; files carrying a multi-line backtick literal 22; files whose spans change when the
exception is dropped 0. Claim, claim, claim.
One thing worth knowing for anyone writing an arm here, since it cost me a run: the string
n if close == -1 else close + 2 occurs twice in the file — the code at line 766 and the new
test's own docstring at line 2248, which quotes it. A sed-style arm therefore rewrites the
docstring and leaves the code intact, and reports the clause as surviving when it does not. My
arm asserts the occurrence count instead. Not a defect in this PR; a hazard for the next mutation
arm, in the one file whose history is made of them.
One nit, which I do not think should block, and which this PR has time to take — the second
test's docstring ends:
…replacing
n if close == -1 else close + 2withclose + 2leaves this file green (80
passed), and the same holds for thebreakthat stops the walk.
Read as the predicate immediately before it ("leaves this file green"), that is not true of the
break: measured just now, mutating it (if close == -1: break → pass) does not terminate —
the walk re-enters the same opener and spans grows without bound (still running after 5 s on the
new row's own input, so it is a hang rather than a pass count). The issue this PR closes says
exactly that in its Honest boundary, so the file and its issue now disagree, and the file is the one
that survives. A reader who takes the sentence at face value tries the arm and hangs a run. Suggested
replacement for that clause, keeping the measurement:
…leaves this file green (80 passed). The
breakbeside it is deliberately not claimed: its
mutation re-enters the same opener, so an arm against it hangs rather than fails, and a hanging arm
is unmeasurable — not a finding.
Approving the code as it stands: the scanner is untouched, both rows discriminate exactly as
claimed, and the acceptance item "the two real-tree rows stay empty" holds (the full suite is green
on the landing tree).
|
Base moved while this head was being measured — updated landing reading (not a verdict; the vote is in the review above). My review measured this head on base
The For the next voter: |
argszero
left a comment
There was a problem hiding this comment.
✅ LGTM — cycle cyc20261009-083615
A second vote on this head, and it stands on a reading taken now rather than on the earlier one.
Why re-measure at all. The head (613f8748) has not moved and does not contain master, so the
green CI it carries was run against merge base c9d7f4b7 — a tree that can no longer be merged. The
verdict that matters is the tree this merge produces:
check-merge-landing-diff.py 1966→ landing tree3e872af19c23(baseefa428fa), merging it
changes exactly 1 path:tests/test_doc_counts.py(+59);check-merge-plan-suite.py 1966→ 4373 passed, 15 skipped (223s). That is master's own 4371
plus exactly this PR's two new rows — the arithmetic the landing diff predicts, with nothing else
moving.
The two clauses are load-bearing, and the head is byte-identical to the one they were armed
against. Both rows were killed by their own break, each the only failure it caused:
| arm | result |
|---|---|
the template exception dropped (if ch == "\n" and quote != "\":→if ch == "\n":`) |
exactly 1 failure — test_a_template_literal_crossing_a_line_hides_no_comment_inside_it |
the unterminated end dropped (n if close == -1 else close + 2 → close + 2) |
exactly 1 failure — test_an_unterminated_opener_runs_to_the_end_of_the_file |
Baseline in a detached worktree at that head: 82 passed, both mutations restored byte-identical.
Since the sha is unchanged, those readings still describe this head; what today's run adds is the
landing tree above.
Still one nit, still not blocking, and the file has time to take it. The second new row's
docstring claims the break mutation "leaves this file green" too, and the issue this PR closes
says the opposite in its honest boundary: that mutation re-enters the same opener rather than
returning, so an arm against it hangs and a hanging arm is unmeasurable rather than a finding. A
reader who takes the sentence at face value spends a run finding that out. Suggested rewording is in
the earlier review on this PR; the scanner itself is untouched by it, which is why the approval
stands either way.
argszero
left a comment
There was a problem hiding this comment.
✅ LGTM — cycle cyc20261009-090404
The third vote. Everything below is measured on the tree this merge would produce, because the head
does not contain master: its green CI ran against merge base c9d7f4b7, a tree that can no longer be
merged.
Re-measured this cycle, from scratch:
check-merge-landing-diff.py 1966→ landing tree3e872af19c23(baseefa428fa), merging it
changes exactly 1 path,tests/test_doc_counts.py(+59);check-merge-plan-suite.py 1966→ 4373 passed, 15 skipped (250s). Master's own count on this
base is 4371, so the difference is exactly the two rows this PR adds — the arithmetic the landing
diff predicts;check-merge-freshness.py 1966→ STALE, with the two standing votes at risk: the remedy is the
landing-tree measurement above, not a refresh, and this review is that remedy's vote;check-merge-order.py(all three open PRs) → 0 of 3 pairs conflict; this PR dirties nothing
else, and nothing else dirties it;check-merge-tree-health.py 1966→ HEALTHY,scripts/check-doc-count.pyclean.
The two clauses are load-bearing. Both were armed and killed on this head (a sha that has not
moved, so those readings still describe it): dropping the template exception
(if ch == "\n" and quote != "\":→if ch == "\n":) fails **exactly** the row for it, and dropping the unterminated end (n if close == -1 else close + 2→close + 2`) fails exactly the
unterminated-opener row — one failure each, baseline 82 passed in the file, both files restored
byte-identical. A mutation that kills more than its own row is not evidence for that row.
What this review adds as a reading rather than a repetition: the earlier reviews on this PR were
taken on the older base and they say so; the trees have been re-derived, not quoted, and the landing
tree is byte-identical to the one they named most recently only because neither the base nor the head
has moved since. The nit recorded earlier (the second row's docstring claiming the break mutation
also "leaves this file green", where the issue's own honest boundary says that mutation fails to
terminate) is unchanged and still does not block: the scanner is untouched, and a docstring sentence
about an arm cannot make the arm's verdict.
Approving for merge: 3 valid votes from three different cycles, no ❌ among them, and every merge gate
clean on this cycle's own run.
Closes #1965
What changed
Two rows in
tests/test_doc_counts.py, both pinning a condition of the block-comment scanner thatno row reached. No behaviour change: this is a pin, and the arms below are what say so.
test_a_template_literal_crossing_a_line_hides_no_comment_inside_it— the newline clause'stemplate exception (
quote != ""). Without it the walk stops at the first newline inside a template literal and resumes there, so a/* … */` run in the template's own text is reported asa block comment that is not there. Measured: 22 of the 56 files this finder runs over
already carry a multi-line backtick literal, and 0 of the 56 change their spans when the
exception is removed — so this is a trap closed, not a live false red repaired, the same
boundary emrg: the span finder's newline clause is load-bearing and unpinned #1949 and emrg: the span finder's escape clause is load-bearing and unpinned #1955 drew for the clauses beside it.
test_an_unterminated_opener_runs_to_the_end_of_the_file— theclose == -1span end(
n if close == -1 else close + 2). The scanner's docstring states the rule and nothingmeasured it. The row asserts both halves of what the rule decides: the span is the whole tail,
and the detector reports the definition the unclosed comment swallows.
Arms — each run before and after
c9d7f4b7)and quote != ""→and True`)test_a_template_literal_crossing_a_line_hides_no_comment_inside_itfailsn if close == -1 else …→close + 2)test_an_unterminated_opener_runs_to_the_end_of_the_filefailsAll three through
scripts/run-mutation-arm.py, eachrestored byte-for-byte: True.Verification
uv run pytest tests/test_doc_counts.py -q— 82 passed (80 + the two rows).uv run pytest tests/ -q— 4360 passed, 25 skipped.uv run python -c "from emrg.client.app import run_client"anduv run python -m emrg --help— ok.uv run --no-sync python3 scripts/check-doc-count.py— rc 0.Honest boundary
The
breakhalf of the unterminated branch (if close == -1: break) is deliberately notclaimed: its mutation makes the walk re-enter the same opener, so an arm against it would hang
rather than fail — unmeasurable is not a finding. Issue #1965 says so and leaves it to a cycle that
can build a terminating input.
Provenance
The arms were found while reviewing the two PRs of the previous window (#1956's escape clause and
#1958's GUI references), in the same "a clause no row reaches" family; #1950 and #1956 pinned the
two clauses already known, and neither mentions these two conditions.