docs(design): describe what shipped - #40
Conversation
Five statements in the design spec do not match the tree, and three of them describe a plan the implementation improved on rather than a mistake. The error type was to be a small enum implementing `Display` and `From` by hand. No such impl exists: `ReadError` derives comparison and debug and nothing else, and every path renders it through one `describe` function into the fixed vocabulary the section above defines. That is better than the plan, because the vocabulary belongs to the tool rather than to the type, and a `Display` impl would have invited a second way to render it. The error vocabulary was said to travel in two message shapes. There are three: the `--ignore-file` read carries it too, in both implementations, and has since before the paragraph naming the other two. The CI section names a `lint` job. There is none and there never has been -- the linting job is `pre-commit` -- so a reader checking the list against the workflow finds four of five and one dangling id. The `parity` bullet omits that job's first step, the CLI tier over both binaries on three platforms, and describes one fresh seed per run where it takes a range derived from the run number. The mirror paragraph still hangs its argument on a generated force-push. The push appends a signed commit and has no force path, which the workflow and the script both say at length, so the argument now names the mechanism that exists. It survives the change unaltered: an irreversible act behind a generated commit is the objection. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Code Review by Qodo🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)
Great, no issues found!Qodo reviewed your code and found no material issues that require reviewTip of the day💡 Did you know, you can add REVIEW.md to your repo root and Qodo follows it on every PR |
PR Summary by QodoAlign Rust port design record with shipped implementation
AI Description
Diagram
High-Level Assessment
Files changed (1)
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #40 +/- ##
=======================================
Coverage 87.37% 87.37%
=======================================
Files 3 3
Lines 689 689
=======================================
Hits 602 602
Misses 87 87 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
🟢 Approval recommended
The changes are documentation-only and the updated statements are consistent with the current code and workflows.
Pull request overview
Updates the Rust port design record (docs/rust-port-design.md) so its CI, error-handling, and mirror-generation descriptions match the current repository implementation and workflows.
Changes:
- Corrects the mirror mechanism wording from “generated force-push” to “generated commit” (consistent with
createCommitOnBranch/no-force-path behavior). - Updates error-handling documentation to reflect the
ReadError+describe(...) -> &'static strfixed-vocabulary approach (noDisplay/Fromimpls). - Fixes CI job references and parity description (pre-commit job name; parity runs CLI tier on 3 OSes plus fuzzing over fixed + run-derived seed ranges).
File summaries
| File | Description |
|---|---|
| docs/rust-port-design.md | Aligns the design documentation with shipped behavior for mirrors, error rendering, CI job naming, and parity/fuzzing scope. |
Review details
- Files reviewed: 1/1 changed files
- Comments generated: 0
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
What changes, and why
Five statements in
docs/rust-port-design.mdthat do not match the tree. Three describe a plan the implementation improved on rather than a mistake, which is worth saying because the document is a design record and the improvement is the interesting part.The error type (
:232). "A small error enum implementingDisplayandFromby hand." No such impl exists:ReadErrorderivesDebug, Clone, Copy, PartialEq, Eqand nothing more, and every path renders it through onedescribe(path, error) -> &'static strinto the fixed vocabulary the section above defines. That is better than the plan — the vocabulary belongs to the tool rather than to the type, and aDisplayimpl would have invited a second way to render it — so the paragraph now says so.The message shapes (
:186). "The two messages carrying it" — there are three. The--ignore-fileread carries the same vocabulary, in both implementations (unwrap.py:1089,src/cli.rs:565), and has since before the paragraph naming the other two.The
lintjob (:236). There is none, and no revision of the workflow has had one.CI.yml's job ids arepre-commit test coverage hook action mirror-identity rust-lint rust-test rust-test-stable parity build— the linting job ispre-commit, so a reader checking the list finds four of five and one dangling id.The
paritybullet (:240). It omits that job's first and primary step — the CLI tier over both binaries, on all three platforms — and says "one fresh seed per run" where the job takes a range:The mirror argument (
:90). Still hung on "a generated force-push". The push appends a signed commit throughcreateCommitOnBranchand has no force path, whichmirrors.ymlandscripts/push_mirror.pyboth state at length. The argument survives the correction unaltered — an irreversible act behind a generated commit is the objection — so only the mechanism's name changes.Corpus
The corpus is the specification, and both implementations answer to it. Tick what applies.
corpus/pins the new behavior. The case was written first and failed first.Checks
make checkpasses, ormake testdoes and this touches no Rust.make tidy, plusunwrap-markdown-prose-py --fail-on-change docs/rust-port-design.md. One Markdown file.