Refuse a duplicate test-evidence heading (improvise-hsoe)
--check-evidence verified that a heading names a test that RUNS, and a duplicate names one just as well, so both copies pass. PR #92 shipped two entries under one heading; the older described a fixture the test no longer had. Refuse rather than warn, on the improvise-rm4s precedent, naming the file and every line in file order. Keyed by the RESOLVED test, so `### f` beside `### m::tests::f` collides too -- the case rm4s's qualification repair creates. Six SELF_TEST_DUPLICATES cases, including the three clean arms: two different tests sharing a leaf are ambiguous and not duplicate, prose repeats freely, and two headings naming two tests are fine. Resolves the two tui.md duplicates: the older column-header copy called THE_MEMBERSHIP_CORRECT_SPANS a live tripwire three hundred lines above the entry recording its removal. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH
This commit is contained in:
@@ -208,6 +208,37 @@ also has no `context/test-evidence/` and no JS roots, so the evidence half is
|
||||
vacuously clean there and the cap is the only thing that can decide the exit
|
||||
code — the isolation is what makes the assertion name one question.
|
||||
|
||||
### Two headings, one test
|
||||
|
||||
An orphan is a heading whose test nobody can find; the mirror image is a test
|
||||
that **two** headings answer for, and it is invisible to every rule above —
|
||||
each copy names a test that runs, so each copy passes. PR #92 shipped two
|
||||
entries under one heading where the older described a fixture the test no
|
||||
longer had and repeated a claim an adversarial review had measured false. A
|
||||
reader meeting the older one first has no way to know a successor exists two
|
||||
thousand lines down, so the stale copy reads as current. That is worse than an
|
||||
orphan, which at least announces itself as broken.
|
||||
|
||||
It is a **refusal and not a warning**, on the precedent `improvise-rm4s` set for
|
||||
the ambiguous leaf: the two are the same defect seen from opposite ends — one
|
||||
name answered by two tests, and one test answered by two names — so they are
|
||||
answered the same way. The refusal names the file and every line, in file order.
|
||||
|
||||
The key is the **resolved test, not the spelling**. `### f` beside
|
||||
`### m::tests::f` is two headings for one test, and grouping by the string would
|
||||
miss it precisely because qualification is the repair `rm4s` asks for; a heading
|
||||
qualified in one file and bare in another would have been the first way this
|
||||
came back. Resolution reuses the same leaf table the ambiguity rule uses, so a
|
||||
leaf that is genuinely ambiguous canonicalises to itself and is reported as
|
||||
ambiguous rather than as a duplicate — two *different* tests sharing a leaf are
|
||||
not two entries for one test, and conflating them would file the wrong repair.
|
||||
The check is over the whole directory rather than per file, because a heading
|
||||
keys a test name and a test name is unique workspace-wide once qualified.
|
||||
|
||||
What it still cannot see, and neither can any of the others: an entry that
|
||||
exists, names a live test, is the only one that does — and describes it
|
||||
backwards.
|
||||
|
||||
## 3. The others
|
||||
|
||||
`scripts/kb` is the only writer of `prolog-sessions/kb.pl`; its own reasoning is
|
||||
|
||||
@@ -192,3 +192,30 @@ reason for a file nobody can find — the same failure as a dead ticket id, whic
|
||||
The first arm was filed as latent and was live when it was written:
|
||||
`context/code/kb-tool.md` had been unpublished, unargued and unnoticed since it
|
||||
was added.
|
||||
|
||||
|
||||
### Duplicate evidence headings — `SELF_TEST_DUPLICATES`
|
||||
|
||||
improvise-hsoe. `--check-evidence` verified that a heading names a test that
|
||||
**runs**, and a duplicate heading names one just as well, so both copies pass.
|
||||
PR #92 shipped two entries under
|
||||
`a_layout_change_moves_the_layout_version_and_no_record`, and two more pairs
|
||||
sat in `tui.md` — one of them asserting that `THE_MEMBERSHIP_CORRECT_SPANS` was
|
||||
"a live tripwire, do not remove it" some three hundred lines above the entry
|
||||
recording that improvise-gd83 had removed it. A reader meeting the older copy
|
||||
first takes it as current, which is worse than an orphan: an orphan announces
|
||||
itself as broken.
|
||||
|
||||
The fault is injected by the six cases in `SELF_TEST_DUPLICATES`, and three of
|
||||
them are the ones that would not get written first. Two *different* tests
|
||||
sharing a leaf must come back **ambiguous and not duplicate**, because the
|
||||
wrong diagnosis files the wrong repair. A prose heading must be free to repeat,
|
||||
because it claims no test. And the bare and module-qualified spellings of one
|
||||
test must collide — grouping by the string rather than by the resolved test
|
||||
would have missed exactly the case improvise-rm4s's qualification repair
|
||||
creates, which is the first way this would have come back.
|
||||
|
||||
The live tree was itself the first injection: before the three merges the check
|
||||
exited 1 naming all three pairs with both line numbers; after them it exits 0.
|
||||
The clean arms (two headings naming two tests, and prose repeated) are what
|
||||
stop an always-refuse from satisfying the rest.
|
||||
|
||||
@@ -87,22 +87,21 @@ Each header line is cut into columns and compared *exactly*. A `contains` would
|
||||
be one-directional: it cannot see a label the browser wrongly blanked (the
|
||||
filter would skip it) and it cannot see one painted under the wrong column.
|
||||
|
||||
**`THE_MEMBERSHIP_CORRECT_SPANS` is a deliberately-wrong assertion and a live
|
||||
tripwire. Do not remove it.** The `assert_ne!` against it starts failing the day
|
||||
improvise-gd83 is fixed, at which point the `assert_eq!` above it should become
|
||||
that constant.
|
||||
**improvise-gd83 is fixed and `THE_MEMBERSHIP_CORRECT_SPANS` is gone**, along
|
||||
with the `assert_ne!` tripwire that named it. The value it predicted —
|
||||
`[(Q1,2), (None,1), (Q1,2), (None,1)]` — is now what the test asserts, element
|
||||
for element. `Sep` is ungrouped, so the run ends before it and resumes under
|
||||
the second measure's own `Q1` header; a precondition asserts the fixture still
|
||||
contains an ungrouped column, because without one the test cannot see a leaked
|
||||
group at all.
|
||||
|
||||
The span assertion above it pins `[(Some("Q1"), 6)]` — Q1 covering all six
|
||||
columns *including* the two under the ungrouped `Sep` — because
|
||||
`expand_category` emits a header on entering a group and no boundary on leaving
|
||||
one, so the nearest-preceding walk keeps answering Q1 (`AxisFrame::group_of`).
|
||||
That is `GridLayout`'s rule and the TUI paints the same span; it is reproduced
|
||||
rather than corrected, because correcting it changes what the TUI draws.
|
||||
|
||||
The membership-correct answer is spelled out rather than guessed, because a
|
||||
wrong future expectation is worse than none: whoever picks up improvise-gd83
|
||||
will read it and believe it. The frame here is **not** one Q1 run — `_Measure`
|
||||
is the outer level, so the column axis is:
|
||||
The superseded expectation — Q1 covering all six columns *including* the two
|
||||
under the ungrouped `Sep`, because `expand_category` emitted a header on
|
||||
entering a group and no boundary on leaving one, so the nearest-preceding walk
|
||||
kept answering Q1 — is recorded here because it is what `AxisFrame::group_of`
|
||||
answered before the fix, and the frame it was derived from still explains the
|
||||
current value. The frame is **not** one Q1 run — `_Measure` is the outer level,
|
||||
so the column axis is:
|
||||
|
||||
col_cats = ["_Measure", "Month"]
|
||||
Group(Q1) [Revenue,Jan] [Revenue,Feb] [Revenue,Sep]
|
||||
@@ -114,6 +113,23 @@ rather than after both. So membership-correct spans are **four** runs, not two
|
||||
run between them. Hence
|
||||
`[(Some("Q1"), 2), (None, 1), (Some("Q1"), 2), (None, 1)]`.
|
||||
|
||||
improvise-8qlr: the TUI arm was `group_line.contains("▼ Q1")`, which is
|
||||
one-directional and passes while the TUI labels **every** column. The run rule
|
||||
now lives once, as `AxisFrame::group_runs`/`group_run_heads`, and both ends
|
||||
call it; the TUI's group line is compared cell for cell against spans derived
|
||||
from the browser's own runs.
|
||||
|
||||
Fault injected exactly as that ticket specifies — forcing the TUI to emit the
|
||||
group label on every column (`head = col_groups.get(ci)`, dropping the run-head
|
||||
lookup). Before: `cargo test -p improvise` stayed green. After: this test fails
|
||||
with `["▼ Q1","▼ Q1","","▼ Q1","▼ Q1",""]` against
|
||||
`["▼ Q1","","","▼ Q1","",""]`.
|
||||
|
||||
One thing the shared rule does **not** flatten: the TUI labels its first
|
||||
*visible* column even mid-run, because a scrolled grid would otherwise draw an
|
||||
unlabelled group line. That is a scrolling concession, kept explicitly, and it
|
||||
is why the comparison is run at `col_offset == 0`.
|
||||
|
||||
### neither_front_end_produces_a_slash_joined_label
|
||||
|
||||
A separate assertion because the agreement tests above would also hold if
|
||||
@@ -165,6 +181,15 @@ so it stays as it is; the multi-level agreement is
|
||||
`tests/grid_frame_agreement.rs`, which drives both front ends over a two-level,
|
||||
grouped fixture.
|
||||
|
||||
Re-pinned for improvise-0o6x. `GROUPED_FRAME` moved by exactly one line: the
|
||||
leading `▼ Co…` header now draws above `East`, and everything below shifts
|
||||
down one. Nothing else in the buffer changed. That the group line, the column
|
||||
headers, the separator, the data rows and the totals strip are byte-identical
|
||||
is the evidence the change is the boundary condition it claims to be and not a
|
||||
reshuffle of the draw loop. It also fails alongside
|
||||
`a_group_header_first_on_its_axis_reaches_both_front_ends` when `body_from`'s
|
||||
match is forced to `false`.
|
||||
|
||||
### two_row_categories_produce_cross_product_labels
|
||||
|
||||
Names the rendering the protocol must not carry: multi-level row headers show
|
||||
@@ -509,44 +534,6 @@ nothing.
|
||||
Fault injected by forcing `body_from`'s match to `false`: this test fails and
|
||||
`the_whole_frame_is_unchanged` fails with it.
|
||||
|
||||
### the_whole_frame_is_unchanged
|
||||
|
||||
Re-pinned for improvise-0o6x. `GROUPED_FRAME` moved by exactly one line: the
|
||||
leading `▼ Co…` header now draws above `East`, and everything below shifts
|
||||
down one. Nothing else in the buffer changed. That the group line, the column
|
||||
headers, the separator, the data rows and the totals strip are byte-identical
|
||||
is the evidence the change is the boundary condition it claims to be and not a
|
||||
reshuffle of the draw loop.
|
||||
|
||||
### the_two_front_ends_draw_the_same_column_header
|
||||
|
||||
Two changes, from two tickets.
|
||||
|
||||
improvise-gd83: `THE_MEMBERSHIP_CORRECT_SPANS` and its `assert_ne!` tripwire are
|
||||
gone, and the value they predicted — `[(Q1,2), (None,1), (Q1,2), (None,1)]` —
|
||||
is now what the test asserts, element for element. `Sep` is ungrouped, so the
|
||||
run ends before it and resumes under the second measure's own `Q1` header. A
|
||||
precondition asserts the fixture still contains an ungrouped column, because
|
||||
without one the test cannot see a leaked group at all.
|
||||
|
||||
improvise-8qlr: the TUI arm was `group_line.contains("▼ Q1")`, which is
|
||||
one-directional and passes while the TUI labels **every** column. The run rule
|
||||
now lives once, as `AxisFrame::group_runs`/`group_run_heads`, and both ends
|
||||
call it; the TUI's group line is compared cell for cell against spans derived
|
||||
from the browser's own runs.
|
||||
|
||||
Fault injected exactly as the ticket specifies — forcing the TUI to emit the
|
||||
group label on every column (`head = col_groups.get(ci)`, dropping the run-head
|
||||
lookup). Before: `cargo test -p improvise` stayed green. After: this test fails
|
||||
with `["▼ Q1","▼ Q1","","▼ Q1","▼ Q1",""]` against
|
||||
`["▼ Q1","","","▼ Q1","",""]`.
|
||||
|
||||
One thing the shared rule does **not** flatten: the TUI labels its first
|
||||
*visible* column even mid-run, because a scrolled grid would otherwise draw an
|
||||
unlabelled group line. That is a scrolling concession, kept explicitly, and it
|
||||
is why the comparison is run at `col_offset == 0`.
|
||||
|
||||
|
||||
### two_abutting_runs_of_one_group_name_are_two_spans_at_both_ends
|
||||
|
||||
improvise-h0iv, end to end. The unit demonstration is in
|
||||
|
||||
@@ -413,8 +413,33 @@ def js_files(roots):
|
||||
yield os.path.join(dirpath, f)
|
||||
|
||||
|
||||
def _duplicate_claims(claims, by_leaf, defined):
|
||||
"""Two headings naming ONE test, keyed by the test they resolve to.
|
||||
|
||||
Resolved and not spelled, so `### f` beside `### m::tests::f` is caught
|
||||
too: the heading is the key a reader looks a test up by, and two entries
|
||||
under one key means the first one found is taken as current. See
|
||||
context/code/tooling.md.
|
||||
"""
|
||||
def canonical(kind, name):
|
||||
if kind == "js" or name in defined["rust"]:
|
||||
return name
|
||||
matches = by_leaf.get(name, set())
|
||||
return next(iter(matches)) if len(matches) == 1 else name
|
||||
|
||||
grouped = {}
|
||||
for (kind, name), where in claims.items():
|
||||
grouped.setdefault((kind, canonical(kind, name)), []).extend(where)
|
||||
def in_file_order(loc):
|
||||
path, _, n = loc.rpartition(":")
|
||||
return (path, int(n) if n.isdigit() else 0)
|
||||
|
||||
return {key: sorted(where, key=in_file_order)
|
||||
for key, where in grouped.items() if len(where) > 1}
|
||||
|
||||
|
||||
def evidence_verdict(claims, defined, citing):
|
||||
"""(orphans, unstated, ambiguous), keyed by ("rust"|"js", name).
|
||||
"""(orphans, unstated, ambiguous, duplicates), keyed by ("rust"|"js", name).
|
||||
|
||||
`orphans` is evidence naming a test nobody can find. `unstated` is the
|
||||
reverse obligation (improvise-lyyc): a test that POINTS AT the evidence
|
||||
@@ -422,8 +447,11 @@ def evidence_verdict(claims, defined, citing):
|
||||
`ambiguous` is a bare leaf that two tests answer to -- a refusal and not a
|
||||
warning, because a heading satisfied by a test in another module answering
|
||||
another question is a pass while the gate does nothing.
|
||||
`duplicates` is the mirror image: two headings answered by one test, where
|
||||
every copy passes and the reader takes whichever comes first as current
|
||||
(improvise-hsoe). Also a refusal, on the improvise-rm4s precedent.
|
||||
Only that narrower reverse obligation, because most tests have no evidence
|
||||
and should not be forced to. What none of the three can see: an entry can
|
||||
and should not be forced to. What none of the four can see: an entry can
|
||||
exist and still describe its subject backwards.
|
||||
"""
|
||||
by_leaf = {}
|
||||
@@ -452,7 +480,7 @@ def evidence_verdict(claims, defined, citing):
|
||||
for name in names:
|
||||
if (kind, name) not in claimed:
|
||||
unstated.setdefault((kind, name), []).extend(sorted(names[name]))
|
||||
return orphans, unstated, ambiguous
|
||||
return orphans, unstated, ambiguous, _duplicate_claims(claims, by_leaf, defined)
|
||||
|
||||
|
||||
def check_evidence(roots, rust_listing=None, rust_ignored=None):
|
||||
@@ -493,10 +521,10 @@ def check_evidence(roots, rust_listing=None, rust_ignored=None):
|
||||
for name in c:
|
||||
citing["js"].setdefault(name, []).append(path)
|
||||
|
||||
orphans, unstated, ambiguous = evidence_verdict(claims, defined, citing)
|
||||
if not orphans and not unstated and not ambiguous:
|
||||
orphans, unstated, ambiguous, duplicates = evidence_verdict(claims, defined, citing)
|
||||
if not orphans and not unstated and not ambiguous and not duplicates:
|
||||
print(f"test-evidence: {len(claims)} entries, all name a test that runs "
|
||||
f"(names from {source}); "
|
||||
f"and no test is named twice (names from {source}); "
|
||||
f"{sum(len(v) for v in citing.values())} tests cite the file and all have one; "
|
||||
f"{prose} prose headings claim nothing")
|
||||
return 0
|
||||
@@ -514,6 +542,11 @@ def check_evidence(roots, rust_listing=None, rust_ignored=None):
|
||||
print(f" [{kind}] {name} ({', '.join(where)})", file=sys.stderr)
|
||||
for path in sorted(by_leaf[name]):
|
||||
print(f" -> {path}", file=sys.stderr)
|
||||
if duplicates:
|
||||
print(f"\n{len(duplicates)} tests are named by more than one test-evidence "
|
||||
"heading; merge them or give each its own test:\n", file=sys.stderr)
|
||||
for (kind, name), where in sorted(duplicates.items()):
|
||||
print(f" [{kind}] {name} ({', '.join(where)})", file=sys.stderr)
|
||||
if unstated:
|
||||
print(f"\n{len(unstated)} tests cite {EVIDENCE_DIR} and have no entry there:\n",
|
||||
file=sys.stderr)
|
||||
@@ -910,15 +943,62 @@ SELF_TEST_AMBIGUITY = [
|
||||
]
|
||||
|
||||
|
||||
def _ambiguity_self_test():
|
||||
# (name, evidence markdown, cargo listing, js source, the tests named twice).
|
||||
#
|
||||
# A heading that names a test that RUNS passes -- and a second heading naming
|
||||
# the same one passes just as well, so an entry superseded by its successor
|
||||
# reads as current to whoever meets it first (improvise-hsoe, found on PR #92).
|
||||
# Every case here is an injection; the clean arms stop an always-fail passing.
|
||||
SELF_TEST_DUPLICATES = [
|
||||
("one heading twice in one file is refused",
|
||||
"### a_real_test\n### a_real_test\n", "m::tests::a_real_test: test\n", "",
|
||||
["m::tests::a_real_test"]),
|
||||
|
||||
("the qualified and bare spellings of one test are the same heading",
|
||||
"### a_real_test\n### m::tests::a_real_test\n", "m::tests::a_real_test: test\n", "",
|
||||
["m::tests::a_real_test"]),
|
||||
|
||||
("two DIFFERENT tests sharing a leaf are not duplicates, they are ambiguous",
|
||||
"### m::tests::shared\n### n::tests::shared\n",
|
||||
"m::tests::shared: test\nn::tests::shared: test\n", "", []),
|
||||
|
||||
("a JS heading twice is refused too",
|
||||
'#### "a spec"\n#### "a spec"\n', "", 'test("a spec", () => {});\n', ["a spec"]),
|
||||
|
||||
("two headings naming two tests are not duplicates",
|
||||
"### one\n### two\n", "m::one: test\nm::two: test\n", "", []),
|
||||
|
||||
("prose headings repeat freely -- they claim no test",
|
||||
"### Helper - x\n### Helper - x\n", "", "", []),
|
||||
]
|
||||
|
||||
|
||||
def _duplicate_self_test():
|
||||
failures = []
|
||||
for name, md, listing, js, want in SELF_TEST_DUPLICATES:
|
||||
claims = {}
|
||||
for kind, cname, n in evidence_claims(md):
|
||||
if kind != "prose":
|
||||
claims.setdefault((kind, cname), []).append(f"fixture:{n}")
|
||||
jd, _ = js_tests(js)
|
||||
defined = {"rust": cargo_test_names(listing), "js": jd}
|
||||
*_, duplicates = evidence_verdict(claims, defined, {"rust": {}, "js": {}})
|
||||
got = sorted(k[1] for k in duplicates)
|
||||
if got != sorted(want):
|
||||
failures.append(f" DUPLICATES: {name}: expected {sorted(want)}, saw {got}")
|
||||
return failures
|
||||
|
||||
|
||||
def _ambiguity_self_test():
|
||||
failures = _duplicate_self_test()
|
||||
for name, md, listing, want_o, want_a in SELF_TEST_AMBIGUITY:
|
||||
claims = {}
|
||||
for kind, cname, n in evidence_claims(md):
|
||||
if kind != "prose":
|
||||
claims.setdefault((kind, cname), []).append(f"fixture:{n}")
|
||||
defined = {"rust": cargo_test_names(listing), "js": set()}
|
||||
orphans, _, ambiguous = evidence_verdict(claims, defined, {"rust": {}, "js": {}})
|
||||
orphans, _, ambiguous, _dupes = evidence_verdict(
|
||||
claims, defined, {"rust": {}, "js": {}})
|
||||
got_o = sorted(k[1] for k in orphans)
|
||||
got_a = sorted(k[1] for k in ambiguous)
|
||||
if got_o != sorted(want_o):
|
||||
@@ -953,7 +1033,9 @@ def _evidence_self_test():
|
||||
defined = {"rust": cargo_test_names(listing), "js": jd}
|
||||
citing = {"rust": {k: ["fixture.rs"] for k in rc},
|
||||
"js": {k: ["fixture.js"] for k in jc}}
|
||||
orphans, unstated, ambiguous = evidence_verdict(claims, defined, citing)
|
||||
orphans, unstated, ambiguous, duplicates = evidence_verdict(claims, defined, citing)
|
||||
if duplicates:
|
||||
failures.append(f" DUPLICATES: {name}: unexpected {sorted(k[1] for k in duplicates)}")
|
||||
got_o = sorted(k[1] for k in orphans)
|
||||
got_u = sorted(k[1] for k in unstated)
|
||||
if got_o != sorted(want_orphans):
|
||||
@@ -1037,6 +1119,7 @@ def self_test():
|
||||
f"{len(SELF_TEST_EVIDENCE)} evidence verdicts, "
|
||||
f"{len(SELF_TEST_LISTING) + len(SELF_TEST_HEADINGS)} listing readings, "
|
||||
f"{len(SELF_TEST_AMBIGUITY)} name resolutions, "
|
||||
f"{len(SELF_TEST_DUPLICATES)} duplicate-heading verdicts, "
|
||||
f"{len(SELF_TEST_DISPATCH)} invocations answer the cap")
|
||||
return 0
|
||||
|
||||
|
||||
Reference in New Issue
Block a user