Repository navigation
README: draw the specification diagram, not only the two-strategy diff - #20
Merged
Merged
Conversation
The before/after is the pitch — the same graph with nothing implemented, then with two implementations bound — and only the second half was ever drawn. `diagram()` was described in prose and never shown. The check that should have caught this could not. `test_the_readme_mermaid_block_is_ the_diagram_the_code_emits` asserted ONE block by name, at a time when the README had exactly one: a check narrower than its claim, and blind to a missing picture because the blocks that are present all pass. Replaced with a set match over every mermaid block in every markdown file, in both directions — a block with no generator fails, and a registered generator with no block fails too. Verified both go red. 282 tests green locally. No CI in this repo, so that is one local run.
There was a problem hiding this comment.
Copilot review overview
🟢 Approval recommended
The documentation matches generator output, and the tests cover both stale and missing diagrams.
Review effort: Balanced
Findings: None
What changed in this PR
Adds the missing pre-implementation specification diagram and ensures documented Mermaid diagrams match generated output bidirectionally.
Changes:
- Adds the plain
diagram()output before the strategy comparison. - Validates every documented Mermaid block against registered generators and vice versa.
- Clarifies quickstart diagram ordering.
| File | Description |
|---|---|
README.md |
Adds and explains the specification diagram. |
tests/test_greeting.py |
Adds bidirectional Mermaid documentation checks. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The README's pitch is you see the design before the code exists. The only mermaid block it drew was
diff_diagram()— two implementations already bound. The picture with nothing implemented was described in prose and never shown, so the before/after that makes the claim visual was missing its "before".This adds the plain
diagram()block, generated, directly above the diff block — same graph, bare boxes.The check that should have caught it
test_the_readme_mermaid_block_is_the_diagram_the_code_emitsasserted one block by name, at a time when the README contained exactly one. That is this repo's recurring defect — a check narrower than its claim — and here it was blind in a way worth naming: a one-directional check cannot see a missing picture, because every block that is present passes.Replaced with a set match over every mermaid block in every markdown file, both directions:
_drawn()with no block in the docs → failBoth verified to go red (renamed a node in the README; deleted the new block).
Notes
examples.greetingprints them in the order the README shows.