Adopt plaintext fences for prose output (#161) - #176
Conversation
|
Important Review skippedNo new commits to review since the last review. ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughUpdate three documentation output fences to ChangesDocumentation and spelling policy
Possibly related PRs
Suggested labels: Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 19 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (19 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@typos.toml`:
- Around line 37-39: Remove the duplicate [default.extend-words] table
declaration in typos.toml, retaining exactly one header before the ASO entry.
Update the generation behavior in scripts/typos_rollout.py if necessary so it
emits one table and validates the rendered TOML with tomllib.loads.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 9e421ee5-0f5d-41e6-bba1-255e69a9da78
📒 Files selected for processing (3)
docs/lading-design.mddocs/users-guide.mdtypos.toml
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
leynos/cuprum(auto-detected)leynos/cmd-mox(auto-detected)leynos/shared-actions(auto-detected)
|
@coderabbitai resume |
✅ Action performedReviews resumed. |
Replace every remaining `text` fence in project documentation with `plaintext`, which is the documented identifier for non-code output. Correct the existing `artefact` spelling in the design document so the Markdown quality gate passes under the configured en-GB policy.
Synchronize the generated Oxford-spelling policy with the current authority.
0390e0d to
cff9ccc
Compare
|
@coderabbitai review |
✅ Action performedReview finished.
|
Post-rebase reconciliation with main's #176 (plaintext prose fences): convert the two publish-plan example fences in the users' guide from `text` to `plaintext` to match the repository convention, and drop a duplicate blank line that the entity merge left before the new "CLI context loading" subsection in the developers' guide (MD012).
Post-rebase reconciliation with main's #176 (plaintext prose fences): convert the two publish-plan example fences in the users' guide from `text` to `plaintext` to match the repository convention, and drop a duplicate blank line that the entity merge left before the new "CLI context loading" subsection in the developers' guide (MD012).
* Tidy minor structural overlaps in plan/CLI/publish paths 13a: collapse _format_crates_section into the single section renderer. The bespoke function duplicated _append_section plus an empty message and a fixed formatter. The renderer is now _render_section, a query returning the section lines (which also keeps the argument count within the lint ceiling); _format_plan composes it for every section. The historical public append_section (listed in __all__) is retained as a thin backwards-compatible shim that extends the passed-in list by delegating to _render_section, so existing importers keep working while new code uses render_section. 13b: catch PublishPreflightError alone in the live pipeline, with a comment noting that PublishError subclasses it, so the redundant tuple is not reintroduced by accident. 13c: deduplicate the load-workspace-and-run block in cli._run_with_context. Configuration scope is resolved first (a nullcontext when already loaded), then a single block runs the command. 13d: document that utils.commands.LADING_CATALOGUE is staged but not yet wired into the execution path, in both the module docstring and the developers' guide, with a pointer to the roadmap Phase 5 steps that will wire it. Tests: property test pins the render_section header/item/empty-message invariants, unit tests pin the append_section shim (list mutation, empty no-op, parity with render_section), syrupy snapshots lock the fully rendered publish plan with and without publishable crates, and a behavioural test pins identical downstream behaviour for both _run_with_context branches. Closes #107 * Document publish-plan output and test the CLI config path Address two review warnings on the publish-plan/CLI work. Users' guide: document the publish-plan summary that `lading publish` prints on completion — the crates-to-publish list, the skipped-crate sections, and the `Crates to publish: none` empty state — which was previously covered only by tests and the developers' guide. CLI-boundary test: add `test_publish_via_app_matches_across_config_branches`, which drives `cli.app` (the public command boundary) for both `_run_with_context` configuration branches. `cli.main` always installs configuration before dispatch, so the disk-loaded branch is reachable publicly only through `cli.app` without an active scope. The test asserts identical downstream behaviour (same workspace, equal configuration) while pinning the provenance difference: the disk branch reloads a fresh config object; the pre-loaded branch reuses the active one. The white-box `test_run_with_context_branches_behave_identically` is retained. * Add assertion messages and link the roadmap reference Address review comments on the publish-plan/CLI test and docs. Tests: give the previously bare assertions in the render_section unit tests focused diagnostic messages — the default-formatting, empty-section, empty-message, the four property invariants, and both snapshot assertions — plus the `minimal_config.exists()` setup check in the CLI config-branch test. Each message states the contract being verified so a failure explains itself. Docs: turn the code-formatted `docs/roadmap.md` reference in the LADING_CATALOGUE / Phase 5 text into a relative Markdown link, `[Phase 5 runner-migration steps](./roadmap.md)`, matching the guide's existing relative-link style. * Add assertion messages and document plan/CLI helpers Address review comments on the publish-plan/CLI work. Tests: give the config-branch tests (`test_run_with_context_branches_ behave_identically`, `test_publish_via_app_matches_across_config_branches`) and the remaining section-helper tests diagnostic assertion messages, so a failure names the invariant it checks (result, call count, workspace root, configuration object, or workspace graph). No test logic changes. Users' guide: move the `Crates to publish: none` empty-state sentence so it introduces the empty-case example rather than the populated one. Developers' guide: document the `publish_plan` section helpers (`render_section`, `append_section`, `format_plan`) under the extracted publish modules, and add a "CLI context loading" subsection describing `_run_with_context` and its single-branch configuration resolution. * Adopt plaintext fences and tidy blank line Post-rebase reconciliation with main's #176 (plaintext prose fences): convert the two publish-plan example fences in the users' guide from `text` to `plaintext` to match the repository convention, and drop a duplicate blank line that the entity merge left before the new "CLI context loading" subsection in the developers' guide (MD012). * Drop duplicate format_plan skipped-sections test `test_format_plan_formats_skipped_sections` in test_publish_formatting.py was a behavioural duplicate of the canonical test in tests/unit/publish/test_formatting_helpers.py (identical assertions on `format_plan` skipped-section rendering; only the crate-construction plumbing differed). Remove it and the imports it alone used (`collections.abc`, `publish_plan`, `_CrateSpec`, `WorkspaceCrate`); the file keeps its unique `test_format_preparation_summary` coverage. * Correct LADING_CATALOGUE migration reference to Phase 5.2 The catalogue-wiring migration target was misrouted to the low-level `subprocess_runner.py` (and cited a roadmap step that does not exist). The catalogue's `scoped(allowlist=…)` enforcement is wired in by roadmap Phase 5.2 "Publish Execution Migration", which rewires `publish_execution.py`'s command execution (the roadmap's `_invoke_via_subprocess()` step; `_invoke` today) onto cuprum. Point the `utils/commands.py` module docstring and the developers' guide note at that step, keeping the "staged, not yet wired" caveat. * Expand the publish_plan module docstring The module summarized itself in one line, leaving its purpose and its place in the publish flow implicit. Describe the two responsibilities it backs — resolving a `PublishPlan` and rendering that plan for operators — and record the call sites: `publish.run` builds the plan before staging and formats it at the end, and `lading.cli` prints that return value verbatim (so the text is user-facing output, though `cli.py` never imports this module). Note that syrupy snapshots pin the rendered summary, so output changes must update them and the users' guide. --------- Co-authored-by: leynos <leynos@rohga>
Summary
This branch replaces the remaining non-code
textfences withplaintext, asrequired by the documentation style guide. It also corrects one existing
Oxford-spelling violation and refreshes the generated spelling configuration
needed for the Markdown validation gate.
It resolves the follow-up from PR #127
and the issue discussion.
Closes #161.
Review walkthrough
artefactspelling correction.Validation
make markdownlint: passed.make nixie: passed.coderabbit review --agent: attempted after each successful validation milestone; every foreground attempt stopped during CodeRabbit setup or summarization without a final result, finding, or rate-limit response.coderabbit doctorreported 9 checks passed.