docs(backlog): two follow-ups from the 2026-05-30 merge session#385
docs(backlog): two follow-ups from the 2026-05-30 merge session#385flyingrobots wants to merge 1 commit into
Conversation
Bad code: PLATFORM_wesley-gen-test-crate-builder-duplication.md Four consumer-crate builder fns in tests/generation.rs each ship ~40 lines of overlapping Cargo.toml/lib.rs/dep-declaration boilerplate. The PR #383 speedup work had to touch all four identically when threading CARGO_TARGET_DIR through; every future harness change pays the same cost. Proposes a single ConsumerCrateBuilder so a fifth generated-output path is a ~5-line call instead of a ~40-line copy. Cool idea: PLATFORM_prettier-before-cargo-hook-ordering.md The pre-commit hook still runs the expensive cargo verify step before the cheap prettier + markdownlint step. PR #383's auto-stage fix removed the worst case (no more abort+manual-restage), but the ordering itself means markdown-only commits still pay the cold cargo cost when prettier could have decided the hook can short- circuit. Proposes flipping prettier ahead of cargo. Neither blocks 0025 Phase 2; both are PLATFORM cleanup that pays for itself the first time anyone touches the same paths again.
📝 WalkthroughWalkthroughTwo backlog documentation files added: one identifying duplicated consumer-crate builder boilerplate across wesley-gen tests and proposing centralization; another proposing pre-commit hook reordering to execute fast checks before expensive cargo verification, with risk assessment and expected commit-type deltas. ChangesBacklog Documentation
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Suggested labels
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@docs/method/backlog/bad-code/PLATFORM_wesley-gen-test-crate-builder-duplication.md`:
- Around line 54-55: The statement claiming "Not a behavior change" about
removing the PID segment from the crate-dir layout is incorrect; update the
documentation text that describes crate path construction (the "crate-dir layout
(drop the PID segment; use only the label/purpose)" note and the related
sentences around it) to say this is behavior-preserving only if
uniqueness/isolation invariants are maintained (e.g., deterministic unique
labeling or explicit tempdir guarantees); explicitly call out that dropping the
PID changes crate path semantics and may affect test isolation/collision under
parallel runs or label reuse, and apply the same rewording to the other
occurrence covering the same crate path/PID change (the block referenced at the
other occurrence).
- Around line 13-16: Remove the unstable "line ~..." annotations and keep only
stable symbol references: keep the function names write_basic_generated_crate,
write_consumer_smoke_crate, write_contract_host_smoke_crate,
write_optic_binding_smoke_crate (optionally append a permalink/commit hash if
you need a fixed anchor), i.e., edit the list entries to drop the line numbers
and replace them with just the function names (or function name + permalink),
and scan the surrounding text for any other hard-coded source line pointers to
remove or replace similarly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 91f8adbc-b185-42ae-b0a1-99d43c3c2f5f
📒 Files selected for processing (2)
docs/method/backlog/bad-code/PLATFORM_wesley-gen-test-crate-builder-duplication.mddocs/method/backlog/cool-ideas/PLATFORM_prettier-before-cargo-hook-ordering.md
| - `write_basic_generated_crate` (line ~944) | ||
| - `write_consumer_smoke_crate` (line ~167) | ||
| - `write_contract_host_smoke_crate` (line ~602) | ||
| - `write_optic_binding_smoke_crate` (line ~1018) |
There was a problem hiding this comment.
Replace hard-coded source line numbers with stable symbol references.
These line ~... pointers will drift and become stale quickly. Keep function names (and optionally permalink commits) but drop mutable line-number references.
As per coding guidelines, "Documentation accuracy matters ... Flag factual errors and stale cross-references."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@docs/method/backlog/bad-code/PLATFORM_wesley-gen-test-crate-builder-duplication.md`
around lines 13 - 16, Remove the unstable "line ~..." annotations and keep only
stable symbol references: keep the function names write_basic_generated_crate,
write_consumer_smoke_crate, write_contract_host_smoke_crate,
write_optic_binding_smoke_crate (optionally append a permalink/commit hash if
you need a fixed anchor), i.e., edit the list entries to drop the line numbers
and replace them with just the function names (or function name + permalink),
and scan the surrounding text for any other hard-coded source line pointers to
remove or replace similarly.
| - the crate-dir layout (drop the PID segment; use only the | ||
| label/purpose) |
There was a problem hiding this comment.
“Not a behavior change” is factually too strong after removing PID path entropy.
Dropping the PID segment changes crate path construction semantics. Under parallel tests or label reuse, this can affect isolation/collision behavior. Reword this as behavior-preserving only if uniqueness/isolation invariants are explicitly retained (e.g., deterministic unique labeling or tempdir guarantees).
As per coding guidelines, "Documentation accuracy matters — especially anything touching determinism guarantees ... Flag factual errors..."
Also applies to: 60-62
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@docs/method/backlog/bad-code/PLATFORM_wesley-gen-test-crate-builder-duplication.md`
around lines 54 - 55, The statement claiming "Not a behavior change" about
removing the PID segment from the crate-dir layout is incorrect; update the
documentation text that describes crate path construction (the "crate-dir layout
(drop the PID segment; use only the label/purpose)" note and the related
sentences around it) to say this is behavior-preserving only if
uniqueness/isolation invariants are maintained (e.g., deterministic unique
labeling or explicit tempdir guarantees); explicitly call out that dropping the
PID changes crate path semantics and may affect test isolation/collision under
parallel runs or label reuse, and apply the same rewording to the other
occurrence covering the same crate path/PID change (the block referenced at the
other occurrence).
1 participant
Summary
Two backlog cards that fell out of patterns I noticed during the PR #382 + #383 + jedit #33 merge session but didn't act on. Both are PLATFORM cleanup; neither blocks Echo 0025 Phase 2.
Test plan
Summary by CodeRabbit