7.2.1: Stable JSONL schemas for observe graph-slice#96
Merged
Conversation
…chemas for observe graph-slice This ExecPlan outlines the design, constraints, risks, progress stages, and testing strategy for implementing a stable JSONL request and response schema for the `observe graph-slice` operation. It establishes the contract and expected behavior for downstream consumers, details the schema ownership within `weaver-cards`, and sets the roadmap milestones for defining and testing the feature comprehensively. No code changes are included; this document guides the upcoming implementation. Co-authored-by: devboxerhub[bot] <devboxerhub[bot]@users.noreply.github.com>
OverviewThis PR implements a stable, serde-annotated JSONL contract for the observe graph-slice surface (contract-only work for milestone 7.2.1). It adds typed request/response schema types, deterministic parsing/validation, CLI discoverability, daemon routing and a handler scaffold that emits a deliberate not-yet-implemented refusal, together with comprehensive unit, snapshot and BDD tests, deterministic fixtures and Insta snapshots. Full traversal implementation and runtime wiring are deferred per the included ExecPlan. ExecPlan
Core API & Schema (crates/weaver-cards/src/graph_slice)
Daemon & CLI integration
Tests, Fixtures & BDD
Documentation & Exports
Review notes, applied refactors and design decisions
Commit highlights
Impact
WalkthroughAdd a new observe graph-slice feature: define stable JSONL request/response schemas, implement CLI parsing and helpers, introduce SliceBudget and graph-slice types, wire CLI discoverability and daemon routing/handler (emits not_yet_implemented refusal), and add unit, snapshot, BDD and integration tests and fixtures. Changes
Sequence Diagram(s)sequenceDiagram
rect rgba(200,220,255,0.5)
participant CLI as "CLI / Client"
participant Router as "Daemon Router"
participant Handler as "graph-slice Handler"
participant Parser as "GraphSliceRequest::parse"
participant Writer as "ResponseWriter (stdout)"
end
CLI->>Router: invoke observe graph-slice + args
Router->>Handler: route to graph-slice handler
Handler->>Parser: parse arguments
Parser-->>Handler: Ok(GraphSliceRequest) / Err(GraphSliceError)
Handler->>Handler: construct GraphSliceResponse::not_yet_implemented()
Handler->>Writer: serialize JSON and write_stdout
Writer-->>CLI: JSONL stdout envelope
Handler-->>Router: return DispatchResult(status)
Poem
🚥 Pre-merge checks | ✅ 5 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Reviewer's GuideIntroduce a stable, serde-annotated JSONL contract for the Sequence diagram for observe graph-slice request handlingsequenceDiagram
actor User
participant WeaverCLI
participant WeaverdRouter
participant GraphSliceHandler
participant WeaverCards as WeaverCardsSchemas
User->>WeaverCLI: run weaver observe graph-slice --uri ... --position ...
WeaverCLI->>WeaverdRouter: JSONL CommandRequest(operation="graph-slice", arguments)
WeaverdRouter->>GraphSliceHandler: route to handle(request, writer)
GraphSliceHandler->>WeaverCards: GraphSliceRequest::parse(arguments)
alt arguments valid
WeaverCards-->>GraphSliceHandler: GraphSliceRequest
GraphSliceHandler->>WeaverCards: GraphSliceResponse::not_yet_implemented()
WeaverCards-->>GraphSliceHandler: GraphSliceResponse::Refusal(NotYetImplemented)
GraphSliceHandler->>WeaverdRouter: DispatchResult(status=1), write JSONL response
WeaverdRouter-->>WeaverCLI: JSONL refusal payload
WeaverCLI-->>User: print structured NotYetImplemented refusal
else arguments invalid
WeaverCards-->>GraphSliceHandler: GraphSliceError
GraphSliceHandler->>WeaverdRouter: DispatchError::invalid_arguments
WeaverdRouter-->>WeaverCLI: JSONL error response
WeaverCLI-->>User: print invalid-arguments error
end
Class diagram for new graph_slice schema typesclassDiagram
class SliceBudget {
+u32 cards
+u32 edges
+u32 estimated_tokens
+max_cards() u32
+max_edges() u32
+max_estimated_tokens() u32
}
class GraphSliceRequest {
+String uri
+u32 line
+u32 column
+u32 depth
+SliceDirection direction
+Vec~SliceEdgeType~ edge_types
+f64 min_confidence
+SliceBudget budget
+DetailLevel entry_detail
+DetailLevel node_detail
+parse(arguments Vec~String~) GraphSliceRequest
+uri() &str
+line() u32
+column() u32
+depth() u32
+direction() SliceDirection
+edge_types() &Vec~SliceEdgeType~
+min_confidence() f64
+budget() &SliceBudget
+entry_detail() DetailLevel
+node_detail() DetailLevel
}
class SliceDirection {
<<enum>>
In
Out
Both
}
class SliceEdgeType {
<<enum>>
Call
Import
Config
+all() [SliceEdgeType]
}
class SliceParseError {
+String kind
+String expected
+String name
}
class GraphSliceError {
<<enum>>
MissingArgument
InvalidValue
UnknownArgument
}
class ResolutionScope {
<<enum>>
FullSymbolTable
PartialSymbolTable
Lsp
}
class EdgeProvenance {
+String source
+Option~EdgeProvenanceDetails~ details
}
class EdgeProvenanceDetails {
+CallSiteInfo call_site
}
class CallSiteInfo {
+String uri
+u32 line
+u32 column
}
class ExternalTarget {
+String language
+String name
}
class SliceEdge {
+u32 edge_version
+SliceEdgeType edge_type
+String from
+Option~String~ to
+Option~ExternalTarget~ to_external
+f64 confidence
+SliceDirection direction
+ResolutionScope resolution_scope
+EdgeProvenance provenance
}
class SliceEntry {
+String symbol_id
}
class SliceConstraints {
+u32 depth
+SliceDirection direction
+Vec~SliceEdgeType~ edge_types
+f64 min_confidence
+SliceBudget budget
+DetailLevel entry_detail
+DetailLevel node_detail
}
class SpilloverCandidate {
+String symbol_id
+u32 depth
}
class SliceSpillover {
+bool truncated
+Vec~SpilloverCandidate~ frontier
+empty() SliceSpillover
}
class SliceRefusalReason {
<<enum>>
NoSymbolAtPosition
PositionOutOfRange
UnsupportedLanguage
NotYetImplemented
BackendUnavailable
}
class SliceRefusal {
+SliceRefusalReason reason
+String message
}
class GraphSliceResponse {
<<enum_like>>
Success
Refusal
+not_yet_implemented() GraphSliceResponse
}
GraphSliceRequest --> SliceBudget
GraphSliceRequest --> SliceDirection
GraphSliceRequest --> SliceEdgeType
GraphSliceRequest --> DetailLevel
SliceEdge --> SliceEdgeType
SliceEdge --> SliceDirection
SliceEdge --> ResolutionScope
SliceEdge --> EdgeProvenance
SliceEdge --> ExternalTarget
EdgeProvenance --> EdgeProvenanceDetails
EdgeProvenanceDetails --> CallSiteInfo
SliceConstraints --> SliceBudget
SliceConstraints --> SliceDirection
SliceConstraints --> SliceEdgeType
SliceConstraints --> DetailLevel
SliceSpillover --> SpilloverCandidate
GraphSliceResponse --> SliceEntry
GraphSliceResponse --> SliceConstraints
GraphSliceResponse --> SliceEdge
GraphSliceResponse --> SliceSpillover
GraphSliceResponse --> SliceRefusal
GraphSliceError --> SliceParseError
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![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.
- Introduce stable JSONL schema types for graph-slice requests, responses, budget constraints, edges, and refusals in weaver-cards. - Implement parsing and normalization of graph-slice request arguments, including traversal depth, direction, edge types, and budget constraints. - Add response types supporting success with cards, edges, spillover metadata, and structured refusals. - Provide a dispatch handler in weaverd that parses graph-slice requests and currently returns a NotYetImplemented refusal. - Add comprehensive behaviour-driven and snapshot tests validating schema contracts and request parsing. - Integrate graph-slice support in CLI discoverability and router dispatch. This initial support lays groundwork for future graph-slice traversal engine implementation. 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.
…request flags - Introduced RequestBuilder in parse.rs to encapsulate flag parsing logic - Moved parsing helpers into parse module for better separation of concerns - Refactored GraphSliceRequest::parse to utilize RequestBuilder - Added rich validation, normalization, and defaulting of flags - Updated SliceBudget field names and ser/de annotation for clarity - Added extensive unit tests for parsing logic - Updated CLI and dispatch code to support new parsing logic - Made minor stylistic improvements and test refactors This change improves maintenance, testability, and extensibility of graph-slice request parsing. 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.
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. Include the file and symbol names indicated in the issue at the head of your response. crates/weaver-cards/src/graph_slice/request_tests.rs Comment on file fn normalizes_duplicate_edge_types() {
let arguments = args(&[
"--uri",
"file:///src/main.rs",
"--position",
"1:1",
"--edge-types",
"import,call,import",
]);
let request = GraphSliceRequest::parse(&arguments).expect("should parse");
assert_eq!(
request.edge_types(),
&[SliceEdgeType::Call, SliceEdgeType::Import]
);
}❌ 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. Include the file and symbol names indicated in the issue at the head of your response. crates/weaverd/src/dispatch/observe/graph_slice_tests.rs Comment on file if let Ok(envelope) = serde_json::from_str::<serde_json::Value>(line)
&& envelope.get("stream").and_then(|v| v.as_str()) == Some("stdout")
&& let Some(data) = envelope.get("data").and_then(|v| v.as_str())❌ New issue: Complex Conditional |
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. Include the file and symbol names indicated in the issue at the head of your response. crates/weaver-cards/src/graph_slice/parse.rs Comment on file //! Argument parsing internals for `observe graph-slice` requests.
❌ New issue: String Heavy Function Arguments |
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. Include the file and symbol names indicated in the issue at the head of your response. crates/weaver-cards/src/graph_slice/request.rs Comment on file pub(super) const fn new(
uri: String,
line: u32,
column: u32,
depth: u32,
direction: SliceDirection,
edge_types: Vec<SliceEdgeType>,
min_confidence: f64,
budget: SliceBudget,
entry_detail: DetailLevel,
node_detail: DetailLevel,
) -> Self {
Self {
uri,
line,
column,
depth,
direction,
edge_types,
min_confidence,
budget,
entry_detail,
node_detail,
}
}❌ New issue: Excess Number of Function Arguments |
This comment was marked as resolved.
This comment was marked as resolved.
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. Include the file and symbol names indicated in the issue at the head of your response. crates/weaver-cards/src/graph_slice/request.rs Comment on file fn from_str(s: &str) -> Result<Self, Self::Err> {
match s {
"in" => Ok(Self::In),
"out" => Ok(Self::Out),
"both" => Ok(Self::Both),
_ => Err(DirectionParseError {
name: String::from(s),
}),
}
}❌ 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.
- Introduced a Flag enum for recognized CLI flags with Display and From implementations, improving error reporting consistency. - Refactored parsing functions and request builder to use Flag enum instead of raw strings. - Consolidated different parse error types into a single SliceParseError with detailed context. - Removed redundant constructors and simplified GraphSliceRequest internals. - Improved test coverage with parameterized tests for edge type normalization. - Updated related handler and documentation formatting to align with code changes. This refactor enhances code maintainability, improves error messages, and prepares the codebase for future extensions. 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.
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. Include the file and symbol names indicated in the issue at the head of your response. crates/weaver-cards/src/graph_slice/parse.rs Comment on file //! Argument parsing internals for `observe graph-slice` requests.
❌ New issue: String Heavy Function Arguments |
This comment was marked as resolved.
This comment was marked as resolved.
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.
Replaced raw &str arguments with RawValue struct that bundles the flag and its string value for parsing. This improves error reporting by providing accurate flag context in error messages across all parse helper functions in graph_slice::parse.rs. 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.
…cation Extracted a generic helper function `parse_with_fromstr` to simplify parsing logic for types implementing `FromStr`. Updated `parse_direction` and `parse_detail` to use this helper, reducing boilerplate and improving code clarity in parse.rs. 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: 3
♻️ Duplicate comments (1)
docs/execplans/7-2-1-define-stable-jsonl-request-and-response-schemas-for-observe-graph-slice.md (1)
19-20:⚠️ Potential issue | 🟡 MinorRemove the misplaced comma in the purpose sentence.
Delete the comma after
edgesso the clause reads naturally and unambiguously.✏️ Proposed fix
-describes the applied constraints, the cards and edges, that fit inside those +describes the applied constraints, the cards and edges that fit inside thoseTriage:
[type:grammar][type:docstyle]As per coding guidelines, documentation must use en-GB-oxendict spelling and
grammar, and follow the documentation style guide for writing, formatting,
grammar, and structure standards in repository documentation.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/execplans/7-2-1-define-stable-jsonl-request-and-response-schemas-for-observe-graph-slice.md` around lines 19 - 20, Remove the misplaced comma in the sentence "describes the applied constraints, the cards and edges, that fit inside those constraints, and any spillover that was excluded when the traversal truncated." — edit the phrase "the cards and edges, that fit" to "the cards and edges that fit" so the clause reads naturally and follows en-GB-oxendict grammar and the repo documentation style guide.
🤖 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-cards/src/graph_slice/parse.rs`:
- Around line 74-91: The Rustdoc for try_apply_known_flag is stale: update the
comment to state that the function returns Ok(true) if the flag was recognised
and handled (via try_apply_traversal_flag, try_apply_budget_flag, or
try_apply_detail_flag) and Ok(false) if the flag is unknown — and importantly
that it does not advance or consume the iterator when returning false (it only
returns the boolean and propagates GraphSliceError on failure). Adjust the doc
text to accurately describe this contract so callers don't assume the iterator
is skipped on unknown flags.
In
`@docs/execplans/7-2-1-define-stable-jsonl-request-and-response-schemas-for-observe-graph-slice.md`:
- Around line 422-423: Update the markdown link so the fixture-catalogue link
target matches its link text: change the hyperlink target to
../../crates/weaver-e2e/src/graph_slice_fixtures/ and use inline link syntax
[crates/weaver-e2e/src/graph_slice_fixtures/](../../crates/weaver-e2e/src/graph_slice_fixtures/)
so the displayed "module tree for graph-shaped workspaces" points to the exact
directory referenced.
- Around line 26-29: The markdown line listing default request fields contains
malformed inline code spans with leading spaces and stray backticks; fix it by
rewriting the wrapped line so each field name is a proper inline code token
(e.g., `depth`, `direction`, `edge_types`, `min_confidence`, `budget.max_cards`,
`budget.max_edges`, `budget.max_estimated_tokens`, `entry_detail`,
`node_detail`) separated by commas (or as a bulleted list) with no extra
backticks or spaces so the tokens render correctly and pass markdownlint.
---
Duplicate comments:
In
`@docs/execplans/7-2-1-define-stable-jsonl-request-and-response-schemas-for-observe-graph-slice.md`:
- Around line 19-20: Remove the misplaced comma in the sentence "describes the
applied constraints, the cards and edges, that fit inside those constraints, and
any spillover that was excluded when the traversal truncated." — edit the phrase
"the cards and edges, that fit" to "the cards and edges that fit" so the clause
reads naturally and follows en-GB-oxendict grammar and the repo documentation
style guide.
🪄 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: 229bd025-0ce5-48e8-b8c3-e6c00884e249
📒 Files selected for processing (2)
crates/weaver-cards/src/graph_slice/parse.rsdocs/execplans/7-2-1-define-stable-jsonl-request-and-response-schemas-for-observe-graph-slice.md
…tion Improve readability and fix minor formatting issues in the documentation for stable JSONL request and response schemas for observe graph-slice. Update comments in parse.rs to better describe the behavior of try_apply_known_flag. Fix links and format for improved clarity in execution plan docs. 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: 6
🤖 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-cards/src/graph_slice/parse.rs`:
- Line 18: Update the doc comment string in parse.rs that reads "Identifies a
recognised CLI flag for error-reporting purposes." to use en-GB-oxendict
spelling by replacing "recognised" with "recognized" so the comment becomes
"Identifies a recognized CLI flag for error-reporting purposes."
- Around line 73-80: Update the doc comment in impl RequestBuilder: replace the
word "recognised" with "recognized" in the summary sentence mentioning
try_apply_traversal_flag, try_apply_budget_flag, and try_apply_detail_flag so
the function comment reads "Returns `Ok(true)` if `flag` was recognized and
handled...". Ensure both occurrences in that comment (the sentence on handling
and the following sentence if repeated) are updated so the text around
RequestBuilder, try_apply_traversal_flag, try_apply_budget_flag, and
try_apply_detail_flag remains unchanged.
In
`@docs/execplans/7-2-1-define-stable-jsonl-request-and-response-schemas-for-observe-graph-slice.md`:
- Line 199: Insert a comma before "so" in the sentence "during implementation so
the design stays authoritative." to read "during implementation, so the design
stays authoritative."; locate that exact sentence in the document (the line
containing "during implementation so the design stays authoritative.") and
update the punctuation accordingly.
- Line 441: The sentence fragment "Keep snapshot names aligned with the fixture
identifiers so regressions are" is missing a comma before the coordinating
conjunction; insert a comma immediately before "so" to read "Keep snapshot names
aligned with the fixture identifiers, so regressions are" to correctly separate
the independent clauses and fix the grammar in the line shown.
- Line 164: Edit the sentence containing "reflected back into the design
document" and remove the redundant word "back" so it reads "reflected into the
design document"; update the sentence in the
docs/execplans/7-2-1-define-stable-jsonl-request-and-response-schemas-for-observe-graph-slice.md
where the phrase appears to maintain concise wording.
- Line 446: The sentence "Update the design doc first so the contract decisions
are recorded where future" is a compound sentence missing a comma before "so";
edit that sentence to insert a comma before "so" (i.e., "...first, so the
contract decisions are recorded...") to correctly separate the two independent
clauses.
🪄 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: 0c09f845-0aa4-4105-b770-50afbc9a2f49
📒 Files selected for processing (2)
crates/weaver-cards/src/graph_slice/parse.rsdocs/execplans/7-2-1-define-stable-jsonl-request-and-response-schemas-for-observe-graph-slice.md
Corrected minor grammatical errors and punctuation in the design and roadmap markdown files to improve clarity and consistency. Also fixed spelling of 'recognized' in code comments in graph_slice parse module. 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: 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 `@crates/weaver-cards/src/graph_slice/parse.rs`:
- Around line 125-128: The parser currently stores any text for "--uri" via
require_arg_value and String::from(raw.value); tighten validation by adding a
parse_uri helper and using it in the "--uri" arm (replace self.uri =
Some(String::from(raw.value)) with self.uri = Some(parse_uri(raw)?)); parse_uri
should inspect raw.value, return Err(GraphSliceError::InvalidValue { flag:
raw.flag.into(), message: format!("expected a file URI, got: {value}") }) when
it doesn't start with "file://", and otherwise return the owned string;
reference require_arg_value, Flag::Uri, raw.value, parse_uri, and
GraphSliceError::InvalidValue when implementing.
- Around line 224-228: Replace the unstable reliance on Ord-based sorting for
edge_types: instead of calling edge_types.sort() in parse.rs, sort with a
canonical schema rank (e.g.,
edge_types.sort_by_key(SliceEdgeType::canonical_rank)) and keep
edge_types.dedup(); add a const fn canonical_rank(self) -> u8 on the
SliceEdgeType enum that returns explicit ranks (e.g., Call = 0, Import = 1,
Config = 2) so wire-order is deterministic regardless of enum declaration order.
In
`@docs/execplans/7-2-1-define-stable-jsonl-request-and-response-schemas-for-observe-graph-slice.md`:
- Line 8: The exec plan file still shows "Status: DRAFT" and unchecked
stage/approval items despite the PR already adding schemas, routing, tests and
docs; update the document to reflect the current implementation by changing
Status to the appropriate value (e.g., APPROVED or IMPLEMENTED), check off
Stages A–F in the stage checklist, and update the approval gate text to indicate
implementation has occurred and who approved; apply the same updates in the
other referenced ranges (lines ~120-139 and ~485-489) so the plan, stage
checklist, and approval statements accurately match the code changes introduced
in this PR.
🪄 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: 693f39ec-a086-4b95-95c6-fd27b33057ed
📒 Files selected for processing (2)
crates/weaver-cards/src/graph_slice/parse.rsdocs/execplans/7-2-1-define-stable-jsonl-request-and-response-schemas-for-observe-graph-slice.md
…ate file URI - Introduce `canonical_rank` for SliceEdgeType to ensure deterministic sorting - Sort and deduplicate edge types using the canonical rank - Add `parse_uri` function to validate that URIs start with 'file://' - Update ExecPlan status to IN_PROGRESS and mark several stages as completed 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: 2
🤖 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-cards/src/graph_slice/parse.rs`:
- Around line 1-402: Split out the value-level parsing helpers into a new
sibling submodule and keep RequestBuilder orchestration in parse.rs: move
RawValue, require_arg_value, and all parse_* helpers (parse_uri, parse_position,
parse_u32, parse_direction, parse_edge_types, parse_confidence,
parse_with_fromstr, parse_detail) into the new submodule file, make their
visibility compatible with RequestBuilder (pub(super) or crate-level as needed),
add a mod declaration and import those symbols into parse.rs, and update any
references in RequestBuilder (e.g., try_apply_traversal_flag,
try_apply_budget_flag, try_apply_detail_flag, build) to use the moved functions;
ensure the new module re-exports or exposes the helpers so the existing tests
and callers still compile.
In
`@docs/execplans/7-2-1-define-stable-jsonl-request-and-response-schemas-for-observe-graph-slice.md`:
- Around line 31-33: The docs mention the resolution_scope enum value `lsp`
which is inconsistent with the implemented schema; update all occurrences of the
documented enum for resolution_scope (the list containing `full_symbol_table`,
`partial_symbol_table`, `lsp`) to use `unresolved` instead of `lsp` so the docs
match the response contract (`full_symbol_table`, `partial_symbol_table`,
`unresolved`), e.g., update the enum examples and any explanatory text
referencing `lsp`.
🪄 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: 6329bb9d-5704-42a7-a937-3c396f20642d
📒 Files selected for processing (3)
crates/weaver-cards/src/graph_slice/parse.rscrates/weaver-cards/src/graph_slice/request.rsdocs/execplans/7-2-1-define-stable-jsonl-request-and-response-schemas-for-observe-graph-slice.md
…_helpers module Moved all value-level command-line argument parsing helper functions and the RawValue struct from parse.rs to a new parse_helpers.rs module. This improves modularity by separating low-level parsing logic from higher-level request building, clarifies code organization, and maintains existing functionality with more focused responsibility in each module. Updated parse.rs to import the helpers from the new module and added parse_helpers mod declaration. 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.
… helpers - Introduced extensive unit tests for functions in parse_helpers.rs - Covered argument value requirement, URI parsing, position parsing, unsigned integer parsing, direction, edge types, confidence levels, and detail level parsing - Improved validation coverage ensuring robust error handling on invalid inputs - Tests make use of rstest for parameterized cases - Overall increase in test coverage and stability of graph-slice request parsing 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: 6
🤖 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-cards/src/graph_slice/parse_helpers.rs`:
- Around line 1-443: Extract the entire #[cfg(test)] mod tests { ... } block
(which contains tests like require_arg_value_returns_value,
parse_uri_accepts_file_uri, parse_position_accepts_valid_pair,
parse_confidence_accepts_valid_range, parse_detail_accepts_valid_levels, etc.)
out of parse_helpers.rs and place it in its own test source file as a standalone
module so parse_helpers.rs falls under the 400-line limit; in the new test file
keep the same use imports (use rstest::rstest; use super::*;) or change to use
crate::graph_slice::parse_helpers::* as appropriate, ensure the test module
still compiles by adjusting visibility/imports for RawValue, require_arg_value,
parse_uri, parse_position, parse_u32, parse_direction, parse_edge_types,
parse_confidence, and parse_detail, and remove the original #[cfg(test)] block
from parse_helpers.rs.
- Around line 258-262: The failing CI is due to formatting drift in the test
file around the test function parse_position_rejects_zero_line and two other
nearby test blocks; run the project formatter and commit the result: run make
check-fmt (or cargo fmt / rustfmt) on the file that contains
parse_position_rejects_zero_line, correct formatting for the three hunks (the
test containing parse_position_rejects_zero_line and the two subsequent test
blocks), then re-run make lint and make test and commit the formatted changes so
CI passes.
- Around line 191-443: Tests call functions like require_arg_value, parse_uri,
parse_position, parse_u32, parse_direction, parse_edge_types, parse_confidence,
and parse_detail using .unwrap() and .unwrap_err(); replace each .unwrap() with
.expect("meaningful message describing the expected success, e.g. expected value
for --uri or expected valid parse of POSITION") and each .unwrap_err() with
.expect_err("meaningful message describing the expected failure, e.g. expected
error for invalid POSITION or unknown --edge-types"), updating messages to
reference the Flag/RawValue under test (e.g. Flag::Uri, Flag::Position,
Flag::EdgeTypes) so failures show clear context; do this for all occurrences in
the test module (including tests for RawValue::new and the various Flag cases).
In `@docs/developers-guide.md`:
- Around line 339-342: Update the first occurrence of "URI" in this section to
read "Uniform Resource Identifier (URI)" and then use "URI" for subsequent
mentions; specifically change the description for parse_uri to refer to a
"Uniform Resource Identifier (URI) prefix" on first use and keep later
references (including in parse_uri, require_arg_value, and parse_position
descriptions) as "URI". Ensure the phrase appears exactly once expanded and all
other instances use the abbreviation.
In `@docs/users-guide.md`:
- Around line 835-837: The sentence describing resolution_scope uses
non-parallel phrasing; rewrite it to a parallel form (e.g., using "either ... or
...") and use British English spelling/grammar; specifically, update the
sentence that mentions the values `full_symbol_table`, `partial_symbol_table`,
and `lsp` so it reads like "Each edge carries a `resolution_scope` that is
either `full_symbol_table`, `partial_symbol_table` or `lsp`, indicating how the
target was resolved." Ensure punctuation and conjunctions follow en-GB
standards.
- Around line 804-823: The example JSON uses "to" but the code exposes the
flattened target shape via SliceEdge (see SliceEdge and its #[serde(flatten)]
target in response.rs), so change the example to replace the "to" field with the
flattened "target" object structure used by SliceEdge (inline the target's
fields at the same level as other edge props), ensuring the example matches the
shape produced by the target flattening and includes the same keys/structure as
defined on SliceEdge's target type.
🪄 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: 22137196-8b35-4854-b8dd-aad1a0315855
📒 Files selected for processing (3)
crates/weaver-cards/src/graph_slice/parse_helpers.rsdocs/developers-guide.mddocs/users-guide.md
…stency - Introduced helper function `assert_err_contains` to simplify error assertion checks. - Replaced direct `unwrap`, `unwrap_err`, and manual error message inspections with helper for more concise tests. - Changed Vec to array literals for argument lists to reduce heap allocations. - Improved formatting and assertion descriptions for better test failure messages. - Overall cleanup enhances readability and maintainability of parsing tests. 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.
♻️ Duplicate comments (1)
crates/weaver-cards/src/graph_slice/parse_helpers.rs (1)
181-434:⚠️ Potential issue | 🟠 MajorSplit the in-file test module now to satisfy the hard 400-line cap.
Move
#[cfg(test)] mod testsinto a separate test file and keepparse_helpers.rsfocused on production parsing helpers.As per coding guidelines: "Files must not exceed 400 lines in length".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/weaver-cards/src/graph_slice/parse_helpers.rs` around lines 181 - 434, Extract the entire #[cfg(test)] mod tests { ... } block out of parse_helpers.rs into a new test file (e.g., src/graph_slice/parse_helpers_tests.rs) so parse_helpers.rs stays under the 400-line limit; in the new file replace use super::*; with use crate::graph_slice::parse_helpers::*; and keep the rstest import (use rstest::rstest;) at top, ensure any helper functions (like assert_err_contains) remain in the new test file, and remove the original #[cfg(test)] mod tests block from parse_helpers.rs.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@crates/weaver-cards/src/graph_slice/parse_helpers.rs`:
- Around line 181-434: Extract the entire #[cfg(test)] mod tests { ... } block
out of parse_helpers.rs into a new test file (e.g.,
src/graph_slice/parse_helpers_tests.rs) so parse_helpers.rs stays under the
400-line limit; in the new file replace use super::*; with use
crate::graph_slice::parse_helpers::*; and keep the rstest import (use
rstest::rstest;) at top, ensure any helper functions (like assert_err_contains)
remain in the new test file, and remove the original #[cfg(test)] mod tests
block from parse_helpers.rs.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 8fabeabb-8092-41b4-aace-031b20d49c4c
📒 Files selected for processing (1)
crates/weaver-cards/src/graph_slice/parse_helpers.rs
Remove inline tests from parse_helpers.rs and move them to a new parse_helpers_tests.rs file. This cleans up the main source file and organizes tests into a dedicated test module. Also update mod.rs to conditionally include the new test modules. Co-authored-by: devboxerhub[bot] <devboxerhub[bot]@users.noreply.github.com>
There was a problem hiding this comment.
♻️ Duplicate comments (3)
docs/execplans/7-2-1-define-stable-jsonl-request-and-response-schemas-for-observe-graph-slice.md (2)
26-29:⚠️ Potential issue | 🟡 MinorFix broken inline code tokens in the defaults list.
Rewrite this wrapped sentence so every field name is a valid inline code span without stray leading spaces or detached backticks.
✏️ Proposed fix
-1. `weaver observe graph-slice --uri <Uniform Resource Identifier (URI)> - --position - <LINE:COL>` parses into a typed request with explicit default values for ` - depth`, `direction`, `edge_types`, `min_confidence`, `budget.max_cards`, ` - budget.max_edges`, `budget.max_estimated_tokens`, `entry_detail`, and ` - node_detail`. +1. `weaver observe graph-slice --uri <Uniform Resource Identifier (URI)> + --position <LINE:COL>` parses into a typed request with explicit default + values for `depth`, `direction`, `edge_types`, `min_confidence`, + `budget.max_cards`, `budget.max_edges`, `budget.max_estimated_tokens`, + `entry_detail`, and `node_detail`.Triage:
[type:syntax/md][type:docstyle]
As per coding guidelines, follow markdownlint-style Markdown syntax and keep inline code spans well-formed in documentation.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/execplans/7-2-1-define-stable-jsonl-request-and-response-schemas-for-observe-graph-slice.md` around lines 26 - 29, The sentence listing default request fields has broken inline code spans; rewrite it so each field name is wrapped as a proper inline code span (no leading spaces or stray backticks) — specifically ensure `depth`, `direction`, `edge_types`, `min_confidence`, `budget.max_cards`, `budget.max_edges`, `budget.max_estimated_tokens`, `entry_detail`, and `node_detail` appear as well-formed inline code tokens and the sentence flows naturally without detached backticks or extra spaces.
148-152:⚠️ Potential issue | 🟠 MajorUpdate stale discovery notes to match completed implementation.
Replace this claim that no
observe graph-sliceimplementation exists. It now conflicts with Line 130-Line 140 and Line 492-Line 496, which state Stages A-D are complete and routing is already wired.✏️ Proposed fix
-- There is no `observe graph-slice` implementation yet anywhere in the - codebase. The current observe router in - [crates/weaverd/src/dispatch/router.rs](../../crates/weaverd/src/dispatch/router.rs) - only recognizes `get-definition`, `find-references`, `grep`, `diagnostics`, - `call-hierarchy`, and `get-card`. +- `observe graph-slice` is now wired into the router and dispatch path. + Remaining work sits in end-to-end fixture breadth and final documentation + synchronization before the roadmap item is fully closed.Triage:
[type:docstyle]
As per coding guidelines, ensure documentation reflects current API and behavioural state indocs/.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/execplans/7-2-1-define-stable-jsonl-request-and-response-schemas-for-observe-graph-slice.md` around lines 148 - 152, The doc contains a stale assertion that "There is no `observe graph-slice` implementation"; remove or replace that sentence in docs/execplans/7-2-1-define-stable-jsonl-request-and-response-schemas-for-observe-graph-slice.md and update the discovery notes to state that `observe graph-slice` is implemented and routing is wired in the observe router (referencing the router in crates/weaverd/src/dispatch/router.rs) and that Stages A–D are complete; ensure the new text aligns with the existing statements about Stages A–D and the routing references elsewhere in the document.docs/users-guide.md (1)
803-808:⚠️ Potential issue | 🟠 MajorAlign the edge example with the actual graph-slice response schema.
Replace the
tofield with the flattened target shape emitted bySliceEdge; this example currently documents a payload shape that does not match the stable contract.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/users-guide.md` around lines 803 - 808, The example edge JSON in docs/users-guide.md uses a "to" field that doesn't match the actual graph-slice response; update the sample to replace the "to" key with the flattened target shape produced by SliceEdge (i.e., use the exact flattened target properties emitted by the SliceEdge implementation rather than a single "to" id), ensuring fields and nesting mirror the SliceEdge output schema so the documented payload matches the stable contract.
🤖 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/7-2-1-define-stable-jsonl-request-and-response-schemas-for-observe-graph-slice.md`:
- Around line 26-29: The sentence listing default request fields has broken
inline code spans; rewrite it so each field name is wrapped as a proper inline
code span (no leading spaces or stray backticks) — specifically ensure `depth`,
`direction`, `edge_types`, `min_confidence`, `budget.max_cards`,
`budget.max_edges`, `budget.max_estimated_tokens`, `entry_detail`, and
`node_detail` appear as well-formed inline code tokens and the sentence flows
naturally without detached backticks or extra spaces.
- Around line 148-152: The doc contains a stale assertion that "There is no
`observe graph-slice` implementation"; remove or replace that sentence in
docs/execplans/7-2-1-define-stable-jsonl-request-and-response-schemas-for-observe-graph-slice.md
and update the discovery notes to state that `observe graph-slice` is
implemented and routing is wired in the observe router (referencing the router
in crates/weaverd/src/dispatch/router.rs) and that Stages A–D are complete;
ensure the new text aligns with the existing statements about Stages A–D and the
routing references elsewhere in the document.
In `@docs/users-guide.md`:
- Around line 803-808: The example edge JSON in docs/users-guide.md uses a "to"
field that doesn't match the actual graph-slice response; update the sample to
replace the "to" key with the flattened target shape produced by SliceEdge
(i.e., use the exact flattened target properties emitted by the SliceEdge
implementation rather than a single "to" id), ensuring fields and nesting mirror
the SliceEdge output schema so the documented payload matches the stable
contract.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 7ad95162-b5ea-443c-b50c-1248f38d7b73
📒 Files selected for processing (8)
crates/weaver-cards/src/graph_slice/mod.rscrates/weaver-cards/src/graph_slice/parse_helpers.rscrates/weaver-cards/src/graph_slice/parse_helpers_tests.rscrates/weaver-cards/src/graph_slice/request.rscrates/weaver-cards/src/graph_slice/request_tests.rsdocs/developers-guide.mddocs/execplans/7-2-1-define-stable-jsonl-request-and-response-schemas-for-observe-graph-slice.mddocs/users-guide.md
Owner
Author
|
@coderabbitai Have the following now been resolved? If further work is required, please provide an AI agent prompt for the remaining work to be done to address this comment. ❌ Failed checks (2 warnings)
|
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 5874
Developer Documentation ✅ Module-Level Documentation ✅
No further work is required for either warning. |
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
Rationale
Plan of work (high level)
Notes for reviewers
📎 Task: https://www.devboxer.com/task/265b9025-1af3-4c49-98b6-bb0ed6fcbb98