feat(morphology): add advisory message passing operation - #3
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fd9e4047db
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } | ||
| fn estimate(&self, counts: GraphCounts, _args: &Value) -> MemoryEstimate { | ||
| // Feature table + accumulator table, bounded by the existing graph size. | ||
| estimate_from_coefficients(counts, 8192, 96, 192, 48, "Morphological message passing") |
There was a problem hiding this comment.
For morphological message passing, the working set is not bounded by node/edge counts alone: run clones every feature vector and builds accumulator vectors, so memory scales with feature_dimension * node_count (often hundreds or thousands of f64s per node). This fixed 96/192 bytes-per-node estimate can be orders of magnitude too low for realistic embeddings, which defeats the estimate-before-run gate and can let jobs through that allocate far more memory than reported.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/rustyred-core/src/algorithm_ops.rs`:
- Around line 952-959: The empty morphological_edges check in algorithm_ops.rs
returns a fixed response structure with rows, node_count, and edge_count fields
regardless of the operation mode. When mode is mutate, the response should match
the contract used in the non-empty branch with nodes_written and mutate_property
fields instead. Modify the return statement in the is_empty() check to
conditionally format the response based on whether mode is mutate mode or not,
ensuring mutate mode returns the appropriate mutate-shaped payload with
nodes_written set to 0 and the correct mutate_property field.
In `@crates/rustyred-core/src/morphology.rs`:
- Around line 73-84: In the from_edge_record method, the confidence field is
directly assigned the result of edge.effective_confidence() without clamping it
to the valid range of 0.0..=1.0. To preserve morphology weight invariants, clamp
the confidence value returned by edge.effective_confidence() to ensure it stays
within the bounds of 0.0..=1.0 before assigning it to the confidence field in
the struct initialization.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 776af787-d5fb-46fc-a1b4-9279f5ad4d3a
📒 Files selected for processing (3)
crates/rustyred-core/src/algorithm_ops.rscrates/rustyred-core/src/lib.rscrates/rustyred-core/src/morphology.rs
| if morphological_edges.is_empty() { | ||
| return Ok(json!({ | ||
| "operation": self.command(), | ||
| "mode": mode.as_str(), | ||
| "node_count": 0, | ||
| "edge_count": 0, | ||
| "rows": [], | ||
| })); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Return a mutate-shaped payload when mode is mutate and no edges are found.
Lines 952-959 currently return rows/node_count for every mode. In mutate mode, this breaks the operation’s own response contract (nodes_written, mutate_property) used in the non-empty branch.
Proposed fix
- if morphological_edges.is_empty() {
- return Ok(json!({
- "operation": self.command(),
- "mode": mode.as_str(),
- "node_count": 0,
- "edge_count": 0,
- "rows": [],
- }));
- }
+ if morphological_edges.is_empty() {
+ return Ok(match mode {
+ OperationMode::Stream => json!({
+ "operation": self.command(),
+ "mode": "stream",
+ "edge_count": 0,
+ "node_count": 0,
+ "rows": [],
+ }),
+ OperationMode::Mutate => json!({
+ "operation": self.command(),
+ "mode": "mutate",
+ "edge_count": 0,
+ "nodes_written": 0,
+ "mutate_property": arg_str(args, "mutate_property")
+ .unwrap_or("morphological_embedding"),
+ }),
+ OperationMode::Stats | OperationMode::Estimate => unreachable!("handled earlier"),
+ });
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if morphological_edges.is_empty() { | |
| return Ok(json!({ | |
| "operation": self.command(), | |
| "mode": mode.as_str(), | |
| "node_count": 0, | |
| "edge_count": 0, | |
| "rows": [], | |
| })); | |
| if morphological_edges.is_empty() { | |
| return Ok(match mode { | |
| OperationMode::Stream => json!({ | |
| "operation": self.command(), | |
| "mode": "stream", | |
| "edge_count": 0, | |
| "node_count": 0, | |
| "rows": [], | |
| }), | |
| OperationMode::Mutate => json!({ | |
| "operation": self.command(), | |
| "mode": "mutate", | |
| "edge_count": 0, | |
| "nodes_written": 0, | |
| "mutate_property": arg_str(args, "mutate_property") | |
| .unwrap_or("morphological_embedding"), | |
| }), | |
| OperationMode::Stats | OperationMode::Estimate => unreachable!("handled earlier"), | |
| }); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/rustyred-core/src/algorithm_ops.rs` around lines 952 - 959, The empty
morphological_edges check in algorithm_ops.rs returns a fixed response structure
with rows, node_count, and edge_count fields regardless of the operation mode.
When mode is mutate, the response should match the contract used in the
non-empty branch with nodes_written and mutate_property fields instead. Modify
the return statement in the is_empty() check to conditionally format the
response based on whether mode is mutate mode or not, ensuring mutate mode
returns the appropriate mutate-shaped payload with nodes_written set to 0 and
the correct mutate_property field.
| pub fn from_edge_record(edge: &EdgeRecord) -> Option<Self> { | ||
| let relation = normalize_relation(&edge.edge_type); | ||
| let (source_kind, target_kind) = relation_kinds(&relation)?; | ||
| Some(Self { | ||
| edge_id: edge.id.clone(), | ||
| source_id: edge.from_id.clone(), | ||
| source_kind, | ||
| relation, | ||
| target_id: edge.to_id.clone(), | ||
| target_kind, | ||
| confidence: edge.effective_confidence(), | ||
| }) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Clamp ingested edge confidence to preserve morphology weight invariants.
Line 83 copies effective_confidence() directly; if an EdgeRecord carries out-of-range confidence, message weights drift outside the intended 0.0..=1.0 band.
Proposed fix
Some(Self {
edge_id: edge.id.clone(),
source_id: edge.from_id.clone(),
source_kind,
relation,
target_id: edge.to_id.clone(),
target_kind,
- confidence: edge.effective_confidence(),
+ confidence: edge.effective_confidence().clamp(0.0, 1.0),
})📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| pub fn from_edge_record(edge: &EdgeRecord) -> Option<Self> { | |
| let relation = normalize_relation(&edge.edge_type); | |
| let (source_kind, target_kind) = relation_kinds(&relation)?; | |
| Some(Self { | |
| edge_id: edge.id.clone(), | |
| source_id: edge.from_id.clone(), | |
| source_kind, | |
| relation, | |
| target_id: edge.to_id.clone(), | |
| target_kind, | |
| confidence: edge.effective_confidence(), | |
| }) | |
| pub fn from_edge_record(edge: &EdgeRecord) -> Option<Self> { | |
| let relation = normalize_relation(&edge.edge_type); | |
| let (source_kind, target_kind) = relation_kinds(&relation)?; | |
| Some(Self { | |
| edge_id: edge.id.clone(), | |
| source_id: edge.from_id.clone(), | |
| source_kind, | |
| relation, | |
| target_id: edge.to_id.clone(), | |
| target_kind, | |
| confidence: edge.effective_confidence().clamp(0.0, 1.0), | |
| }) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/rustyred-core/src/morphology.rs` around lines 73 - 84, In the
from_edge_record method, the confidence field is directly assigned the result of
edge.effective_confidence() without clamping it to the valid range of 0.0..=1.0.
To preserve morphology weight invariants, clamp the confidence value returned by
edge.effective_confidence() to ensure it stays within the bounds of 0.0..=1.0
before assigning it to the confidence field in the struct initialization.
1 participant
Summary
rustyred-core::morphologytyped morphological edge/topology/message-passing primitivesrustyred.algorithm.morphological_message_passingValidation
cargo check -p rustyred-corecargo test -p rustyred-core morphologycargo test -p rustyred-core morphological_message_passing_stream_stats_and_mutateSummary by CodeRabbit
New Features