Add shared cross-plugin rename-symbol fixtures and tests (test-support)#90
Merged
Add shared cross-plugin rename-symbol fixtures and tests (test-support)#90
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
WalkthroughConsolidate rename-symbol test infrastructure: extract shared e2e harness and routing, centralise plugin contract fixtures behind a feature-gated test-support API, add BDD contract tests for rope and rust‑analyzer, extend refactor dispatch rollback tests, and enforce refusal of plugin success responses that lack diff output. (48 words) Changes
Sequence Diagram(s)sequenceDiagram
participant CLI as CLI (weaver)
participant Daemon as Daemon (FakeDaemon)
participant Plugin as Plugin runtime
participant FS as Filesystem
CLI->>Daemon: connect to --daemon-socket and send refactor request (JSONL)
Daemon->>Plugin: route request (automatic or explicit provider)
Plugin-->>Daemon: respond (diff stdout OR non-diff success OR error)
Daemon-->>CLI: stream stdout/stderr JSONL lines and exit status
alt Plugin returned diff
CLI->>FS: apply diff to files
else Plugin success but no diff
CLI-->>FS: abort and leave files unchanged
end
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 8 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (8 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![codescene-delta-analysis[bot]](https://avatars.githubusercontent.com/in/53921?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Reviewer's GuideCentralizes rename-symbol contract test fixtures in a feature-gated weaver-plugins test_support module and wires them into rope, rust-analyzer, daemon, and e2e/CLI tests, while adding rollback/routing coverage and an ExecPlan plus roadmap/users-guide updates. Sequence diagram for non-diff success response and rollback behavioursequenceDiagram
participant Testrunner
participant RefactorWorld
participant RefactorHandler
participant StubRuntime
Testrunner->>RefactorWorld: given_non_diff_success()
RefactorWorld->>RefactorWorld: runtime_mode = EmptySuccess
Testrunner->>RefactorWorld: when_refactor_executes()
RefactorWorld->>RefactorHandler: execute act refactor command
RefactorHandler->>StubRuntime: execute(rename request)
StubRuntime-->>RefactorHandler: PluginResponse success Empty
RefactorHandler->>RefactorHandler: detect non-diff success
RefactorHandler->>RefactorHandler: enforce rollback invariant
RefactorHandler-->>RefactorWorld: failure outcome, no diff applied
RefactorWorld-->>Testrunner: assertions on stderr and filesystem
Class diagram for capability test_support fixtures and runtime changesclassDiagram
class capability {
}
class test_support {
+RenameSymbolRequestFixture
+RenameSymbolResponseFixture
+rename_symbol_request_fixtures() Vec~RenameSymbolRequestFixture~
+rename_symbol_response_fixtures() Vec~RenameSymbolResponseFixture~
}
class RenameSymbolRequestFixture {
+name: &'static str
+request: RenameSymbolRequest
}
class RenameSymbolResponseFixture {
+name: &'static str
+response: PluginResponse
}
class RenameSymbolRequest {
}
class PluginResponse {
}
class weaver_plugins_lib {
<<module re-export>>
+RenameSymbolRequestFixture
+RenameSymbolResponseFixture
+rename_symbol_request_fixtures()
+rename_symbol_response_fixtures()
}
class RuntimeMode {
<<enum>>
DiffSuccess
RuntimeError
MalformedDiff
EmptySuccess
}
class StubRuntime {
+mode: RuntimeMode
+execute(request: RenameSymbolRequest) PluginResponse
}
capability <|-- test_support
test_support --> RenameSymbolRequestFixture
test_support --> RenameSymbolResponseFixture
RenameSymbolRequestFixture --> RenameSymbolRequest
RenameSymbolResponseFixture --> PluginResponse
weaver_plugins_lib --> capability
weaver_plugins_lib --> test_support
RuntimeMode <.. StubRuntime
StubRuntime --> PluginResponse
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
leynos
changed the title
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Owner
Author
|
@coderabbitai Please suggest a fix for this issue and supply a prompt for an AI coding agent to enable it to apply the fix: crates/weaver-e2e/tests/refactor_rope_cli_snapshots.rs Comment on file fn refactor_automatic_rope_routing_cli_snapshot() {
let daemon = FakeDaemon::start(1).expect("fake daemon should start");
let endpoint = daemon.endpoint();
let command_string = String::from(
"weaver --daemon-socket tcp://<daemon-endpoint> --output json act refactor --refactoring rename --file src/main.py new_name=renamed_symbol offset=4",
);
let mut command = Command::new(weaver_binary_path());
let output = command
.args([
"--daemon-socket",
endpoint.as_str(),
"--output",
"json",
"act",
"refactor",
"--refactoring",
"rename",
"--file",
"src/main.py",
"new_name=renamed_symbol",
"offset=4",
])
.output()
.expect("command should execute");
let transcript = output_to_transcript(command_string, &output, daemon.requests());
daemon.join();
assert_debug_snapshot!("refactor_automatic_rope_routing", transcript);
}❌ New issue: Code Duplication |
Owner
Author
|
@coderabbitai Please suggest a fix for this issue and supply a prompt for an AI coding agent to enable it to apply the fix: crates/weaver-e2e/tests/refactor_rust_analyzer_cli_snapshots.rs Comment on file fn refactor_automatic_rust_routing_cli_snapshot() {
let daemon = FakeDaemon::start(1).expect("fake daemon should start");
let endpoint = daemon.endpoint();
let command_string = String::from(
"weaver --daemon-socket tcp://<daemon-endpoint> --output json act refactor --refactoring rename --file src/main.rs new_name=renamed_name offset=3",
);
let mut command = Command::new(weaver_binary_path());
let output = command
.args([
"--daemon-socket",
endpoint.as_str(),
"--output",
"json",
"act",
"refactor",
"--refactoring",
"rename",
"--file",
"src/main.rs",
"new_name=renamed_name",
"offset=3",
])
.output()
.expect("command should execute");
let transcript = output_to_transcript(command_string, &output, daemon.requests());
daemon.join();
assert_debug_snapshot!("refactor_automatic_rust_routing", transcript);
}❌ New issue: Code Duplication |
Owner
Author
|
@coderabbitai Please suggest a fix for this issue and supply a prompt for an AI coding agent to enable it to apply the fix: crates/weaver-plugins/src/capability/test_support.rs Comment on file pub const fn new(
name: &'static str,
request: PluginRequest,
expected_error_fragment: Option<&'static str>,
) -> Self {
Self {
name,
request,
expected_error_fragment,
}
}❌ New issue: Code Duplication |
Owner
Author
|
@coderabbitai Please suggest a fix for this issue and supply a prompt for an AI coding agent to enable it to apply the fix: crates/weaverd/src/dispatch/act/refactor/rollback_tests.rs Comment on file fn successful_non_diff_response_leaves_target_file_unchanged() {
let outcome = run_failure_case(RollbackRuntime {
resolution: selected_resolution(SelectedResolution {
capability: weaver_plugins::CapabilityId::RenameSymbol,
language: "python",
provider: "rope",
selection_mode: super::resolution::SelectionMode::Automatic,
requested_provider: None,
}),
execute_result: ExecuteResult::Success(PluginResponse::success(PluginOutput::Analysis {
data: serde_json::json!({ "unexpected": true }),
})),
});
assert_eq!(outcome.status, 1);
assert_eq!(
outcome.content,
original_content_for(std::path::Path::new("notes.py"))
);
assert!(outcome.stderr.contains("did not return diff output"));
}❌ New issue: Code Duplication |
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
leynos
changed the title
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Owner
Author
|
@coderabbitai Please suggest a fix for this issue and supply a prompt for an AI coding agent to enable it to apply the fix: crates/weaver-e2e/tests/refactor_rust_analyzer_cli_snapshots.rs Comment on file fn refactor_rust_analyzer_actuator_isolation_cli_snapshot() {
run_refactor_snapshot(
"refactor_rust_analyzer_actuator_isolation",
"weaver --daemon-socket tcp://<daemon-endpoint> --output json act refactor --provider rust-analyzer --refactoring rename --file src/main.rs new_name=renamed_name offset=3",
&[❌ New issue: Code Duplication |
This comment was marked as resolved.
This comment was marked as resolved.
leynos
changed the title
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Add a comprehensive ExecPlan document outlining unit, behavioural, and end-to-end test coverage for the `rename-symbol` capability. This living document details milestones, constraints, risks, progress, decisions, and plans spanning shared contract fixtures, plugin-level tests, daemon routing tests, e2e tests, and documentation updates, ensuring cross-plugin contract conformance and rollback guarantees. Co-authored-by: devboxerhub[bot] <devboxerhub[bot]@users.noreply.github.com>
…overage - Introduce a feature-gated shared test-support module in `weaver-plugins` providing canonical request and response fixtures for the `rename-symbol` capability. - Extend `rope` and `rust-analyzer` plugin crates to consume these shared fixtures for unit-level contract validation and behavior-driven tests, improving consistency. - Enhance daemon routing tests to cover automatic provider resolution, explicit provider mismatches refusals, and rollback scenarios ensuring failure paths leave files unchanged. - Add CLI end-to-end snapshot tests to verify automatic routing and structured refusal outputs for Python and Rust rename flows. - Update documentation in users-guide.md and roadmap.md to reflect completed coverage milestone 5.2.5. This improves test consistency and coverage for Python and Rust rename-symbol workflows, ensuring plugin conformance and robustness under the new capability architecture. Co-authored-by: devboxerhub[bot] <devboxerhub[bot]@users.noreply.github.com>
- Consolidate repeated test setup and command execution logic into reusable helper functions for refactor snapshots. - Replace direct CLI command string assembly with parameterized command argument vectors. - Update rename-symbol contract fixtures and validation calls to use payload access consistently. - Extract common rollback test logic and assertions for cleaner test cases. - General improvements to test code clarity, reducing duplication across weaver-e2e and plugin test suites. Co-authored-by: devboxerhub[bot] <devboxerhub[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@crates/weaver-e2e/tests/test_support/refactor_routing.rs`:
- Around line 62-67: Replace the verbose argument.as_str().map_or_else(||
panic!(...), |value| value) with the idiomatic expectation call: use
argument.as_str().expect("daemon request arguments must all be JSON strings")
inside the .map closure so the code becomes .map(|argument|
argument.as_str().expect("daemon request arguments must all be JSON strings"));
this keeps the same fail-fast behavior but follows the `.expect()` guideline.
🪄 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
Run ID: 84989876-760e-47c5-ab7e-ff150e3c9df2
📒 Files selected for processing (1)
crates/weaver-e2e/tests/test_support/refactor_routing.rs
- Add comprehensive domain and operation listings in top-level --help output. - Implement client-side domain validation before configuration loading and daemon startup. - Provide actionable, consistent error messaging and suggestions for unknown domains and operations. - Standardize exit codes and output streams for --version and --help. - Include detailed documentation updates and extensive unit, integration, and behavioural tests covering new CLI guidance and error paths. This change enhances user experience by making command discovery clearer and early validation more helpful, reducing daemon dependency for invalid input cases and unifying error presentation across the CLI. Co-authored-by: devboxerhub[bot] <devboxerhub[bot]@users.noreply.github.com>
🧩 Analysis chain🏁 Script executed: Unrecoverable error: Failed to clone repository
If the branch has been updated since then, please trigger a fresh review and I will re-evaluate. If the prompts have not yet been applied, the outstanding work from the previous response remains: Remaining work (from previous analysis)
The AI coding-agent prompt to address all three in one pass was provided in the previous reply. Apply it, then re-trigger the review. ✅ Actions performedReview triggered.
|
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
leynos
changed the title
There was a problem hiding this comment.
Actionable comments posted: 45
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
docs/execplans/phase-1-implement-the-socket-listener-in-weaverd.md (1)
171-223:⚠️ Potential issue | 🟡 MinorRenumber the ordered list items in “Concrete Steps”.
Replace repeated
1.markers with sequential numbering to keep the document
compliant with repository Markdown style rules and to make future diffs clearer.Triage:
[type:docstyle][type:syntax/md]As per coding guidelines, “Use
-as the first level bullet and renumber lists
when items change in documentation”.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/execplans/phase-1-implement-the-socket-listener-in-weaverd.md` around lines 171 - 223, The "Concrete Steps" ordered list uses repeated "1." markers; renumber the items sequentially so the list follows proper Markdown ordered-list formatting: locate the "Concrete Steps" section (the block that currently lists steps starting with repeated "1.") and replace the repeating "1." entries with ascending numbers (1., 2., 3., ...) so each step is uniquely numbered and the document follows the repository Markdown style guidelines.docs/execplans/phase-1-jsonl-request-dispatch-loop.md (1)
17-37: 🧹 Nitpick | 🔵 TrivialRenumber the ordered list items sequentially in the source.
The "Design Decisions" list uses
1.for all five items. While Markdown renderers auto-number such lists correctly, explicit sequential numbering (1, 2, 3, 4, 5) improves source readability and maintainability.Triage:
[type:syntax/md]📝 Proposed fix
## Design Decisions 1. **Modular dispatch architecture**: Create a new `dispatch` module with clear separation of concerns (errors, request parsing, response writing, routing, and connection handling). Each file remains under 400 lines per AGENTS.md. -1. **Minimum viable product (MVP) placeholder handlers**: The `get-definition` +2. **Minimum viable product (MVP) placeholder handlers**: The `get-definition` operation in the `observe` domain is fully implemented with LSP integration. Other known operations (`find-references`, `grep`, `diagnostics`, `call-hierarchy` in `observe`; all operations in `act` and `verify`) return "not implemented" responses with exit status 1 while backend wiring is completed. -1. **Structured error responses**: All errors (malformed JSONL, unknown domain, +3. **Structured error responses**: All errors (malformed JSONL, unknown domain, unknown operation) are written to stderr as structured messages before the terminal exit message. -1. **Case-insensitive domain matching**: Domain names (`observe`, `act`, +4. **Case-insensitive domain matching**: Domain names (`observe`, `act`, `verify`) are matched case-insensitively for robustness. -1. **Re-use existing transport infrastructure**: Build on the existing +5. **Re-use existing transport infrastructure**: Build on the existing `ConnectionHandler` trait and `read_request_line` helper from the transport module.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/execplans/phase-1-jsonl-request-dispatch-loop.md` around lines 17 - 37, The ordered list under "Design Decisions" uses "1." for every item; update the five list markers so they are sequential (1., 2., 3., 4., 5.) to improve source readability—specifically change the leading "1." tokens for the items starting with "**Modular dispatch architecture**", "**Minimum viable product (MVP) placeholder handlers**", "**Structured error responses**", "**Case-insensitive domain matching**", and "**Re-use existing transport infrastructure**" to 1 through 5 respectively, preserving the rest of the Markdown and wording.docs/adr-001-plugin-capability-model-and-act-extricate.md (1)
150-165:⚠️ Potential issue | 🟡 MinorRestore sequential numbering in the migration plan.
Renumber the ordered list as
1.through4.so the phase sequence is explicit in rendered output and review diffs.Triage:
[type:docstyle]As per coding guidelines: "
docs/**/*.md: Use-as the first level bullet and renumber lists when items change in documentation."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/adr-001-plugin-capability-model-and-act-extricate.md` around lines 150 - 165, The ordered list in the migration plan must use sequential numbering 1.–4.; update the four top-level items ("Capability scaffolding", "Python provider rollout", "Rust provider rollout", "Hardening and expansion") so their list markers are "1." through "4." respectively (replace the repeated "1." markers), ensuring the rendered markdown and diffs show the explicit phase sequence and follow the docs styling rule for renumbered ordered lists.docs/execplans/2-3-1-validate-domains-client-side-before-daemon-startup.md (1)
431-437:⚠️ Potential issue | 🟠 MajorFix broken footnote definitions in References.
Replace escaped labels and the percent-encoded roadmap link with valid GitHub-flavoured footnotes; the current forms do not render as expected.
Proposed fix
-\[^level3\]: -[docs/ui-gap-analysis.md Level 3](../ui-gap-analysis.md#level-3--unknown-domain-weaver-bogus-something) -\[^level10\]: -[docs/ui-gap-analysis.md Level 10](../ui-gap-analysis.md#level-10--error-messages-and-exit-codes) - -[^roadmap]: %5Bdocs/roadmap.md%5D(../roadmap.md) +[^level3]: [docs/ui-gap-analysis.md Level 3](../ui-gap-analysis.md#level-3--unknown-domain-weaver-bogus-something) +[^level10]: [docs/ui-gap-analysis.md Level 10](../ui-gap-analysis.md#level-10--error-messages-and-exit-codes) +[^roadmap]: [docs/roadmap.md](../roadmap.md)Triage:
[type:syntax/md]As per coding guidelines: "
docs/**/*.md: Use GitHub-flavoured numeric footnotes referenced as[^1]in documentation."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/execplans/2-3-1-validate-domains-client-side-before-daemon-startup.md` around lines 431 - 437, The footer contains escaped footnote labels (\[^level3\], \[^level10\]) and a percent-encoded roadmap link ([^roadmap]) that don't render; replace them with GitHub-flavoured numeric footnotes (e.g. [^1], [^2], [^3]) and update their corresponding definitions in the References section so each numeric label maps to the proper target (e.g. replace the percent‑encoded roadmap URL with a normal relative link to ../roadmap.md); also update all in-text references elsewhere in the document from the old escaped names to the new numeric footnote labels to keep links consistent.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@agent-skill/weaver/SKILL.md`:
- Around line 175-176: Update the example safe-rename invocation in SKILL.md so
it is copy-pasteable: replace the standalone backticked command string starting
with "act refactor --refactoring rename --file src/main.py new_name=build_index
offset=123" with the full executable prefix "weaver --output json act refactor
--refactoring rename --file src/main.py new_name=build_index offset=123" so the
command actually runs from a shell; ensure the modified backticked line in the
doc exactly matches the new invocation.
In `@AGENTS.md`:
- Around line 228-229: The sentence uses second-person pronoun in the phrase
"when you own the trait definitions"; change that clause in the line mentioning
`newt-hype`, tuple structs, and `the-newtype` to neutral project language (e.g.,
"when the trait definitions are owned by the crate" or "when the crate owns the
trait definitions") so the sentence remains normative without "you".
In `@docs/adr-002-query-language-for-feature-extraction-in-weaver.md`:
- Around line 124-127: Replace the four leading "1." lines in the migration
steps with a first-level hyphen list (use "-" as the first-level bullet for
docs) and ensure the items are sequential/ordered as written; also correct the
possessive phrase "Weaver internal" to "Weaver’s internal" (update the phrase in
the line "Add execution over Tree-sitter-backed structures with capture support"
or whichever line contains "Weaver internal") so the document uses "- " bullets
and the correct possessive form.
In `@docs/adr-003-sempai-semgrep-compatible-query-engine.md`:
- Around line 108-111: The ordered list in the "routing sequence" section
currently repeats `1.` at Line 108 and Line 110; update those list markers so
the sequence is numeric and sequential (e.g., change the second `1.` to `2.`)
and ensure the surrounding first-level bullets use `-` where required by the doc
guidelines; update the list items in the routing sequence (the repeated numbered
entries) to a proper 1., 2., ... ordering so markdown renders and lints
correctly.
- Around line 130-133: The ordered list in the "Migration plan" section
currently uses repeated "1." markers, making the step order ambiguous; update
that list to use sequential numbering (1., 2., 3., ...) or convert to top-level
"-" bullets per doc guidelines and renumber items accordingly so each step is
unambiguous; locate the repeated "1." tokens in the Migration plan block and
replace them with the correct sequential numbers.
In `@docs/adr-005-verification-trust-boundary.md`:
- Around line 92-95: The markdown list uses repeated "1." markers; renumber the
items sequentially (1., 2., 3.) so the three migration steps "List trusted
external tools and filesystem assumptions explicitly.", "Make proof harnesses
refer to the modelled assumptions by name.", and "Keep future-proof work inside
the documented boundary." are ordered 1–3 (or 1–4 if there is a fourth item
elsewhere); update those list markers accordingly so the list renders with
explicit ordered numbering instead of repeated "1." markers.
In `@docs/adr-006-plugin-execution-and-orchestration-strategy.md`:
- Around line 68-70: Add an explicit "Verified Kernel" statement in this ADR
that names the verified kernel components as: transaction ordering, path policy,
capability routing, refusal semantics, and bounded counters, and clearly
separates trusted/verified components (the kernel) from unverified
implementation components such as plugins, parsers, and OS/filesystem semantics;
update the paragraph around "The broker should own request validation..." to
state that the broker enforces the verified kernel boundaries (transaction
ordering, path policy, capability routing, refusal semantics, bounded counters)
while plugins remain untrusted implementation details that cannot alter commit
behaviour or kernel semantics.
- Around line 89-91: The three migration plan list items currently all start
with "1."; update them to be sequentially numbered "1.", "2.", "3.", and "4."
(specifically change the lines containing "Make broker validation and routing
explicit.", "Preserve the safety-harness handoff for accepted edits.", and "Add
tests that exercise refusal, success, and rollback paths.") so the markdown
renders as an ordered sequence and follows the docs style guideline for
docs/**/*.md.
In `@docs/building-an-error-recovering-parser-with-chumsky.md`:
- Around line 77-79: In the "Linting" ordered list, fix the numbering so the
three steps progress sequentially: change the second "1." before "Run the cheap
local checks (duplicates, arity, type holes)." to "2." and change the third "1."
before "Emit a constraint set and immediately feed it to the solver; conflicts
become diagnostics." to "3.", ensuring the list reads 1., 2., 3. and reflects
the intended progression.
- Around line 50-53: The ordered list under "Error recovery" currently uses
repeated `1.` markers; change them to consecutive numbers (`1.`, `2.`, `3.`) so
the steps are sequential: update the second item mentioning
`.labelled("expression")` to `2.` and the third item about returning
`Option<AstNode>` (tri-state nodes) to `3.` to preserve explicit ordering in the
document and follow docs/*.md numbering conventions.
In `@docs/complexity-antipatterns-and-refactoring-strategies.md`:
- Around line 91-96: The ordered list under the "Nesting" and "Shorthand
Discount" items is using repeated "1." markers which makes the sequence
ambiguous; update these list entries to use sequential numbering (1., 2., etc.)
and scan other updated ordered lists in the same file to renumber them
consistently so each item follows proper numeric order; specifically locate the
list items titled "Nesting" and "Shorthand Discount" and change their leading
markers to the correct successive numbers, and apply the same renumbering rule
to any other ordered lists you modified in this markdown.
In `@docs/contents.md`:
- Line 3: Reflow the long Markdown list entries (including the link text
"Architectural decision record (ADR) 001: Plugin capability model and `act
extricate`" and the other bulleted entries referenced at the review: lines 3, 6,
9, 11, 14, 17, and 73–74) so each paragraph or bullet line is wrapped to a
maximum of 80 columns; preserve the Markdown list markers, link syntax, and
inline code backticks, break long sentences into additional indented lines as
needed to maintain readability, and ensure no content or punctuation is lost
when wrapping.
In `@docs/documentation-style-guide.md`:
- Line 346: The footnote definition uses an escaped marker "\[^1\]:" which
prevents Markdown from resolving the reference; locate the escaped footnote
marker "\[^1\]:" in the documentation-style-guide content and replace it with a
proper GitHub-Flavored Markdown footnote marker "[^1]:" so the reference
resolves correctly (search for the literal string "\[^1\]:" and change it to
"[^1]:").
In `@docs/execplans/2-2-3-top-level-version-output.md`:
- Around line 33-60: Update the Markdown ordered lists in
docs/execplans/2-2-3-top-level-version-output.md so that each ordered-list uses
sequential numbering (1., 2., 3., ...) rather than repeated "1." — specifically
renumber the constraints, outcomes, validation, and commit sequence sections
changed in the diff and also fix the same pattern at the other mentioned ranges
(lines around 154-159, 448-455, 512-520). Ensure numbering is consistent and
sequential across each list block so the execution order is clear.
In `@docs/execplans/2-2-4-provide-contextual-domain-guidance.md`:
- Around line 391-394: The footnote definitions use escaped sequences (\[^1\]:
and \[^2\]:) so they don't render; replace the two occurrences of "\[^1\]:" and
"\[^2\]:" with standard GitHub-flavoured footnote definitions "[^1]:" and
"[^2]:" so the links to the UI gap-analysis anchors render as actual footnotes;
update the text in the document where the literal strings "\[^1\]:" and
"\[^2\]:" appear (search for those exact tokens) and save the file.
In `@docs/execplans/2-3-2-valid-operation-alternatives-for-unknown-operations.md`:
- Around line 470-475: Replace the escaped footnote definitions like \[^1\]:
\[^2\]: etc. with proper GitHub-flavored Markdown footnote definitions using
unescaped `[^n]:` on their own separate lines so references resolve and lint
passes; locate the escaped sequences shown in the diff (e.g., `\[^1\]:`,
`\[^2\]:`, `\[^3\]:`, `\[^4\]:`, `\[^5\]:`) and convert each to a plain `[^n]:`
footnote definition line matching the referenced text.
In `@docs/execplans/3-1-1-weaver-plugins-crate.md`:
- Around line 343-347: The markdown file contains many fenced code blocks using
bare ```; update each to include an explicit language identifier (e.g., ```sh
for shell commands like the shown make commands, ```rust for Rust snippets,
```json for JSON blobs, or ```plaintext for non-code text) so Markdown
validation is deterministic—replace the bare fences at the shown code blocks
(including the snippet with the make commands and the other ranges listed in the
comment) with the appropriate language-tagged fences throughout the document.
In `@docs/execplans/3-1-1a-plugin-for-rope.md`:
- Around line 329-336: The fenced code blocks that currently start with lines
like "set -o pipefail && make fmt 2>&1 | tee /tmp/3-1-1a-make-fmt.log" need
explicit language identifiers; change each triple-backtick fence to include the
appropriate language (e.g., ```sh for shell commands, ```rust for Rust snippets,
or ```plaintext for non-code text) for the blocks at the shown diff and also the
other affected ranges (340-344, 394-398, 402-410) so renderers and linters
correctly parse them.
In `@docs/execplans/3-1-1b-plugin-for-rust-analyzer.md`:
- Around line 108-110: Replace the first instance of the acronym "BDD" in the
sentence "Decision: keep all unit/BDD tests mock-based at adapter boundary."
with its expanded form "Behaviour-Driven Development (BDD)" so the sentence
reads "Decision: keep all unit/Behaviour-Driven Development (BDD) tests
mock-based at adapter boundary." Ensure only the first prose occurrence is
expanded and subsequent uses can remain as "BDD."
In `@docs/execplans/4-1-1-scaffold-sempai-core-and-sempai.md`:
- Around line 162-175: The ordered lists in the outcomes/acceptance sections use
repeated "1." markers; update them to explicit sequential numbering (1., 2., 3.,
...) so items are numbered correctly (e.g., the block listing RUSTDOCFLAGS,
public types, tests, make targets, docs update, and roadmap task should be
numbered 1–6), and make the same change for the other list referenced (lines
629–636) so both lists render with proper sequential numbers.
In `@docs/execplans/4-1-2-structured-diagnostics-with-stable-error-codes.md`:
- Around line 141-148: The ordered checklist in
docs/execplans/4-1-2-structured-diagnostics-with-stable-error-codes.md currently
shows repeated "1." for each completion criterion; update the checklist so the
items (the six criteria and the final make checks) are sequentially numbered (1
through 7) to restore explicit ordered numbering; edit the list block containing
"Stable diagnostic payload..." through "`make check-fmt`, `make lint`, and `make
test` pass." and replace the repeated "1." prefixes with ascending numbers to
keep the completion checklist unambiguous.
In `@docs/execplans/4-1-4-mode-aware-sempai-validation.md`:
- Around line 232-251: Switch the top-level checklist to a first-level
dash-style bullet and renumber the outcome items sequentially as a nested
numbered list from 1 to 9; specifically, replace the current flat numbered
entries that mention symbols like `sempai_yaml`,
`sempai::Engine::compile_yaml(...)`, `docs/sempai-query-language-design.md`, and
`docs/users-guide.md` with a single leading "-" bullet and then a nested 1.–9.
numbered list containing the same nine outcomes in order (ensuring the items
referencing `extract`, `taint`, `join`, `r2c-internal-project-depends`, and
`NOT_IMPLEMENTED` remain unchanged except for numbering); run markdown linting
to ensure the new dash + nested numbering conforms to docs guidelines.
In `@docs/execplans/5-1-1-entry-help.md`:
- Around line 769-770: The docs claim adding is_bare_invocation() to Cli in
cli.rs but earlier text chose lib.rs as the canonical location; move the
is_bare_invocation() implementation into the lib.rs module (add the function on
the appropriate impl or struct in lib.rs rather than on Cli in cli.rs), remove
or revert the conflicting change in cli.rs, and ensure you register mod
localizer; and update the corresponding imports in lib.rs so the new function
compiles and uses the localizer where intended.
In `@docs/execplans/5-2-1-define-the-rename-symbol-capability-contract.md`:
- Around line 38-70: The ordered lists in the document use repeated "1."
markers; update those to sequential numbers so the compliance criteria and
acceptance checks are explicitly numbered (e.g., change the repeated "1."
entries between the block covering lines ~38-70 and the other block around
~379-390 to 1., 2., 3., ...), ensuring all list items are renumbered in order
and that any references to these numbered items in the same file are adjusted
accordingly.
In
`@docs/execplans/5-2-5-unit-behavioural-and-end-to-end-coverage-for-rename-symbol.md`:
- Around line 612-664: Update the docs to match the actual exports from
crates/weaver-plugins/src/capability/test_support.rs: remove the stale
declarations for RenameSymbolFixture, RenameSymbolRequestFixture,
RenameSymbolResponseFixture, rename_symbol_request_fixtures,
rename_symbol_response_fixtures, assert_rename_symbol_request_fixture_contract
and assert_rename_symbol_response_fixture_contract, and replace the sample
import/usage with the real re-export path and symbol names that plugin tests
actually import from weaver_plugins::test_support (use the current public
functions/types defined in test_support.rs instead of the undocumented helpers),
ensuring the example in
docs/execplans/.../5-2-5-unit-behavioural-and-end-to-end-coverage-for-rename-symbol.md
imports the real API and shows the correct iteration/assert pattern used by
plugin contract_fixtures.rs.
In `@docs/execplans/7-1-1-stable-jsonl-schemas-for-observe-get-card.md`:
- Around line 32-53: The ordered list in
docs/execplans/7-1-1-stable-jsonl-schemas-for-observe-get-card.md currently
repeats "1." for every item; update the list so each item is sequentially
numbered (1., 2., 3., ...) in that block (items describing the new crate
weaver-cards, insta snapshots, rstest-bdd scenarios,
DomainRoutingContext::OBSERVE.known_operations addition, handler returning
GetCardResponse::Refusal, docs/users-guide.md update, and roadmap item
completion) to follow the documentation numbering guideline.
- Line 960: Replace the escaped footnote marker "\[^1\]: `docs/roadmap.md`,
lines 788–795" with a valid GitHub-flavoured footnote definition using the
unescaped marker "[^1]:" so the reference resolves (i.e., change the leading
"\[^1\]:" to "[^1]:"). Locate the footnote definition text in the
docs/execplans/7-1-1-stable-jsonl-schemas-for-observe-get-card.md file (the line
containing the escaped marker) and ensure the footnote is numbered in appearance
order and formatted as "[^1]: <footnote text>".
In
`@docs/execplans/7-1-3-implement-optional-lsp-enrichment-for-observe-get-card.md`:
- Around line 157-158: The retrospective's file-count arithmetic is
inconsistent: the headline total ("12 files") doesn't match the per-crate
breakdown which sums to 14 (e.g., for `weaverd`: added `enrich.rs` and modified
`mod.rs`, `get_card.rs`, `router.rs`); fix it by reconciling the numbers—either
update the headline total to the correct sum or adjust the itemized counts to
match 12—making the change in the markdown section that lists the per-crate file
changes (referencing `enrich.rs`, `mod.rs`, `get_card.rs`, `router.rs`) so the
execution record is auditable and consistent.
- Around line 29-37: The ordered lists in this document use repeated "1."
markers; update each ordered list block (for example the block starting "The
same request against a file..." and the other blocks referenced) so items are
numbered sequentially (1., 2., 3., …) instead of all "1.", keeping the rest of
the text unchanged; apply the same renumbering to the other list ranges
mentioned in the comment (47-63, 72-75, 338-347, 379-381, 404-406, 413-415,
419-423, 463-478) to satisfy markdownlint and the repo docstyle conventions.
In
`@docs/execplans/7-2-1-define-stable-jsonl-request-and-response-schemas-for-observe-graph-slice.md`:
- Around line 23-27: Remove the spurious leading spaces before the field names
in the markdown enumeration so the items read `depth`, `budget.max_edges`, and
`node_detail` without extra indentation; update the line(s) that list `depth`,
`budget.max_edges`, and `node_detail` in the observe graph-slice description so
all field names are aligned consistently with the other entries (no leading
spaces before the identifiers).
In `@docs/execplans/adopt-ortho-config-v0-8-0.md`:
- Around line 248-249: The sentence uses the en-GB-oxendict style incorrectly by
spelling "localisation"; update the wording to "localization" so the line reads
that `crates/weaver-cli/src/localizer.rs` exercises the Fluent localization APIs
added in v0.7.0 and must continue to compile after the v0.8.0 upgrade, ensuring
any other occurrences of "localisation" in this document are changed to
"localization" (reference: crates/weaver-cli/src/localizer.rs).
- Line 1: The H1 heading currently uses title case; change the first-line
heading text "# Adopt `ortho-config` v0.8.0 Across Weaver" to sentence case by
updating it to "# Adopt `ortho-config` v0.8.0 across Weaver" so the document
follows the sentence-case docs guideline.
- Around line 98-99: Replace British “normalise”/“Normalise” with
Oxford-compliant “normalize”/“Normalize” throughout this document, specifically
updating the phrase “Mitigation: normalise every member manifest to
`rust-version.workspace = true` as part of the migration.” and the other
occurrences referenced around lines 200-201; ensure all instances in
docs/execplans/adopt-ortho-config-v0-8-0.md use “-ize” spelling (e.g.,
normalize, serialized) so the repo’s en-GB-oxendict rule is followed.
In `@docs/execplans/phase-1-end-to-end-domain-command-execution.md`:
- Around line 34-35: Replace plain mentions of users-guide.md with an inline
Markdown link: locate occurrences of the literal text "users-guide.md"
(including the one in the argument format line containing `--uri file:///path.rs
--position 10:5`) and convert them to an inline link like `[users
guide](relative/path/to/users-guide.md)` or use angle-bracketed URL; apply the
same change for the other occurrences referenced (the ones near the argument
format and at the second occurrence) so all plain-document-path mentions become
proper inline links.
In `@docs/execplans/phase-1-human-readable-output.md`:
- Around line 20-33: The ordered list in the design doc has lost sequential
numbering; renumber the decision items so they form a proper 1..N sequence
(preserve the existing item text for "CLI-side rendering", "Output format
selection", "Miette-style renderer configuration", "Lightweight response
models", and "Source grouping and fallback") so the list displays as a single
ordered sequence rather than repeating "1." for each entry; update the numeric
prefixes accordingly to reflect their intended order.
In `@docs/execplans/phase-1-process-based-language-server-adapters.md`:
- Around line 22-30: In
docs/execplans/phase-1-process-based-language-server-adapters.md replace the
repeated "1." prefixes in the ordered list items (the entries starting "Must
implement `Send`", "Synchronous interface", "Single initialization", "Graceful
degradation", "Zero-trust sandbox") with proper sequential numbering (1., 2.,
3., 4., 5.) and likewise renumber the subsequent ordered list items around lines
36–39 to be sequential; also ensure first-level bullets use "-" where the
guideline requires a dash for top-level bullets so the document follows
markdownlint-cli2 and repo docstyle rules.
In `@docs/execplans/phase-2-double-lock-on-syntactic-lock.md`:
- Around line 17-19: The ordered lists under the "Syntactic Lock" and "Semantic
Lock" sections currently use repeated "1." entries; update these to explicit
sequential numbering (1., 2., 3., ...) so steps and scenarios read in proper
order, and convert top-level bullets to use "-" where the docs style mandates
first-level hyphens; locate the blocks titled "Syntactic Lock" and "Semantic
Lock" and the scenario list currently repeated in the later section (the block
around the scenarios referenced in the comment) and renumber each list item
sequentially while changing first-level list markers to "-" as required.
In `@docs/execplans/phase-4-weaver-graph.md`:
- Line 124: The ordered list in phase-4-weaver-graph.md uses repeated "1."
markers for multiple steps (e.g., the step "If the crate does not exist, create
it and add it to the workspace.") which breaks sequential numbering and lint
rules; update the ordered list so items are numbered sequentially (1, 2, 3, … up
to 8) across the steps referenced (lines around the crate creation step and the
other occurrences noted) to replace repeated "1." markers with the correct
incremental numbers and ensure top-level bullets use "-" elsewhere per docs
guidelines.
- Around line 234-238: Replace the wrapped prose listing of provider method
signatures with a fenced Rust code block: enclose the three signatures (fn
build_graph, fn callers_graph, fn callees_graph) inside a ```rust ... ``` fence
so they render as Rust code; ensure the opening fence includes the language
identifier "rust" and close with a matching ``` fence, removing the
inline/wrapped prose version.
In `@docs/execplans/sempai-design.md`:
- Around line 66-77: The ordered list in the docs/execplans/sempai-design.md
uses repeated "1." entries; update the six implementation steps so they use a
proper sequential numbered list (1., 2., 3., 4., 5., 6.) instead of repeating
"1." — specifically renumber the list items beginning with "Inventory current
docs and link references...", "Normalize style for headings...", "Repair broken
or inconsistent repository-local links.", "Add `docs/contents.md`...", "Add
`docs/repository-layout.md`...", and "Run markdown quality gates and capture
logs." to be 1 through 6 respectively and ensure markdownlint-cli2-compatible
formatting is preserved.
In `@docs/formal-verification-methods-in-weaver.md`:
- Around line 447-479: Replace the escaped and inline-packed footnote block
(\[^1\] .. \[^18\]) with proper GitHub-flavored numeric footnote definitions:
convert each escaped marker like \[^1\] into a standalone definition line using
the form `[^1]: <URL or description>` and ensure there is exactly one `[^n]:`
entry per reference (for all markers \[^1\] through \[^18\]) ordered by
appearance; remove the backslashes and packed inline entries so each footnote is
resolvable and markdownlint-compliant while preserving the original URLs and
brief descriptions from the diff.
In `@docs/jacquard-card-first-symbol-graph-design.md`:
- Line 20: The markdown uses escaped footnote markers like "\[^1\]" which render
literally; update all occurrences in this document (e.g., the reference near
"Symbol Delta Ledger (SDL)" and the corresponding definitions between lines
~1115-1151) to use standard GitHub footnote syntax "[^1]" for both inline
references and the matching footnote definition, removing the backslashes so
footnotes render and link correctly.
In `@docs/ortho-config-v0-6-0-migration-guide.md`:
- Around line 240-274: The footnote blocks use escaped label syntax (e.g.
\[^forwarded-features\], \[^hello-world-cargo\], \[^reexports\], etc.) which
breaks GitHub-flavored numeric footnotes; replace each escaped marker with a
numeric footnote reference ([^1], [^2], ...) in order of appearance and create
one separate footnote definition per block (one paragraph per footnote) so each
label becomes a single numbered definition and definitions are numbered
sequentially to preserve reference resolution and readability.
In `@docs/pratt-parser-for-ddlog-expressions.md`:
- Around line 22-32: In the documentation
"pratt-parser-for-ddlog-expressions.md" the procedural steps are incorrectly all
numbered "1."; update the four step items under the expression parser section so
they are a single sequential numbered list (1., 2., 3., 4.) and change the
top-level bullet style to use "-" for first-level bullets per docs guidelines;
also apply the same renumbering and dash-style change to the other occurrence
referenced (around lines 393-399) so all procedural lists follow the same
sequential numbering and first-level "-" bullet convention.
In `@docs/rstest-bdd-users-guide.md`:
- Around line 543-557: Change the unordered/duplicate "1." list into a
sequentially numbered ordered list (1 through 4) in the macro-expansion flow so
the execution order is explicit; specifically renumber the steps that describe
building the StepContext, looking up step functions by (keyword, pattern)
(Given‑When‑Then), invoking the registered step function with the StepContext,
and running the original test body so they appear as 1., 2., 3., 4. in that
order and update any surrounding punctuation/spacing to match the project's docs
style.
---
Outside diff comments:
In `@docs/adr-001-plugin-capability-model-and-act-extricate.md`:
- Around line 150-165: The ordered list in the migration plan must use
sequential numbering 1.–4.; update the four top-level items ("Capability
scaffolding", "Python provider rollout", "Rust provider rollout", "Hardening and
expansion") so their list markers are "1." through "4." respectively (replace
the repeated "1." markers), ensuring the rendered markdown and diffs show the
explicit phase sequence and follow the docs styling rule for renumbered ordered
lists.
In `@docs/execplans/2-3-1-validate-domains-client-side-before-daemon-startup.md`:
- Around line 431-437: The footer contains escaped footnote labels (\[^level3\],
\[^level10\]) and a percent-encoded roadmap link ([^roadmap]) that don't render;
replace them with GitHub-flavoured numeric footnotes (e.g. [^1], [^2], [^3]) and
update their corresponding definitions in the References section so each numeric
label maps to the proper target (e.g. replace the percent‑encoded roadmap URL
with a normal relative link to ../roadmap.md); also update all in-text
references elsewhere in the document from the old escaped names to the new
numeric footnote labels to keep links consistent.
In `@docs/execplans/phase-1-implement-the-socket-listener-in-weaverd.md`:
- Around line 171-223: The "Concrete Steps" ordered list uses repeated "1."
markers; renumber the items sequentially so the list follows proper Markdown
ordered-list formatting: locate the "Concrete Steps" section (the block that
currently lists steps starting with repeated "1.") and replace the repeating
"1." entries with ascending numbers (1., 2., 3., ...) so each step is uniquely
numbered and the document follows the repository Markdown style guidelines.
In `@docs/execplans/phase-1-jsonl-request-dispatch-loop.md`:
- Around line 17-37: The ordered list under "Design Decisions" uses "1." for
every item; update the five list markers so they are sequential (1., 2., 3., 4.,
5.) to improve source readability—specifically change the leading "1." tokens
for the items starting with "**Modular dispatch architecture**", "**Minimum
viable product (MVP) placeholder handlers**", "**Structured error responses**",
"**Case-insensitive domain matching**", and "**Re-use existing transport
infrastructure**" to 1 through 5 respectively, preserving the rest of the
Markdown and wording.
🪄 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
Run ID: 6a1a3f83-ef86-47cc-bffa-32dde517ca9a
📒 Files selected for processing (69)
AGENTS.mdREADME.mdagent-skill/weaver/SKILL.mddocs/adr-001-plugin-capability-model-and-act-extricate.mddocs/adr-002-query-language-for-feature-extraction-in-weaver.mddocs/adr-003-sempai-semgrep-compatible-query-engine.mddocs/adr-004-plugin-routing-refusal-semantics.mddocs/adr-005-verification-trust-boundary.mddocs/adr-006-plugin-execution-and-orchestration-strategy.mddocs/building-an-error-recovering-parser-with-chumsky.mddocs/complexity-antipatterns-and-refactoring-strategies.mddocs/contents.mddocs/developers-guide.mddocs/documentation-style-guide.mddocs/execplans/2-2-1-act-apply-patch-sub-command.mddocs/execplans/2-2-2-list-all-domains-in-top-level-help.mddocs/execplans/2-2-3-top-level-version-output.mddocs/execplans/2-2-4-provide-contextual-domain-guidance.mddocs/execplans/2-3-1-validate-domains-client-side-before-daemon-startup.mddocs/execplans/2-3-2-valid-operation-alternatives-for-unknown-operations.mddocs/execplans/2-3-3-standardize-actionable-guidance-in-startup-errors.mddocs/execplans/3-1-1-weaver-plugins-crate.mddocs/execplans/3-1-1a-plugin-for-rope.mddocs/execplans/3-1-1b-plugin-for-rust-analyzer.mddocs/execplans/4-1-1-scaffold-sempai-core-and-sempai.mddocs/execplans/4-1-2-structured-diagnostics-with-stable-error-codes.mddocs/execplans/4-1-3-yaml-rule-parsing-via-saphyr-and-serde-saphyr.mddocs/execplans/4-1-4-mode-aware-sempai-validation.mddocs/execplans/5-1-1-entry-help.mddocs/execplans/5-2-1-define-the-rename-symbol-capability-contract.mddocs/execplans/5-2-2-update-weaver-plugin-rope-manifest.mddocs/execplans/5-2-3-update-rust-plugin-to-declare-rename-symbol.mddocs/execplans/5-2-4-daemon-capability-resolution-for-rename-symbol.mddocs/execplans/5-2-5-unit-behavioural-and-end-to-end-coverage-for-rename-symbol.mddocs/execplans/7-1-1-stable-jsonl-schemas-for-observe-get-card.mddocs/execplans/7-1-2-tree-sitter-symbol-card-extraction.mddocs/execplans/7-1-3-implement-optional-lsp-enrichment-for-observe-get-card.mddocs/execplans/7-1-4-cache-integration-for-card-extraction-keyed-by-uri.mddocs/execplans/7-2-1-define-stable-jsonl-request-and-response-schemas-for-observe-graph-slice.mddocs/execplans/adopt-ortho-config-v0-8-0.mddocs/execplans/phase-1-end-to-end-domain-command-execution.mddocs/execplans/phase-1-human-readable-output.mddocs/execplans/phase-1-implement-the-socket-listener-in-weaverd.mddocs/execplans/phase-1-jsonl-request-dispatch-loop.mddocs/execplans/phase-1-process-based-language-server-adapters.mddocs/execplans/phase-2-double-lock-on-syntactic-lock.mddocs/execplans/phase-4-weaver-graph.mddocs/execplans/sempai-design.mddocs/formal-verification-methods-in-weaver.mddocs/jacquard-card-first-symbol-graph-design.mddocs/ortho-config-users-guide.mddocs/ortho-config-v0-6-0-migration-guide.mddocs/ortho-config-v0-8-0-migration-guide.mddocs/pratt-parser-for-ddlog-expressions.mddocs/reliable-testing-in-rust-via-dependency-injection.mddocs/repository-layout.mddocs/roadmap.mddocs/rstest-bdd-users-guide.mddocs/rust-doctest-dry-guide.mddocs/rust-extricate-actuator-plugin-technical-design.mddocs/rust-parser-testing-comprehensive-guide.mddocs/rust-testing-with-rstest-fixtures.mddocs/semgrep-language-reference/semgrep-legacy-vs-v2-guidance.mddocs/semgrep-language-reference/semgrep-minimal-examples.mddocs/semgrep-language-reference/semgrep-operator-precedence.mddocs/sempai-query-language-design.mddocs/ui-gap-analysis.mddocs/users-guide.mddocs/weaver-design.md
…rror handling - Added comprehensive client-side validation for known and unknown domains with guidance - Enhanced CLI output for missing operations to list available operations and provide hints - Implemented structured unknown-operation error payloads from daemon with human and JSON rendering - Unified and standardized actionable error guidance for startup and routing errors - Introduced edit-distance based domain suggestions for mistyped domains - Improved --help and --version behavior with proper exit codes and output streams - Updated documentation, tests (unit, integration, BDD), and roadmap accordingly This improves user experience by providing clearer, deterministic feedback and hints in CLI invocations, reducing confusion and aiding discoverability of commands and domain operations. Co-authored-by: devboxerhub[bot] <devboxerhub[bot]@users.noreply.github.com>
leynos
changed the title
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
docs/execplans/5-2-5-unit-behavioural-and-end-to-end-coverage-for-rename-symbol.md (1)
656-661:⚠️ Potential issue | 🟡 MinorFix the fixture import path to match the shipped re-exports.
Use the crate-root re-exports in the example, not
weaver_plugins::test_support, so the snippet matchescrates/weaver-plugins/src/lib.rs.Proposed fix
-use weaver_plugins::test_support::{ +use weaver_plugins::{ assert_rename_symbol_request_fixture_contract, assert_rename_symbol_response_fixture_contract, rename_symbol_request_fixtures, rename_symbol_response_fixtures, };Triage:
[type:docstyle]As per coding guidelines: documentation must reflect current API and behavioural changes in
docs/.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/execplans/5-2-5-unit-behavioural-and-end-to-end-coverage-for-rename-symbol.md` around lines 656 - 661, The import path uses the internal module weaver_plugins::test_support but the crate exposes these helpers at the crate root; change the use line to import the fixtures and assertions from the crate root re-exports (e.g. use weaver_plugins::{assert_rename_symbol_request_fixture_contract, assert_rename_symbol_response_fixture_contract, rename_symbol_request_fixtures, rename_symbol_response_fixtures};) so the example matches the public API in crates/weaver-plugins/src/lib.rs and uses the exported symbols named assert_rename_symbol_request_fixture_contract, assert_rename_symbol_response_fixture_contract, rename_symbol_request_fixtures, and rename_symbol_response_fixtures.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/developers-guide.md`:
- Around line 362-367: The docs list currently exposes an internal generic type
that may not be part of the public weaver_plugins surface; check whether
RenameSymbolFixture<T> is publicly re-exported from the weaver_plugins crate and
if not remove it from the “access” list in docs/developers-guide.md, leaving
only the public symbols (e.g. RenameSymbolRequestFixture,
RenameSymbolResponseFixture, rename_symbol_request_fixtures(),
rename_symbol_response_fixtures()) that downstream crates can import directly;
if RenameSymbolFixture<T> is intended to be public, instead ensure it is
actually re-exported from weaver_plugins and update the docs to match that
public API.
---
Duplicate comments:
In
`@docs/execplans/5-2-5-unit-behavioural-and-end-to-end-coverage-for-rename-symbol.md`:
- Around line 656-661: The import path uses the internal module
weaver_plugins::test_support but the crate exposes these helpers at the crate
root; change the use line to import the fixtures and assertions from the crate
root re-exports (e.g. use
weaver_plugins::{assert_rename_symbol_request_fixture_contract,
assert_rename_symbol_response_fixture_contract, rename_symbol_request_fixtures,
rename_symbol_response_fixtures};) so the example matches the public API in
crates/weaver-plugins/src/lib.rs and uses the exported symbols named
assert_rename_symbol_request_fixture_contract,
assert_rename_symbol_response_fixture_contract, rename_symbol_request_fixtures,
and rename_symbol_response_fixtures.
🪄 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
Run ID: 8437173e-88d5-4f16-af72-15d315277f0f
📒 Files selected for processing (4)
docs/developers-guide.mddocs/execplans/5-2-5-unit-behavioural-and-end-to-end-coverage-for-rename-symbol.mddocs/roadmap.mddocs/users-guide.md
Change request_arguments to return io::Result and handle non-string arguments with proper error instead of panicking, improving daemon request robustness. Co-authored-by: devboxerhub[bot] <devboxerhub[bot]@users.noreply.github.com>
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/execplans/phase-4-weaver-graph.md (1)
10-10:⚠️ Potential issue | 🟡 MinorApply sentence case to all headings.
Multiple headings capitalise words that should be lowercase. Sentence case requires only the first word and proper nouns to be capitalised.
Triage:
[type:docstyle]Apply this diff to fix all heading capitalisations
-## Purpose / Big Picture +## Purpose / big picture -## Surprises & Discoveries +## Surprises & discoveries -## Decision Log +## Decision log -## Outcomes & Retrospective +## Outcomes & retrospective -## Context and Orientation +## Context and orientation -## Plan of Work +## Plan of work -## Concrete Steps +## Concrete steps -## Validation and Acceptance +## Validation and acceptance -## Idempotence and Recovery +## Idempotence and recovery -## Artifacts and Notes +## Artifacts and notes -## Interfaces and Dependencies +## Interfaces and dependenciesAs per coding guidelines, "Write headings in sentence case in documentation."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/execplans/phase-4-weaver-graph.md`, apply sentence case to the following headings by changing only the incorrectly capitalised words to lowercase: at line 10 change "Big Picture" to "big picture"; at line 31 change "Discoveries" to "discoveries"; at line 35 change "Log" to "log"; at line 42 change "Retrospective" to "retrospective"; at line 46 change "Orientation" to "orientation"; at line 75 change "Work" to "work"; at line 111 change "Steps" to "steps"; at line 178 change "Acceptance" to "acceptance"; at line 192 change "Recovery" to "recovery"; at line 200 change "Notes" to "notes"; at line 221 change "Dependencies" to "dependencies".Also applies to: 31-31, 35-35, 42-42, 46-46, 75-75, 111-111, 178-178, 192-192, 200-200, 221-221
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/execplans/phase-4-weaver-graph.md` at line 10, Several Markdown headings use Title Case instead of sentence case; update the heading texts by lowercasing only the non-initial words listed (e.g., change "Purpose / Big Picture" → "Purpose / big picture", "Surprises & Discoveries" → "Surprises & discoveries", "Decision Log" → "Decision log", "Outcomes & Retrospective" → "Outcomes & retrospective", "Context and Orientation" → "Context and orientation", "Plan of Work" → "Plan of work", "Concrete Steps" → "Concrete steps", "Validation and Acceptance" → "Validation and acceptance", "Idempotence and Recovery" → "Idempotence and recovery", "Artifacts and Notes" → "Artifacts and notes", and "Interfaces and Dependencies" → "Interfaces and dependencies"); make only the minimal edits to the heading strings so the first word and proper nouns remain capitalized and all other words are lowercase.
♻️ Duplicate comments (1)
docs/execplans/5-1-1-entry-help.md (1)
776-777:⚠️ Potential issue | 🟠 MajorFix the ownership reference for
is_bare_invocation()toCli, notCliRunner.Replace this step text so it matches B4 and the interface section, which place
is_bare_invocation()onimpl Cli. Keep one canonical owner to prevent
implementation drift.Triage:
[type:docstyle]Patch
-4. Add `is_bare_invocation()` to the `CliRunner` implementation in `lib.rs` +4. Add `is_bare_invocation()` to the `Cli` implementation in `lib.rs` (B4).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/execplans/5-1-1-entry-help.md` around lines 776 - 777, Update the step text that currently says “Add is_bare_invocation() to the CliRunner implementation in lib.rs (B4)” so it instead states that is_bare_invocation() belongs on impl Cli (not CliRunner), matching the B4 description and the interface section; ensure the doc references the symbol is_bare_invocation(), the owner type Cli, and lib.rs/B4 consistently so the documentation uses a single canonical owner and prevents implementation drift.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@agent-skill/weaver/SKILL.md`:
- Around line 176-177: Remove the inline markdownlint suppression and replace
the single-line command with a fenced code block using a shell language tag;
locate the command string "weaver --output json act refactor --refactoring
rename --file src/main.py new_name=build_index offset=123" in SKILL.md and wrap
it like ```sh ... ``` so the command remains copy-pasteable and no HTML comment
disabling (<!-- markdownlint-disable-next-line MD013 -->) remains in the file.
In `@crates/weaver-e2e/tests/test_support/daemon_harness.rs`:
- Around line 213-230: The code currently converts JSON parse failures into a
synthetic payload (parsed_request via unwrap_or_else), hiding real errors;
instead fail fast by returning an InvalidData error when
serde_json::from_str(request_line.trim()) fails. Replace the unwrap_or_else that
creates the json! fallback with logic that maps the serde_json::Error into a
std::io::Error::new(std::io::ErrorKind::InvalidData, err) (or propagate the
serde error) so the caller sees the parse failure; keep the rest of the flow
that uses parsed_request, Operation::from(operation_str), and pushing into
requests unchanged.
---
Outside diff comments:
In `@docs/execplans/phase-4-weaver-graph.md`:
- Line 10: Several Markdown headings use Title Case instead of sentence case;
update the heading texts by lowercasing only the non-initial words listed (e.g.,
change "Purpose / Big Picture" → "Purpose / big picture", "Surprises &
Discoveries" → "Surprises & discoveries", "Decision Log" → "Decision log",
"Outcomes & Retrospective" → "Outcomes & retrospective", "Context and
Orientation" → "Context and orientation", "Plan of Work" → "Plan of work",
"Concrete Steps" → "Concrete steps", "Validation and Acceptance" → "Validation
and acceptance", "Idempotence and Recovery" → "Idempotence and recovery",
"Artifacts and Notes" → "Artifacts and notes", and "Interfaces and Dependencies"
→ "Interfaces and dependencies"); make only the minimal edits to the heading
strings so the first word and proper nouns remain capitalized and all other
words are lowercase.
---
Duplicate comments:
In `@docs/execplans/5-1-1-entry-help.md`:
- Around line 776-777: Update the step text that currently says “Add
is_bare_invocation() to the CliRunner implementation in lib.rs (B4)” so it
instead states that is_bare_invocation() belongs on impl Cli (not CliRunner),
matching the B4 description and the interface section; ensure the doc references
the symbol is_bare_invocation(), the owner type Cli, and lib.rs/B4 consistently
so the documentation uses a single canonical owner and prevents implementation
drift.
🪄 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
Run ID: 020ae643-77ab-4f72-9ea3-9ed2e23f679b
📒 Files selected for processing (11)
agent-skill/weaver/SKILL.mdcrates/weaver-e2e/tests/test_support/daemon_harness.rscrates/weaver-e2e/tests/test_support/refactor_routing.rsdocs/adr-006-plugin-execution-and-orchestration-strategy.mddocs/developers-guide.mddocs/execplans/3-1-1b-plugin-for-rust-analyzer.mddocs/execplans/5-1-1-entry-help.mddocs/execplans/5-2-5-unit-behavioural-and-end-to-end-coverage-for-rename-symbol.mddocs/execplans/adopt-ortho-config-v0-8-0.mddocs/execplans/phase-1-end-to-end-domain-command-execution.mddocs/execplans/phase-4-weaver-graph.md
Corrected "Weaver internal query operators" to "Weaver's internal query operators" in docs/adr-002-query-language-for-feature-extraction-in-weaver.md for grammatical accuracy. Co-authored-by: devboxerhub[bot] <devboxerhub[bot]@users.noreply.github.com>
leynos
changed the title
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
- Format SKILL.md agent flow command as a code block for clarity. - Adjust capitalization style across execplans documentation for consistency. - Minor fixes in 5-1-1-entry-help.md for accurate references. Co-authored-by: devboxerhub[bot] <devboxerhub[bot]@users.noreply.github.com>
leynos
changed the title
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@crates/weaver-e2e/tests/test_support/daemon_harness.rs`:
- Around line 219-224: The code currently defaults a missing command.operation
to an empty string and lets Operation::from map it to Other; instead, detect
when parsed_request.get("command").and_then(|c|
c.get("operation")).and_then(Value::as_str) is None and immediately return an
InvalidData (or appropriate error) instead of unwrap_or_default; update the
logic around operation_str / operation (the parsed_request extraction and the
call to Operation::from) to fail fast with a clear structured error message
indicating the missing command.operation field.
🪄 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
Run ID: 43a994f1-4000-41b1-a26e-7227964299cf
📒 Files selected for processing (5)
agent-skill/weaver/SKILL.mdcrates/weaver-e2e/tests/test_support/daemon_harness.rsdocs/adr-002-query-language-for-feature-extraction-in-weaver.mddocs/execplans/5-1-1-entry-help.mddocs/execplans/phase-4-weaver-graph.md
…ness In crates/weaver-e2e/tests/test_support/daemon_harness.rs, replaced unwrap_or_default with explicit error return when the command.operation string field is missing from a fake daemon request. Also corrected capitalization in docs/execplans/adopt-ortho-config-v0-8-0.md to maintain style consistency. Co-authored-by: devboxerhub[bot] <devboxerhub[bot]@users.noreply.github.com>
Owner
Author
|
@coderabbitai Have the following failed checks now been resolved? If further work is required, please provide an AI agent prompt for the remaining work to be done to address these failures. ❌ Failed checks (1 warning)
|
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 2733 🏁 Script executed: Length of output: 1999
Additional related test modules also carry module docs:
No further action is required. |
This was referenced Apr 22, 2026
Merged
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
1 participant
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.
Summary
Changes
test-support.features = ["test-support"]and register contract/conformance tests.Task
Updated Git diff: