diff --git a/scripts/check-memory-rotation.py b/scripts/check-memory-rotation.py index 839ef005..2c1057a0 100644 --- a/scripts/check-memory-rotation.py +++ b/scripts/check-memory-rotation.py @@ -209,11 +209,26 @@ def _value_of(cells: list[str]) -> str | None: def parse_bookmark(path: Path) -> list[dict]: """Every body row of the bookmark's table, in file order, as key/value pairs. - A table this reader cannot read is **not** an empty table. A row whose key cell is a - code span - the shape every bookmark row has - but which carries no stamp and no note - in any of its cells is reported as unmeasurable, because reading it as "no row" would - report its project as never visited while its row sits in the file, and that is the - defect this tool was written to end (issue #2408). + A table this reader cannot read is **not** an empty table, and neither is a file this + reader cannot read as a table. Both are reported as unmeasurable, because reading + either as "no row" would report its projects as never visited while their rows sit in + the file, and that is the defect this tool was written to end (issue #2408). Two + shapes are refused for that reason (issue #2419): + + * a file that carries **no table line at all** - the shape §1.5's prose describes, + "one line per project", which says nothing about a markdown table. Measured before + this arm existed: a `- `alpha` ` file, a prose file and a table of rows + written without code spans all read as `0 row(s)` and produced one "never been + visited" fault per registered project (exit 1), whose remedy is to re-read every + project rather than to fix the bookmark's shape; + * a **body** row that names its project in no code span, each offending line named. + A header is not a body row: markdown decides that by the separator under it, so a + header is skipped whether or not its first cell is a code span. + + A table that really is empty - a header and no body rows - keeps the honest reading: + every project never visited, exit 1. That is a true statement about a first-ever + bookmark, and the guard above must not swallow it; a test pins both directions, + because those two files produce the same `0 row(s)` line and differ only in shape. """ if not path.is_file(): raise Unmeasurable(f"no bookmark at {path}") @@ -224,19 +239,23 @@ def parse_bookmark(path: Path) -> list[dict]: lines = text.splitlines() rows = [] unreadable = [] + keyless = [] + saw_table = False for index, line in enumerate(lines): if not _is_table_line(line): continue + saw_table = True cells = _cells(line) if not cells or all(SEPARATOR_CELL.match(cell) for cell in cells if cell): continue # the header's separator, not a body row - key_cell = KEY_CELL.match(cells[0]) if cells else None - if key_cell is None: - continue # prose, or a header written without a code span following = next((l for l in lines[index + 1:] if l.strip()), "") if _is_table_line(following) and all( SEPARATOR_CELL.match(cell) for cell in _cells(following) if cell): - continue # a header written as `| `project` | stamp | … |` + continue # a header, with or without a code span + key_cell = KEY_CELL.match(cells[0]) if cells else None + if key_cell is None: + keyless.append(f"line {index + 1}: {line.strip()}") + continue value = _value_of(cells[1:]) if value is None: unreadable.append(f"line {index + 1}: {line.strip()}") @@ -244,6 +263,20 @@ def parse_bookmark(path: Path) -> list[dict]: key = key_cell.group("key") hint = key_cell.group("hint") rows.append({"key": f"{key} ({hint})" if hint else key, "value": value}) + if not saw_table: + raise Unmeasurable( + f"the bookmark at {path} carries no markdown table - {len(lines)} line(s) read " + f"and not one of them is a `| ... |` row - so there are no rows here to read: a " + f"bookmark in another shape is not an empty bookmark, and reading it as one " + f"would report every registered project as never visited" + ) + if keyless: + raise Unmeasurable( + f"the bookmark at {path} carries {len(keyless)} row(s) this reader cannot " + f"read - a table row names its project in its first cell, and these do not " + f"name it in a code span - so the reading below would call their projects " + f"never visited: " + "; ".join(keyless) + ) if unreadable: raise Unmeasurable( f"the bookmark at {path} carries {len(unreadable)} row(s) this reader cannot " diff --git a/tests/test_check_memory_rotation.py b/tests/test_check_memory_rotation.py index 584c0e65..669f5d3e 100644 --- a/tests/test_check_memory_rotation.py +++ b/tests/test_check_memory_rotation.py @@ -369,6 +369,79 @@ def test_a_header_written_with_a_code_span_is_not_a_body_row(mod, tmp_path): assert reading["faults"] == [], reading["faults"] +def test_a_bookmark_in_another_shape_is_unmeasurable_rather_than_empty(mod, tmp_path): + """The shape §1.5's prose asks for - "one line per project" - is not a table. + + Nothing in that sentence says markdown table with a code span in the first cell, so a + cycle that follows it writes a file this reader cannot parse. Read as an empty bookmark + that file produces one "never been visited" fault per project (exit 1), whose remedy is + to re-read all of them — the opposite of the truth, which is that the bookmark's shape + needs fixing. Measured on this host in the cycle that filed issue #2419: a bullet-list + bookmark and a prose-only bookmark both answered `0 row(s)` with a fault per project. + """ + root = _root(tmp_path, "unlisted") + register = _projects(tmp_path / "projects.yml", [{"name": "unlisted", "path": root}]) + for shape, text in ( + ("one line per project", + f"# Rotation bookmark\n\n- `unlisted` {BASE} (cyc20261020-214924)\n"), + ("prose only", "# Rotation bookmark\n\nNothing has been read yet.\n")): + book = tmp_path / f"{shape.replace(' ', '-')}.md" + book.write_text(text, encoding="utf-8") + with pytest.raises(mod.Unmeasurable) as excinfo: + mod.read(register, book) + message = str(excinfo.value) + assert "no markdown table" in message, shape + assert str(book) in message, shape + assert "never visited" in message, shape + + +def test_a_body_row_that_names_its_project_in_no_code_span_is_unmeasurable(mod, tmp_path): + """The same silence one level in: a real table whose keys are written plainly. + + `KEY_CELL` requires the span, so such a row was skipped by the loop with no record + that it had been seen. Rows are there and the reading says `0 row(s)`, so the two + refusals are the same defect - a shape the reader does not know answered as an absence. + """ + root = _root(tmp_path, "plain") + register = _projects(tmp_path / "projects.yml", [{"name": "plain", "path": root}]) + book = tmp_path / "bookmark.md" + book.write_text("\n".join([ + "# Bookmark", "", + "| project | newest read | files |", + "| --- | --- | --- |", + f"| plain | {BASE} | 0 |", + ]) + "\n", encoding="utf-8") + with pytest.raises(mod.Unmeasurable) as excinfo: + mod.read(register, book) + message = str(excinfo.value) + assert "cannot" in message and "never visited" in message + assert "line 5" in message and "| plain |" in message + + +def test_a_bookmark_that_really_is_empty_still_reports_every_project_never_visited(mod, tmp_path): + """The control for the arm above: the same `0 row(s)`, and the honest answer. + + A header and no body rows is a first-ever bookmark, so "never been visited" is true + and exit 1 is right. This file and the bullet-list file above differ only in shape, + and the guard must separate them — measured here rather than assumed, because a guard + that swallowed this case would turn an empty bookmark into "could not measure" and the + rotation would never start. + """ + root = _root(tmp_path, "fresh") + register = _projects(tmp_path / "projects.yml", [{"name": "fresh", "path": root}]) + book = tmp_path / "bookmark.md" + book.write_text("\n".join([ + "# Bookmark", "", + "| project | newest read | files |", + "| --- | --- | --- |", + ]) + "\n", encoding="utf-8") + reading = mod.read(register, book) + assert reading["row_count"] == 0 + assert reading["states"][0]["state"] == "no bookmark row" + assert len(reading["faults"]) == 1 + assert "never been visited" in reading["faults"][0] + + def test_a_project_with_no_row_is_a_fault_and_is_reported_as_never_visited(mod, tmp_path): root = _root(tmp_path, "unrowded") reading = _reading(mod, tmp_path, [{"name": "unrowded", "path": root}], [])