Skip to content

Read a Tool Row Without a Trailing Pipe, and Report the License Column - #636

Merged
ptr727 merged 2 commits into
developfrom
read-a-tool-row-without-a-trailing-pipe
Aug 9, 2026
Merged

Read a Tool Row Without a Trailing Pipe, and Report the License Column#636
ptr727 merged 2 commits into
developfrom
read-a-tool-row-without-a-trailing-pipe

Conversation

@ptr727

Copy link
Copy Markdown
Owner

Raised by Copilot on the promotion pull request #635, against develop code rather than against anything that promotion introduces. Both halves of the finding are real, and both are measured below rather than argued.

The trailing pipe

_TOOL_ROW required a closing | after the description. GitHub's Markdown makes that pipe optional, so a valid row written without one matched nothing and was skipped by continue before any check ran on it:

MATCH '| [cspell][cspell-link] | Spell checker. |' -> 'Spell checker.'
SKIPPED '| [cspell][cspell-link] | Spell checker.' -> None
MATCH '|[cspell][cspell-link]|Spell checker.|' -> 'Spell checker.'
MATCH '| [cspell][cspell-link] | Spell checker. | MIT |' -> 'Spell checker.'

A skipped row loses its link check, its description check, and its place in the ordering list, so a whole table written that way reports zero findings. That is the failure shape this dimension exists to avoid: it scores as clean rather than as unread.

Nothing else catches it either. MD060 is disabled outright in .markdownlint-cli2.jsonc, and MD055 defaults to consistent, which passes a table whose rows all omit the closing pipe. So the form is permitted everywhere and was read nowhere.

The License column

spec/project-types.json already asserted "The section carries no license column" at verdict letter, and nothing read it. spec/readme-structure.md states the rule and the reason: a license belongs to the dependency and is authoritative at its source. This makes the existing assert honest rather than adding a new rule.

Two decisions bound it:

  • Read off the header, not the rows, so it reports once for the table rather than once per tool, and so a tool whose own role is the word "License" is not mistaken for the column. There is a test in that direction.
  • That one column and no other. Copilot suggested flagging extra columns generally; the spec forbids the License column and says nothing about any other, so a general check would grade the fleet on a rule nobody wrote.

A row carrying the extra column still parses correctly, because the description capture is non-greedy, so the per-tool checks keep working alongside the new finding.

Measured against the corpus

All 22 cataloged READMEs, at their ground-truth branches, old check against new, read in both directions:

OldNew
Blog45
LanguageTags1212
MediaTools1313
PlexCleaner2727
Total5657

One finding added, Blog's License column, and no verdict lost in the other direction. The categories break down as 48 absent descriptions, 4 links, 3 descriptions, 1 ordering, 1 License column.

The honest half of that result: no repo in the fleet writes a trailing-pipe-less row today, so that fix closes a blind spot rather than a live miss. It is worth taking anyway, because the blind spot is silent and the 17 repos that still owe this section have not written their tables yet.

TODO.md takes the new count, and two pre-existing miscounts in the same bullet are corrected while I am in it: PlexCleaner's 27 is its total rather than its absent-description count, which is 25, and all three of Blog's roles differ from the catalog rather than two, since Hugo's matches in substance but differs by its opening capital and its full stop.

Verification

spec/audit.py --selftest passes with 7 new cases, covering both halves in both directions: a row read without a trailing pipe, one judged without one, a whole table ordered without them, the License column reported once, the rows still read alongside it, a table without the column not reported, and a row cell reading "License" not mistaken for the header. spec/validate.py, prose_lint.py --diff develop, repo_gate.py --check eol-coverage, editorconfig-checker, markdownlint-cli2 and cspell all clean. spec/audit.py stays LF, TODO.md stays CRLF.

🤖 Generated with Claude Code

The 3rd Party Tools check required a closing pipe on every table row, which
GitHub's Markdown makes optional, so a row written without one was skipped
outright and its link, description and ordering all went unread. A table
written that way scored clean rather than unread.
spec/project-types.json asserted "The section carries no license column" at
verdict letter, and nothing read it. The header is now read for that column,
once per table rather than once per row, and it is the only extra column
named, since spec/readme-structure.md forbids that one and no other.
Measured against all 22 cataloged READMEs, in both directions: 56 findings
become 57, the one addition being Blog's License column, and no verdict is
lost. No repo writes a trailing-pipe-less row today, so that half closes a
blind spot rather than a live miss.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
CopilotAI lite review requested due to automatic review settings August 9, 2026 03:50

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR tightens the README audit’s parsing of the 3rd Party Tools section so valid Markdown table rows aren’t skipped, and adds a new rule to report a forbidden License column based on the table header. It also updates TODO.md to reflect the newly measured finding counts and corrected breakdown.

