From 6de35a38bbc7c8729354f95998e3a42aa27d5fa8 Mon Sep 17 00:00:00 2001 From: melbinjp Date: Wed, 19 Aug 2026 08:41:49 +0530 Subject: [PATCH] fix: read a directory diagram whose entries carry a description column ComposioHQ/composio draws its layout in a ```text block: ts/ TypeScript SDK workspace packages/core/ @composio/core packages/providers/ Provider adapters python/ Python SDK and provider packages looks_like_a_tree said no. Its indentation-only branch requires every line to be a single token and requires some line to END in a slash, and both fail here because the description sits where the slash would be. So the block fell through to the plain path reader, which took `packages/providers/` as a path from the repository root and reported it deleted. ts/packages/providers exists. The trigger is what makes this worth fixing rather than tolerating. A path is only reported as deleted when history holds a commit that removed it, and composio moved packages/ under ts/ on 2025-06-16. A directory that never sat at the root produces no finding at all. So the misreading fires precisely on the projects that reorganised, which are the ones whose layout diagrams are most likely to be stale and most worth checking. STRICTLY ADDITIVE. The relaxation is a new branch reached only when the existing one declines, and it is confined to the FIRST token of each line: every one must be path-shaped, carrying a slash or a file extension, and at least one must end in a slash. So an options list, a shell transcript, a flat list of filenames, and a block with one bare word among the paths are all still not diagrams. parse() already read the name as name.split(" ")[0], which is why nothing below the detector had to change. MEASURED, not reasoned: - 106 of 110,615 fenced blocks across 217 repositories newly read as diagrams (0.1%). Every one of a twelve-block sample is a real layout - scrapy, sanity, gsd-core, rustfs, SparkyFitness, llmgateway and others. - Re-swept 68 repositories with recorded results: batch 10 moved exactly one, composio, from one finding to zero. Batches 8 and 9 moved nothing. One false positive removed, no findings added anywhere. - Where a line holds two entries side by side, the second is not claimed. That gap is asserted in a test so nobody later 'fixes' it into guessing. 190 tests, up from 184. Two of the six new ones fail against the old detector; the other four are negative controls and pass either way, which is what a guard should do. --- src/docproof/tree.py | 55 ++++++++++++++++++- tests/test_described_tree.py | 100 +++++++++++++++++++++++++++++++++++ 2 files changed, 153 insertions(+), 2 deletions(-) create mode 100644 tests/test_described_tree.py diff --git a/src/docproof/tree.py b/src/docproof/tree.py index 133c055..c7aff9f 100644 --- a/src/docproof/tree.py +++ b/src/docproof/tree.py @@ -121,9 +121,60 @@ def looks_like_a_tree(text: str) -> bool: # token rather than a statement. if not any(line[:1] in " \t" for line in lines): return False - if not any(line.rstrip().endswith("/") for line in lines): + if all(len(line.split()) == 1 and not set(line) & set("=(){};") for line in lines): + return any(line.rstrip().endswith("/") for line in lines) + return _described_tree(lines) + + +def _path_shaped(token: str) -> bool: + """Is this token, on its own, unambiguously a path rather than a word? + + Deliberately strict. It is the ONLY thing standing between the relaxed branch below + and reading an options list or a shell transcript as a directory layout, so a bare + word is not enough: the token has to carry a slash or a file extension. + """ + if set(token) & set("=(){};,\"'`*?<>|"): + return False + if "/" in token: + return True + stem, dot, extension = token.rpartition(".") + return bool(stem and dot and extension.isalnum() and len(extension) <= 5) + + +def _described_tree(lines: list[str]) -> bool: + """A layout whose entries are followed by a DESCRIPTION, with no comment marker. + + ```text + ts/ TypeScript SDK workspace + packages/core/ @composio/core + packages/providers/ Provider adapters + python/ Python SDK and provider packages + ``` + + **This is `ComposioHQ/composio`'s README and it produced a false positive.** The + branch above requires every line to be a single token, and requires a line to END in + a slash; both fail here, because the description sits where the slash would be. So + the block was not a tree, and the plain path reader took `packages/providers/` as a + path from the repository ROOT. It is at `ts/packages/providers`, and it exists. + + Worse than an ordinary false positive, because of what triggers it: the finding only + appears when the path ALSO existed at the root once, since that is what gives it a + deletion commit to cite. composio moved `packages/` under `ts/` in 2025-06-16. So the + misreading selects precisely for projects that reorganised, which are the projects + whose diagrams are most worth checking. + + The relaxation is confined to the FIRST token of each line. Every one of them must be + path-shaped, so `Options:` / `--verbose` and `python -m pytest` are still not trees, + and at least one must end in a slash, so a plain list of files is not a layout. + `parse` already reads the name as `name.split(" ")[0]`, which is why nothing below + this function had to change to understand the description column. + """ + firsts = [line.split()[0] for line in lines if line.split()] + if len(firsts) != len(lines): + return False + if not any(token.endswith("/") for token in firsts): return False - return all(len(line.split()) == 1 and not set(line) & set("=(){};") for line in lines) + return all(_path_shaped(token) for token in firsts) def parse(text: str, first_line: int) -> list[Entry]: diff --git a/tests/test_described_tree.py b/tests/test_described_tree.py new file mode 100644 index 0000000..927da4d --- /dev/null +++ b/tests/test_described_tree.py @@ -0,0 +1,100 @@ +"""A directory diagram whose entries carry a description column is still a diagram. + +**Measured on `ComposioHQ/composio`.** Its README draws the repository layout like this: + + ```text + ts/ TypeScript SDK workspace + packages/core/ @composio/core + packages/providers/ Provider adapters + packages/cli/ Composio CLI + python/ Python SDK and provider packages + ``` + +`looks_like_a_tree` said no, because the indentation-only branch demanded that every line +be a single token and that some line END in a slash. Both fail here: the description sits +where the slash would be. So the block fell through to the plain path reader, which took +`packages/providers/` as a path from the repository root and reported it deleted. + +`ts/packages/providers` exists. + +**The trigger is what makes this worth fixing rather than tolerating.** A path is only +reported as deleted when history has a commit that removed it, and composio moved +`packages/` under `ts/` on 2025-06-16. A directory that never sat at the root produces no +finding at all. So the misreading fires precisely on projects that reorganised, which are +the projects whose layout diagrams are most likely to be stale and most worth checking. + +Measured after the change: **106 of 110,615 fenced blocks across 217 repositories** newly +read as diagrams, and re-sweeping batch 10 moved exactly one repository, composio, from one +finding to zero. No new findings anywhere. +""" + +from __future__ import annotations + +from docproof.tree import looks_like_a_tree, parse + +COMPOSIO = """\ +ts/ TypeScript SDK workspace + packages/core/ @composio/core + packages/providers/ Provider adapters + packages/cli/ Composio CLI +python/ Python SDK and provider packages +docs/ Documentation site (docs.composio.dev)""" + + +def test_the_composio_block_is_a_diagram_and_resolves_under_its_parent() -> None: + assert looks_like_a_tree(COMPOSIO) + at = {entry.path: entry.line for entry in parse(COMPOSIO, 156)} + # The claim that was reported deleted, now carrying the prefix the drawing gives it. + assert "ts/packages/providers" in at + assert "packages/providers" not in at + # Siblings at column zero stay at the top level rather than inheriting `ts/`. + assert "python" in at and "docs" in at + # And the line is the entry's own, which is what a maintainer opens. + assert at["ts/packages/providers"] == 158 + + +def test_an_options_list_is_not_a_diagram() -> None: + """The first token has to be path-shaped, and `--verbose` is not.""" + assert not looks_like_a_tree("Options:\n --verbose be loud\n --quiet be quiet") + + +def test_a_shell_transcript_is_not_a_diagram() -> None: + """`python` is a bare word, so the block never reaches the slash test.""" + assert not looks_like_a_tree("python -m pip install x\n python -m pytest\npython -m build") + + +def test_one_bare_word_disqualifies_the_whole_block() -> None: + """**The narrow part of the relaxation, asserted directly.** + + Two of these three lines are paths and one is the word `make`. Accepting the block + would claim `make` as a path at the top level. Every first token has to qualify, not + most of them, because the cost of being wrong here is a finding about a file nobody + wrote. + """ + assert not looks_like_a_tree("src/ source\nbuild/ output\n make run it") + + +def test_a_flat_list_of_files_is_not_a_layout() -> None: + """Nothing ends in a slash, so there is no directory for a description column to + hang off, and a list of filenames in a code block stays a list.""" + assert not looks_like_a_tree("alpha.py the first\n beta.py the second\ngamma.py third") + + +def test_a_line_holding_several_entries_under_claims_rather_than_guesses() -> None: + """`rustfs` writes `p0-before/ p0-after/` on one line, two directories side by side. + + `parse` reads the first token and drops the rest, so the second entry is not claimed. + That is a gap and it is the safe direction: a claim not made costs coverage, a claim + invented costs the reader's trust. Asserted so that nobody 'fixes' it into guessing. + """ + text = "bench/\n p0-before/ p0-after/ summaries\n p1-before/ p1-after/ deltas" + assert looks_like_a_tree(text) + paths = {entry.path for entry in parse(text, 1)} + # `bench/` is the diagram's own root with everything nested under it, so it names the + # repository and is dropped. That is existing behaviour and this test asserted the + # opposite on the first attempt, which is worth leaving recorded: the prefix rule and + # the root rule interact, and only one of them is new here. + assert paths == {"p0-before", "p1-before"} + # The second entry on each line is not claimed. A claim not made costs coverage; a + # claim invented costs the reader's trust. + assert "p0-after" not in paths