Skip to content

docs(corpus): state the CLI tier symlink rule - #44

Open
michen00 wants to merge 2 commits into
mainfrom
docs/corpus-cli-readme
Open

docs(corpus): state the CLI tier symlink rule#44
michen00 wants to merge 2 commits into
mainfrom
docs/corpus-cli-readme

Conversation

@michen00

@michen00 michen00 commented Sep 3, 2026

Copy link
Copy Markdown
Owner

What changes, and why

corpus/cli/README.md is what a second harness gets written from, so a rule it omits is a rule the second harness invents.

The symlink rule was omitted. The reference harness copies tree/ with symlinks=True and snapshots a symlink as its target string, and _snapshot's own docstring gives the reason: "A regular file maps to its bytes. A symlink maps to its target string… through one would compare a symlink against a regular copy of its target and call them equal. That is precisely the rewrite the symlink case exists" to catch. The obvious implementation — read through the link — is the wrong one, and the tier ships a case that depends on the answer. So the rule now sits beside the others in "The rules that make it unambiguous", which is the section whose stated purpose is exactly this: "each of these is a question the format would otherwise leave to whoever writes the second harness."

The program name was stale. The section on --help named unwrap-markdown-prose as the Python program. There is no such console script:

$ grep -A2 'project.scripts' pyproject.toml
unwrap-markdown-prose-py = 'markdown_prose_hooks.unwrap:main'
$ ls .venv/bin | grep unwrap
unwrap-markdown-prose-py

And the parse-error claim was true of one implementation. "Usage goes to stdout on --help and to stderr on a parse error" — the Python prints the diagnostic and the usage block; the Rust prints the diagnostic alone:

$ unwrap-markdown-prose-rs --nope z.md
error: unrecognized arguments: --nope

That difference is itself part of why the tier does not compare stderr, so the sentence now says which half is which rather than describing one implementation as though it were both.

Note

This file sits inside corpus/cli/, which .unwrapignore excludes as a subtree and .pre-commit-config.yaml excludes with ^corpus/[^/]+/, so no tidying hook reaches it. The new rule was written as a single line by hand to match the file's existing style.

Corpus

The corpus is the specification, and both implementations answer to it. Tick what applies.

  • This changes no behavior the corpus specifies.
  • This changes what gets joined, and a case in corpus/ pins the new behavior. The case was written first and failed first.
  • The change makes the tool join more than it did. The section above says what it will not eat.

Documentation of the tier's format. No case, answer key or harness changes — the symlink rule describes what the harness already does.

Checks

  • make check passes, or make test does and this touches no Rust.

make tidy. One Markdown file, no Rust and no Python.

This file is what a second harness is written from, and it left one
decision to the reader. The reference harness copies `tree/` with links
preserved and compares a symlink by its target string, never by the
bytes behind it -- and it says why at the snapshot it takes: reading
through the link accepts a run that replaced the link with a regular
file holding the target's bytes, which is the one rewrite a symlink case
exists to rule out. The obvious implementation is the wrong one here, so
the rule belongs beside the others rather than in the harness alone.

Two corrections in the same section. The Python program is named
`unwrap-markdown-prose-py`; the bare name has not been a console script
since before the first tag, and `[project.scripts]` carries only the
suffixed one. And usage does not go to stderr on a parse error in both
implementations: the Python prints the diagnostic and the usage block,
the Rust prints the diagnostic alone. That difference is itself a reason
the tier does not compare stderr, so the sentence now says which half is
which instead of describing one implementation as though it were both.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@qodo-code-review

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)

Grey Divider

Great, no issues found!

Qodo reviewed your code and found no material issues that require review

Grey Divider

Tip of the day
💡 Did you know, you can add REVIEW.md to your repo root and Qodo follows it on every PR

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Document CLI tier symlink and usage rules

📝 Documentation 🕐 Less than 10 minutes

Grey Divider

AI Description

• Documents symlink identity checks to prevent dereferenced files passing corpus comparisons.
• Corrects the Python console script name in help-message guidance.
• Clarifies implementation-specific parse-error output and stderr exclusions.
Diagram

graph TD
  A["CLI tier spec"] -->|defines| B["Harness implementation"] -->|copies| C["Preserved symlinks"] -->|snapshots| D["Target strings"] -->|compares| E["Expected tree"]
Loading
High-Level Assessment

Documenting the existing behavior in the corpus format specification is the appropriate approach because independent harness authors rely on this file. Leaving the rule only in the reference harness was considered but would not prevent other implementations from incorrectly dereferencing symlinks.

Files changed (1) +3 / -1

Documentation (1) +3 / -1
README.mdClarify symlink comparisons and implementation-specific usage output +3/-1

Clarify symlink comparisons and implementation-specific usage output

• Documents that harnesses must preserve symlinks and compare their target strings rather than dereferenced bytes. Corrects the Python executable name and distinguishes Python and Rust parse-error usage behavior while reaffirming that stderr is not compared.

corpus/cli/README.md

@codecov-commenter

codecov-commenter commented Sep 3, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 87.37%. Comparing base (1e9ac89) to head (97ee827).

Additional details and impacted files
@@           Coverage Diff           @@
##             main      #44   +/-   ##
=======================================
  Coverage   87.37%   87.37%           
=======================================
  Files           3        3           
  Lines         689      689           
=======================================
  Hits          602      602           
  Misses         87       87           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

The instruction said to generate the answer key by running the tool, and
the harness has a path for exactly that which the page never named. So a
reader following this file alone runs the binary by hand -- which is the
drift-prone procedure the harness itself argues against, because it owns
the copy of the tree, the modes, the working directory and the snapshot,
and a key produced beside it can disagree with the one it would compare.

The command is now on the line that asks for it, along with the reason
the harness rather than the tool is the thing to run.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟢 Approval recommended

The change is documentation-only and correct in substance; the only feedback is a minor punctuation consistency fix with an inline suggestion.

Pull request overview

This PR updates the CLI corpus documentation (corpus/cli/README.md) to make the CLI-tier rules unambiguous for anyone implementing an independent harness, specifically capturing the symlink comparison behavior and clarifying implementation differences in help/parse-error output.

Changes:

  • Document that expected/ symlinks are asserted as symlinks (by target string), not compared via their dereferenced file contents.
  • Update the documented Python CLI program name and clarify that parse errors differ between Python and Rust regarding whether usage text is also printed.
File summaries
File Description
corpus/cli/README.md Adds the missing symlink rule and clarifies CLI output expectations/differences to prevent harness drift.
Review details
  • Files reviewed: 1/1 changed files
  • Comments generated: 1
  • Review effort level: Lite

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

Comment thread corpus/cli/README.md
Write the files and it is picked up automatically; nothing registers cases by name. Prefer a case that pins one decision, and put the argument in `why` rather than in the slug — the slug becomes the test id, and the `why` is what the next person needs when they are staring at a failure and deciding whether the rule or the case is wrong.

Generate `expected/` and `stdout.txt` by running the tool rather than by writing them out. An answer key written by hand pins what its author believed, which is the one thing a conformance case must not do.
Generate `expected/` and `stdout.txt` with the harness rather than by writing them out: `REGENERATE_CLI_CORPUS=1 uv run python -m pytest tests/test_cli_corpus.py -k <slug>` rewrites both from what the reference run did, leaving `exit_code` as the one expectation you state rather than observe. An answer key written by hand pins what its author believed, which is the one thing a conformance case must not do, and running the tool yourself instead of through the harness reintroduces the same risk one step further back -- the harness owns the copy, the modes, the working directory and the snapshot, and a key produced beside it can disagree with the one it would compare.
Sign up for free to 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.

3 participants