Changes:

  • Update _TOOL_ROW parsing to accept tool-table rows that omit the trailing |.
  • Add header-based detection of a License column in the 3rd Party Tools table, plus selftests for the new behaviors.
  • Refresh TODO.md measurement dates/counts and correct prior miscounts.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

FileDescription
TODO.mdUpdates the recorded fleet measurement totals/dates and corrects the described breakdown for 3rd Party Tools findings.
spec/audit.pyImproves Markdown table parsing for 3rd Party Tools rows and adds a header-based License-column finding with expanded selftests.
Suppressed comments (1)

spec/audit.py:710

  • table_cells() only recognizes rows that start with |. GitHub-flavored Markdown also allows header/separator/data rows without the leading pipe (e.g. Tool | Role). With the new License-column check keyed off the header, a table using that valid form won’t be recognized and can again score as clean despite being present. Expanding table_cells() to accept rows with or without the leading pipe avoids that blind spot.
 s = line.strip()
if not s.startswith("|"):
return None
s = s[1:-1] if s.endswith("|") else s[1:]
return [c.strip() for c in s.split("|")]

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment threadspec/audit.py Outdated
Copilot raised the symmetric case on #636: GitHub's Markdown makes the
leading pipe optional as well, so a table written without it was skipped by
_TOOL_ROW and unread by table_cells, which is the exact shape the first
commit fixed on the other side.
table_cells now requires a separating pipe rather than a leading one. That
is what keeps a thematic break out: `---` alone carries no pipe, where
treating it as a one-cell delimiter row would have read the paragraph above
it as a table header.
Re-measured against all 22 cataloged READMEs: the total stays 57 and no
verdict moves in either direction, so the widening introduces no false
positive and catches nothing the fleet writes today. Five new selftest
cases cover the leading-pipe row, a table with neither outer pipe, a
License column without them, the thematic break, and prose naming a
cataloged tool.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
CopilotAI review requested due to automatic review settings August 9, 2026 03:55
@ptr727

Copy link
Copy Markdown
OwnerAuthor

Answering the suppressed comment on spec/audit.py:710, which carries no thread to resolve:

table_cells() only recognizes rows that start with |. GitHub-flavored Markdown also allows header/separator/data rows without the leading pipe (e.g. Tool | Role). With the new License-column check keyed off the header, a table using that valid form won't be recognized and can again score as clean despite being present.

Accepted, and fixed in 45fde3e together with the threaded finding it duplicates. It is the sharper statement of the two, because it names the consequence exactly: the License column is read off the header, so a header written without a leading pipe would have taken the check straight back to scoring clean on a table nothing had read.

table_cells now requires a separating pipe rather than a leading one, and drops each outer pipe only where it is present. The separator is doing real work: --- alone carries no pipe and returns None, so a thematic break is not a one-cell delimiter row and the paragraph above it is not read as a table header. Selftest cases cover that direction and prose naming a cataloged tool.

Re-measured across all 22 cataloged READMEs, in both directions: the total stays 57 and no verdict moves, so no false positive. Worth being plain about the other half of that result, though: neither outer-pipe form appears anywhere in the fleet today, so both fixes close a silent blind spot rather than catch a live miss. The 17 repos that still owe this section have not written their tables yet, which is the reason to take it now rather than after.

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

@ptr727
ptr727 merged commit 1e2a4ba into developAug 9, 2026
7 checks passed
@ptr727
ptr727 deleted the read-a-tool-row-without-a-trailing-pipe branch August 9, 2026 04:00
ptr727 added a commit that referenced this pull request Aug 9, 2026
…ate (#635)
Promotion of develop at 2082547, six squashes since the last one. Closes#633.
#631 routes the README by reader and documents the GH_WRITE_GUARD_ALLOW grant where a denied cross-owner write puts the reader. #632 moves readme.sections from intent to letter with four checks beside it, backed by spec/readme-sections.json and spec/third-party-tools.json, and settles the tagline rule. #634 adds repo_gate.py --check eol-coverage, reading the line-ending pins against the tree rather than only against .editorconfig.
#636 and #637 repair two readers Copilot found on this pull request, both defects develop already carried: a tool row required both outer table pipes that GitHub's Markdown makes optional, and a retired badge written as an inline image was invisible to a scan that read reference definitions alone. #638 turns the host contract's presence check into a version gate, and retires two gh workarounds that were artifacts of a stale distribution package, re-tested on an upgraded host rather than inferred.
Four carried files owe a downstream re-vendor and none is recorded in the TODO.md entry yet. GOVERNANCE.md Repository Details is verbatim, so the audit reports it, and it propagates a rule: the About description is the tagline alone, and Docker Hub receives it from the About panel rather than from the README. CODESTYLE.md item 4 and .gitattributes are intent, so nothing reports them, and the second couples to the new gate through the forward-declared mark. .github/copilot-instructions.md is intent and propagates a correction rather than a refresh, so a repo left on the old copy is wrong rather than merely stale.
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@ptr727