Update rope plugin and weaverd for rename-symbol capability#74
Merged
Conversation
Reviewer's GuideImplements the new rename-symbol capability end-to-end for the rope plugin and weaverd: argument parsing is refactored into a new module, failures now carry structured ReasonCode diagnostics, weaverd advertises the capability and maps CLI rename requests into contract-conforming rename-symbol PluginRequests, tests/BDD/features are updated accordingly, and docs/roadmap/execplans are aligned. Sequence diagram for CLI rename mapped to rename_symbol plugin requestsequenceDiagram
actor User
participant WeaverCli as WeaverCli
participant Weaverd as WeaverdDaemon
participant Refactor as SandboxRefactorRuntime
participant Rope as RopePlugin
participant Adapter as PythonRopeAdapter
User->>WeaverCli: run act_refactor --provider rope --refactoring rename --file path new_name=X offset=N
WeaverCli->>Weaverd: HTTP request act_refactor(provider=rope, refactoring=rename, file, extra_args)
Weaverd->>Refactor: handle(provider, refactoring, file, extra_args)
Note over Refactor: Build plugin_args from extra_args
Refactor->>Refactor: apply_rename_symbol_mapping(plugin_args, file)
Refactor->>Refactor: effective_operation = rename_symbol
Refactor->>Rope: PluginRequest(operation=rename_symbol, files=[FilePayload(path,file_content)], arguments={uri,position,new_name})
Rope->>Rope: run_with_adapter(stdin,stdout,adapter)
Rope->>Rope: read_request() -> PluginRequest
Rope->>Rope: execute_request(adapter,request)
Rope->>Rope: match operation rename_symbol
Rope->>Rope: execute_rename(adapter,request)
Rope->>Rope: parse_rename_symbol_arguments(arguments)
Rope->>Adapter: rename(file, offset, new_name)
Adapter-->>Rope: modified_content or error
alt success and content changed
Rope->>Rope: build_search_replace_patch()
Rope-->>Weaverd: PluginResponse.success(Diff)
else adapter error or unchanged
Rope->>Rope: map error to PluginFailure(reason_code)
Rope->>Rope: failure_response(PluginFailure)
Rope-->>Weaverd: PluginResponse.failure(diagnostic with ReasonCode)
end
Weaverd-->>WeaverCli: refactor result (diff or diagnostics)
WeaverCli-->>User: show diff or error with reason_code
Updated class diagram for rope plugin rename_symbol arguments and failuresclassDiagram
class RenameSymbolArgs {
-usize offset
-String new_name
+offset() usize
+new_name() &str
}
class PluginFailure {
-String message
-Option~ReasonCode~ reason_code
+plain(message String) PluginFailure
+with_reason(message String, reason ReasonCode) PluginFailure
+fmt(self, f Formatter) fmtResult
}
class RopeAdapter {
<<interface>>
+rename(file FilePayload, offset usize, new_name &str) Result~String, RopeAdapterError~
}
class RopeAdapterError {
<<enum>>
}
class ReasonCode {
<<enum>>
SymbolNotFound
MacroGenerated
AmbiguousReferences
UnsupportedLanguage
IncompletePayload
NameConflict
OperationNotSupported
}
class PluginRequest {
+operation() &str
+arguments() &HashMap~String, Value~
+files() &[FilePayload]
}
class PluginResponse {
+failure(diagnostics Vec~PluginDiagnostic~) PluginResponse
}
class PluginDiagnostic {
+new(severity DiagnosticSeverity, message String) PluginDiagnostic
+with_reason_code(self, code ReasonCode) PluginDiagnostic
}
class Functions {
<<static>>
+parse_rename_symbol_arguments(arguments &HashMap~String, Value~) Result~RenameSymbolArgs, String~
+execute_request(adapter RopeAdapter, request &PluginRequest) Result~PluginResponse, PluginFailure~
+execute_rename(adapter RopeAdapter, request &PluginRequest) Result~PluginResponse, PluginFailure~
+failure_response(failure PluginFailure) PluginResponse
+read_request(stdin BufRead) Result~PluginRequest, PluginFailure~
}
RenameSymbolArgs ..> PluginRequest : parsed_from_arguments
PluginFailure --> ReasonCode : optional_reason_code
PluginFailure ..> PluginResponse : used_by_failure_response
Functions ..> RenameSymbolArgs : creates
Functions ..> PluginFailure : returns_on_error
Functions ..> RopeAdapter : calls_rename
Functions ..> PluginRequest : reads_operation_and_arguments
Functions ..> PluginResponse : constructs
PluginResponse o--> PluginDiagnostic : contains
PluginDiagnostic --> ReasonCode : optional
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Summary by CodeRabbit
WalkthroughSummarise the addition of structured argument parsing and failure modelling for the rename-symbol capability, update the runtime refactor handler to map legacy rename requests to the new contract, and propagate structured PluginFailure diagnostics through plugin execution and tests. (≤50 words) Changes
Sequence Diagram(s)sequenceDiagram
actor Client
participant weaverd as "weaverd\nRefactor Handler"
participant plugin as "weaver-plugin-rope"
participant adapter as "Rope Adapter"
participant lsp as "LSP"
Client->>weaverd: invoke rename refactoring (may supply offset)
weaverd->>weaverd: apply_rename_symbol_mapping\n(map rename→rename-symbol,\ninject uri, offset→position)
weaverd->>plugin: PluginRequest (rename-symbol with uri, position, new_name)
plugin->>plugin: parse_rename_symbol_arguments\n(validate uri, parse position, check new_name)
plugin->>adapter: request rename (offset/new_name)
adapter->>lsp: query symbol/location
lsp-->>adapter: symbol location
adapter-->>plugin: diff result or error
alt Success with diff
plugin-->>weaverd: PluginResponse (diff)
weaverd->>Client: apply-patch and return result
else No change or adapter error
plugin-->>plugin: produce PluginFailure (ReasonCode::SymbolNotFound)
plugin-->>weaverd: PluginResponse with failure + reason_code
weaverd->>Client: surface failure diagnostics
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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.
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-plugin-rope/src/tests/behaviour.rs Comment on file fn then_failure_has_reason_code(world: &mut World, code: String) {
let needle = code.trim_matches('"');
let response = resolved_response(world);
let diagnostics = response.diagnostics();
assert!(
diagnostics
.iter()
.any(|d| d.reason_code().is_some_and(|rc| rc.as_str() == needle)),
"expected reason code '{needle}' in diagnostics: {diagnostics:?}",
);
}❌ 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-plugin-rope/src/tests/mod.rs Comment on file #[test]
fn unsupported_operation_returns_error() {
fn unsupported_operation_returns_error_with_reason_code() {❌ New issue: Code Duplication |
![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/weaverd/src/dispatch/act/refactor/mod.rs Comment on file // Map rename operations to the rename-symbol capability contract.
// The CLI still accepts `--refactoring rename`; the handler translates
// to the contract operation name and argument schema.
let effective_operation = match args.refactoring.as_str() {
"rename" => {
plugin_args
.entry(String::from("uri"))
.or_insert_with(|| serde_json::Value::String(args.file.clone()));
if let Some(offset_val) = plugin_args.remove("offset") {
plugin_args
.entry(String::from("position"))
.or_insert(offset_val);
}
String::from("rename-symbol")
}
_ => args.refactoring.clone(),
};❌ New issue: Bumpy Road Ahead |
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.
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 6 issues
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="crates/weaver-plugin-rope/src/tests/mod.rs" line_range="133-140" />
<code_context>
+ );
+}
+
#[rstest]
-#[case::missing_offset(remove_offset as fn(&mut _), Some("offset"))]
-#[case::boolean_offset(set_boolean_offset as fn(&mut _), Some("offset"))]
-#[case::negative_offset(set_negative_offset as fn(&mut _), Some("non-negative integer"))]
-#[case::numeric_offset_succeeds(set_numeric_offset as fn(&mut _), None)]
+#[case::missing_uri(remove_uri as fn(&mut _), Some("uri"))]
+#[case::missing_position(remove_position as fn(&mut _), Some("position"))]
+#[case::boolean_position(set_boolean_position as fn(&mut _), Some("position"))]
+#[case::negative_position(set_negative_position as fn(&mut _), Some("non-negative integer"))]
+#[case::numeric_position_succeeds(set_numeric_position as fn(&mut _), None)]
#[case::numeric_new_name(set_numeric_new_name as fn(&mut _), Some("new_name argument must be a string"))]
#[case::empty_new_name(set_empty_new_name as fn(&mut _), Some("new_name"))]
fn rename_argument_validation(
</code_context>
<issue_to_address>
**suggestion (testing):** Add tests for invalid `uri` values (non-string and empty) to fully exercise `rename-symbol` argument validation
`parse_rename_symbol_arguments` now validates `uri` as required, string-typed, and non-empty, but tests only cover the missing `uri` case. Please add cases for:
- `uri` present but non-string (e.g., boolean/number), asserting the error mentions `"uri argument must be a string"`.
- `uri` present but empty/whitespace-only, asserting the error mentions `"uri argument must not be empty"`.
These can mirror the existing negative cases for `position`/`new_name` and should still surface `ReasonCode::IncompletePayload` via `execute_request`.
Suggested implementation:
```rust
#[rstest]
#[case::missing_uri(remove_uri as fn(&mut _), Some("uri"))]
#[case::boolean_uri(set_boolean_uri as fn(&mut _), Some("uri argument must be a string"))]
#[case::empty_uri(set_empty_uri as fn(&mut _), Some("uri argument must not be empty"))]
#[case::missing_position(remove_position as fn(&mut _), Some("position"))]
#[case::boolean_position(set_boolean_position as fn(&mut _), Some("position"))]
#[case::negative_position(set_negative_position as fn(&mut _), Some("non-negative integer"))]
#[case::numeric_position_succeeds(set_numeric_position as fn(&mut _), None)]
#[case::numeric_new_name(set_numeric_new_name as fn(&mut _), Some("new_name argument must be a string"))]
#[case::empty_new_name(set_empty_new_name as fn(&mut _), Some("new_name"))]
fn rename_argument_validation(
```
You will also need to:
1. Define `set_boolean_uri` and `set_empty_uri` helpers in this test module, following the existing pattern used by `set_boolean_position`, `set_negative_position`, `set_numeric_position`, `set_numeric_new_name`, and `set_empty_new_name`. For example:
- `set_boolean_uri(args: &mut Value)` should set `args["uri"]` to a boolean (e.g. `Value::Bool(true)`).
- `set_empty_uri(args: &mut Value)` should set `args["uri"]` to an empty or whitespace-only string (e.g. `Value::String("".into())` or `" ".into()`), matching how emptiness is validated in `parse_rename_symbol_arguments`.
2. Ensure these helpers are wired into whatever common “rename arguments” builder you have (likely something like `rename_arguments` or similar) so that `request_with_args(rename_arguments)` plus the helper mutation produces the invalid payload sent to `execute_request`.
3. If the error messages in `parse_rename_symbol_arguments` differ slightly from those referenced here, adjust the `"uri argument must be a string"` and `"uri argument must not be empty"` substrings to exactly match the implementation so the tests pass.
</issue_to_address>
### Comment 2
<location path="crates/weaverd/src/dispatch/act/refactor/tests.rs" line_range="267-276" />
<code_context>
+ );
+}
+
#[rstest]
-#[case::missing_offset(remove_offset as fn(&mut _), Some("offset"))]
-#[case::boolean_offset(set_boolean_offset as fn(&mut _), Some("offset"))]
</code_context>
<issue_to_address>
**suggestion (testing):** Extend refactor handler tests to cover pre-existing `uri`/missing `offset` edge cases in the rename-symbol mapping
The current test only covers the happy-path mapping from `--refactoring rename` to a `rename-symbol` request. Please also add coverage for:
- Calls where `plugin_args` already includes a `uri`, to confirm it is not overwritten by `--file`.
- Calls where `offset` is omitted, to confirm the mapping does not introduce a `position` key.
These cases will lock in the contract of `apply_rename_symbol_mapping` and help prevent regressions in future changes.
</issue_to_address>
### Comment 3
<location path="docs/execplans/5-2-2-update-weaver-plugin-rope-manifest.md" line_range="485" />
<code_context>
+from the beginning of that stage. No destructive operations are involved. The
+new `arguments.rs` file is additive; existing files are edited in place.
+
+## Artifacts and notes
+
+### Line budget analysis
</code_context>
<issue_to_address>
**nitpick (typo):** Align "Artifacts" spelling with the en‑GB Oxford spelling convention stated earlier in the document.
Per constraint 7 (en‑GB‑oxendict), this heading should use “Artefacts and notes”.
Suggested implementation:
```
## Artefacts and notes
```
Search the rest of `docs/execplans/5-2-2-update-weaver-plugin-rope-manifest.md` (and related execplan documents) for any other occurrences of “Artifacts” or “artifact” and adjust them to “Artefacts” / “artefact” as required by the en‑GB‑oxendict convention, to keep the terminology consistent throughout the documentation set.
</issue_to_address>
### Comment 4
<location path="docs/execplans/5-2-2-update-weaver-plugin-rope-manifest.md" line_range="63" />
<code_context>
+ items have `///` rustdoc comments.
+7. **en-GB-oxendict spelling.** Comments and documentation use British English
+ with Oxford "-ize" / "-yse" / "-our" spelling.
+8. **rstest-bdd v0.5.0.** BDD tests use v0.5.0 with mutable world fixtures
+ (`&mut`). The fixture parameter must be named exactly `world` (not
+ `_world`). Use `let _ = world;` to suppress unused warnings.
</code_context>
<issue_to_address>
**suggestion (review_instructions):** The acronym “BDD” is used here without being expanded on first use in this document.
To follow the style guidance, expand the acronym on first use, for example: `Behaviour-driven development (BDD) tests use v0.5.0 with mutable world fixtures`.
<details>
<summary>Review instructions:</summary>
**Path patterns:** `**/*.md`
**Instructions:**
Define uncommon acronyms on first use.
</details>
</issue_to_address>
### Comment 5
<location path="docs/execplans/5-2-2-update-weaver-plugin-rope-manifest.md" line_range="69" />
<code_context>
+9. **Lint suppressions must use `#[expect]` with reason**, not `#[allow]`.
+10. **Do not modify `crates/weaver-plugins/`.** The capability contract types
+ from roadmap item 5.2.1 are complete and must not be changed.
+11. **CLI backward compatibility.** Users still pass `--refactoring rename`,
+ `offset=N`, and `new_name=X`. The handler does the translation internally.
+12. **`str_to_string` denied.** Use `String::from()` or `.into()`, not
</code_context>
<issue_to_address>
**suggestion (review_instructions):** The acronym “CLI” is introduced without being defined on first use in this document.
Consider expanding this to something like `Command-line interface (CLI) backward compatibility` so the acronym is defined when first introduced.
<details>
<summary>Review instructions:</summary>
**Path patterns:** `**/*.md`
**Instructions:**
Define uncommon acronyms on first use.
</details>
</issue_to_address>
### Comment 6
<location path="docs/execplans/5-2-2-update-weaver-plugin-rope-manifest.md" line_range="223" />
<code_context>
+ updated in roadmap item 5.2.1 and must NOT be modified in 5.2.2.
+
+- `crates/weaver-plugin-rope/` — The Python rope-backed actuator plugin. A
+ standalone binary crate that reads one JSONL request from stdin and writes
+ one JSONL response to stdout. Main logic is in `src/lib.rs` (384 lines).
+ Tests are in `src/tests/mod.rs` (224 lines) and `src/tests/behaviour.rs` (176
</code_context>
<issue_to_address>
**suggestion (review_instructions):** The acronym “JSONL” is used without being expanded on first use in this document.
Please expand JSONL when first mentioned, for example `JSON Lines (JSONL) request`, to comply with the acronym guidance.
<details>
<summary>Review instructions:</summary>
**Path patterns:** `**/*.md`
**Instructions:**
Define uncommon acronyms on first use.
</details>
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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-plugin-rope/src/lib.rs`:
- Around line 293-297: The adapter.rename call currently maps all errors to
PluginFailure::with_reason(..., ReasonCode::SymbolNotFound); change the map_err
closure to match on the RopeAdapterError variants returned by adapter.rename:
map semantic misses (e.g., RopeAdapterError::EngineFailed { .. } or whatever
indicates a true rename miss) to PluginFailure::with_reason(...,
ReasonCode::SymbolNotFound), map path/validation errors (e.g.,
RopeAdapterError::InvalidPath { .. }) to PluginFailure::with_reason(...,
ReasonCode::IncompletePayload), and map any other infrastructure faults to
PluginFailure::plain(error.to_string()) so adapter.rename(...) returns the
correct domain vs non-domain diagnostics.
In `@crates/weaverd/src/dispatch/act/refactor/mod.rs`:
- Around line 336-343: In apply_rename_symbol_mapping, stop preserving
caller-supplied "uri" and "position" by replacing the entry-based or_insert
behavior with direct insertion: always set plugin_args["uri"] to
serde_json::Value::String(file.to_owned()) via plugin_args.insert(...), and when
an "offset" value is present (the removed offset_val), set
plugin_args["position"] by inserting that value with plugin_args.insert(...),
ensuring CLI-provided file/offset overwrite any existing fields.
In `@docs/execplans/5-2-2-update-weaver-plugin-rope-manifest.md`:
- Around line 151-152: Change the phrase "the acceptance criteria says" to "the
acceptance criteria say" in the rationale sentence (the fragment shown as "the
acceptance criteria says") to fix subject–verb agreement and ensure the sentence
follows en-GB-oxendict grammar conventions; update that single sentence in the
rationale paragraph while keeping the rest of the wording and dual-support
rationale intact.
In `@docs/jacquard-card-first-symbol-graph-design.md`:
- Around line 834-839: The single-line reference chain "[1]: weaver-design.md
[2]: ../crates/weaverd/src/dispatch/router.rs [3]:
../crates/weaverd/src/backends.rs [4]:
../crates/weaverd/src/semantic_provider/mod.rs [5]:
../crates/weaver-graph/src/provider.rs [6]:
<https://github.com/GlitterKill/sdl-mcp> [7]:
../crates/weaverd/src/dispatch/request.rs [8]:
../crates/weaver-graph/src/uri.rs" must be split so each reference definition is
on its own line (e.g., "[1]: weaver-design.md" on one line, then "[2]:
../crates/weaverd/src/dispatch/router.rs" on the next, etc.), and ensure each
resulting line is wrapped to <=80 columns to satisfy markdownlint and paragraph
wrapping rules.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: a5315461-d135-408c-806b-f523504d2a89
📒 Files selected for processing (14)
crates/weaver-plugin-rope/src/arguments.rscrates/weaver-plugin-rope/src/lib.rscrates/weaver-plugin-rope/src/tests/behaviour.rscrates/weaver-plugin-rope/src/tests/mod.rscrates/weaver-plugin-rope/tests/features/rope_plugin.featurecrates/weaverd/src/dispatch/act/refactor/mod.rscrates/weaverd/src/dispatch/act/refactor/tests.rsdocs/execplans/4-1-1-scaffold-sempai-core-and-sempai.mddocs/execplans/5-2-1-define-the-rename-symbol-capability-contract.mddocs/execplans/5-2-2-update-weaver-plugin-rope-manifest.mddocs/jacquard-card-first-symbol-graph-design.mddocs/repository-layout.mddocs/roadmap.mddocs/users-guide.md
…tructured failures - Extract argument parsing for rename-symbol to a new `arguments` module - Validate and parse `uri`, `position`, and `new_name` arguments per contract - Introduce `PluginFailure` struct to carry error messages with optional reason codes - Update plugin dispatch to accept `rename-symbol` operation and reject old `rename` with proper reason code - Return failure diagnostics with stable ReasonCode values - Update `weaverd` refactor handler to register rename-symbol capability and translate CLI `rename` to `rename-symbol` with argument mapping - Adapt tests and BDD scenarios for the new contract and failure semantics - Update documentation and roadmap entries accordingly This completes rename-symbol capability conformance and enhances diagnostic structure with reason codes. Co-authored-by: devboxerhub[bot] <devboxerhub[bot]@users.noreply.github.com>
…d map rename args in refactor handler - Introduce assert_any_diagnostic helper to centralize diagnostic assertions in plugin-rope tests. - Parametrize unsupported operations test with rstest and merge similar test cases. - Refactor rename argument handling in weaverd refactor handler to map `rename` to `rename-symbol` contract and transform args accordingly. - Clean up imports and simplify code comments for clarity. Co-authored-by: devboxerhub[bot] <devboxerhub[bot]@users.noreply.github.com>
- Differentiate infrastructure vs semantic adapter failures in execute_rename: only EngineFailed emits SymbolNotFound reason code - Force-overwrite uri/position in apply_rename_symbol_mapping via insert - Add boolean_uri and empty_uri validation test cases - Add handler tests for pre-existing uri overwrite and missing offset - Fix spelling: Artifacts → Artefacts, criteria says → criteria say - Expand acronyms on first use: BDD, CLI, JSONL - Restore one-per-line reference definitions in jacquard doc Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
devboxerhub
Bot
force-pushed
the
update-weaver-plugin-rope-manifest-4kvzqk
branch
from
b04dc17 to
e147b36
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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/execplans/5-2-2-update-weaver-plugin-rope-manifest.md`:
- Line 335: The doc line containing "Update parameterised
`rename_argument_validation` test cases for new" uses British spelling
"parameterised"; update that occurrence to the repository's Oxford -ize form
"parameterized" so the line reads "Update parameterized
`rename_argument_validation` test cases for new", keeping the surrounding text
and the `rename_argument_validation` identifier unchanged.
- Around line 42-43: The long bullet beginning "Behaviour-driven development
(BDD) scenarios cover happy path, missing arguments, unsupported operation,
adapter failure, unchanged output, and reason code verification." must be
wrapped to 80 columns or less to satisfy MD013; edit the bullet in the
docs/execplans entry (the list item text) and split it into multiple
continuation lines (breaking after commas or logical phrase boundaries) so each
line is ≤80 characters while preserving the same content and punctuation.
In `@docs/users-guide.md`:
- Around line 574-580: The docs are inconsistent about the refactor contract's
position semantics (some places say UTF-8 byte offset, others say line:column).
Update the contract section so "position" consistently means the UTF-8 byte
offset integer (matching the `offset` field behavior), change any examples like
`10:5` to numeric offsets, and update descriptions and validation rules for
`position` and `offset` (e.g., "Non-negative integer, provided as a number or
numeric string") so callers use a single semantic model; search for occurrences
of `position`, `offset`, `--refactoring`, and `rename` in the refactor docs and
harmonize them accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 92eca7bc-f4ba-470f-ad25-a6aa06e75806
📒 Files selected for processing (11)
crates/weaver-plugin-rope/src/arguments.rscrates/weaver-plugin-rope/src/lib.rscrates/weaver-plugin-rope/src/tests/behaviour.rscrates/weaver-plugin-rope/src/tests/mod.rscrates/weaver-plugin-rope/tests/features/rope_plugin.featurecrates/weaverd/src/dispatch/act/refactor/mod.rscrates/weaverd/src/dispatch/act/refactor/tests.rsdocs/execplans/5-2-1-define-the-rename-symbol-capability-contract.mddocs/execplans/5-2-2-update-weaver-plugin-rope-manifest.mddocs/roadmap.mddocs/users-guide.md
Owner
Author
|
@coderabbitai Have the following now been resolved? Annul any requirements that violate the en-GB-oxendict spelling (-ize / -yse / -our) conventions, or where the requirement unnecessarily increases cyclomatic complexity. |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chainI'll read the relevant files to determine the current state before evaluating each comment. 🏁 Script executed: Length of output: 23020
Comment 1 — invalid
|
…scriptions - Rename operation from `rename` to `rename-symbol` for clarity. - Change argument `offset` to `position` and add new `uri` argument. - Update user guide and execution plan docs to reflect these changes. - Clarify `position` is a UTF-8 byte offset. - Fix typos and improve formatting in BDD scenarios descriptions. Co-authored-by: devboxerhub[bot] <devboxerhub[bot]@users.noreply.github.com>
There was a problem hiding this comment.
♻️ Duplicate comments (3)
docs/users-guide.md (1)
753-753:⚠️ Potential issue | 🟠 MajorFix the
positionfield type to match byte-offset semantics.Replace
stringwith the contract’s numeric offset typing, or explicitly document dual acceptance (numberand numeric string). Keep one model throughout the guide; the current type cell conflicts with the neighbouring parameter rules and will cause incompatible payloads.🛠️ Proposed doc fix
-| `position` | string | Position of the symbol as a UTF-8 byte offset. | +| `position` | integer \| string | UTF-8 byte offset of the symbol (for example `4` or `"4"`). |🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/users-guide.md` at line 753, Update the docs to make the `position` field's type consistent with the contract byte-offset semantics: change the table cell that currently lists `position` as "string" to use a numeric offset type (e.g., "number" or "integer (byte offset)"), or explicitly state that `position` accepts both a numeric value and a numeric string (and normalize which one the API expects). Ensure this single model is used everywhere the `position` symbol appears (including the parameter rules and examples) so payloads are compatible and unambiguous.docs/execplans/5-2-2-update-weaver-plugin-rope-manifest.md (2)
42-44:⚠️ Potential issue | 🟡 MinorWrap the overlong bullet to satisfy MD013 and coding guidelines.
Split this bullet into continuation lines breaking after commas. Each line must not exceed 80 characters.
🛠️ Proposed fix
-- Behaviour-driven development (BDD) scenarios cover happy path, - missing arguments, unsupported operation, adapter failure, - unchanged output, and reason code verification. +- Behaviour-driven development (BDD) scenarios cover happy path, missing + arguments, unsupported operation, adapter failure, unchanged output, and + reason code verification.🤖 Prompt for AI agents
Verify each finding against the current code and fix only if needed. In `@docs/execplans/5-2-2-update-weaver-plugin-rope-manifest.md` at lines 42-44, wrap the bullet beginning "Behaviour-driven development (BDD) scenarios cover happy path, missing arguments, unsupported operation, adapter failure, unchanged output, and reason code verification." to 80 columns by inserting line breaks after commas while preserving the list item formatting and all content.As per coding guidelines, Markdown paragraphs and bullet points must be wrapped at 80 columns.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/execplans/5-2-2-update-weaver-plugin-rope-manifest.md` around lines 42 - 44, Locate the bullet that starts "Behaviour-driven development (BDD) scenarios cover happy path, missing arguments, unsupported operation, adapter failure, unchanged output, and reason code verification." and wrap it to 80 columns by inserting line breaks after commas so each continuation line is <=80 characters, preserving the leading list marker and indentation and keeping the full original text and commas intact; ensure no other content changes.
89-89:⚠️ Potential issue | 🟡 MinorFix subject–verb agreement in the Ambiguity tolerance.
Replace "The acceptance criteria states" with "The acceptance criteria state". The noun "criteria" is plural and requires the plural verb form.
🛠️ Proposed fix
-- **Ambiguity:** The acceptance criteria states "Legacy provider routing is not +- **Ambiguity:** The acceptance criteria state "Legacy provider routing is not🤖 Prompt for AI agents
Verify each finding against the current code and fix only if needed. In `@docs/execplans/5-2-2-update-weaver-plugin-rope-manifest.md` at line 89, change "The acceptance criteria states" to "The acceptance criteria state" to fix the subject–verb disagreement (criteria is plural).As per coding guidelines, documentation must use en-GB-oxendict spelling and grammar.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/execplans/5-2-2-update-weaver-plugin-rope-manifest.md` at line 89, Fix the subject–verb agreement in the "Ambiguity" line where the phrase reads "The acceptance criteria states" — change it to "The acceptance criteria state" (criteria is plural); update the sentence in the Ambiguity section of the 5-2-2-update-weaver-plugin-rope-manifest document and ensure the rest of the sentence follows en-GB-oxendict spelling and grammar conventions.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@docs/execplans/5-2-2-update-weaver-plugin-rope-manifest.md`:
- Around line 42-44: Locate the bullet that starts "Behaviour-driven development
(BDD) scenarios cover happy path, missing arguments, unsupported operation,
adapter failure, unchanged output, and reason code verification." and wrap it to
80 columns by inserting line breaks after commas so each continuation line is
<=80 characters, preserving the leading list marker and indentation and keeping
the full original text and commas intact; ensure no other content changes.
- Line 89: Fix the subject–verb agreement in the "Ambiguity" line where the
phrase reads "The acceptance criteria states" — change it to "The acceptance
criteria state" (criteria is plural); update the sentence in the Ambiguity
section of the 5-2-2-update-weaver-plugin-rope-manifest document and ensure the
rest of the sentence follows en-GB-oxendict spelling and grammar conventions.
In `@docs/users-guide.md`:
- Line 753: Update the docs to make the `position` field's type consistent with
the contract byte-offset semantics: change the table cell that currently lists
`position` as "string" to use a numeric offset type (e.g., "number" or "integer
(byte offset)"), or explicitly state that `position` accepts both a numeric
value and a numeric string (and normalize which one the API expects). Ensure
this single model is used everywhere the `position` symbol appears (including
the parameter rules and examples) so payloads are compatible and unambiguous.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 1f4d9bbd-b945-49ea-bc8b-511d37e89946
📒 Files selected for processing (2)
docs/execplans/5-2-2-update-weaver-plugin-rope-manifest.mddocs/users-guide.md
This was referenced Mar 11, 2026
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
Core changes
Refactor handshake and messaging
Tests
Documentation and roadmap
Validation plan
Acceptance criteria mapping
Quick notes
◳ Generated by DevBoxer ◰
ℹ️ Tag @devboxerhub to ask questions and address PR feedback
📎 Task: https://www.devboxer.com/task/c3aa7b37-2ae5-4c67-a142-80785dc9c47b
Summary by Sourcery
Adopt the new rename-symbol capability end-to-end for the rope plugin and weaverd refactor handler, including contract-conforming arguments, capability metadata, and reason-coded diagnostics, while updating tests and documentation accordingly.
New Features:
Bug Fixes:
Enhancements:
Documentation: