Allow for multiple SDNA classes with same name by same author#621
Merged
Allow for multiple SDNA classes with same name by same author#621
Conversation
Contributor
WalkthroughCentralized Prolog query execution via a new prolog_query_helper and updated prolog_query* methods; expanded GraphQL type imports; simplified add_sdna to always create two Shared links and return Ok(true); minor import cleanup and doc/comment adjustments. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Client
participant PerspectiveInstance
participant PrologServicePool as Prolog Engine Pool
participant PrologService
rect rgb(242,248,255)
note over Client,PerspectiveInstance: Prolog query (generic wrapper flow)
Client->>PerspectiveInstance: prolog_query*(query, options)
activate PerspectiveInstance
PerspectiveInstance->>PerspectiveInstance: prolog_query_helper(query, use_lock, pool_provider, executor)
Note right of PerspectiveInstance: resolve pool name via pool_provider
PerspectiveInstance->>PrologServicePool: ensure/prepare pool
PrologServicePool-->>PerspectiveInstance: pool ready
PerspectiveInstance->>PrologServicePool: get PrologService (by pool)
PrologServicePool-->>PerspectiveInstance: PrologService
PerspectiveInstance->>PrologService: executor(query, lock?)
PrologService-->>PerspectiveInstance: Result<QueryResolution or error>
PerspectiveInstance-->>Client: QueryResolution or error
deactivate PerspectiveInstance
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
🚨 Bugbot Trial ExpiredYour Bugbot trial has expired. Please purchase a license in the Cursor dashboard to continue using Bugbot. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
rust-executor/src/perspectives/perspective_instance.rs (2)
6-6: Importdid()to enable author-scoped idempotency in add_sdna (used in suggested fix below).This keeps the door open for checking the current agent’s DID when avoiding duplicate links.
Apply together with the add_sdna change below:
-use crate::agent::{create_signed_expression}; +use crate::agent::{create_signed_expression, did};
1158-1215: Allow multiple SDNA implementations per class, but keep the “self → class name” link idempotent and avoid exact duplicate “class → sdna_code” links (by same author). Return whether anything was added.Unconditionally adding both links on every call will create duplicate “has_subject_class” links and always return true. The change below:
- keeps multiple implementations allowed (second link can accumulate),
- prevents redundant duplicates for the same author,
- returns a truthful bool.
@@ ) -> Result<bool, AnyError> { - //let mut added = false; + // Keep SDNA updates serialized let mutex = self.sdna_change_mutex.clone(); let _guard = mutex.lock().await; @@ let literal_name = Literal::from_string(name) .to_url() .expect("just initialized Literal couldn't be turned into URL"); - - let mut sdna_links: Vec<Link> = Vec::new(); if (Literal::from_url(sdna_code.clone())).is_err() { sdna_code = Literal::from_string(sdna_code) .to_url() .expect("just initialized Literal couldn't be turned into URL"); } - // let links = self - // .get_links(&LinkQuery { - // source: Some("ad4m://self".to_string()), - // predicate: Some(predicate.to_string()), - // target: Some(literal_name.clone()), - // from_date: None, - // until_date: None, - // limit: None, - // }) - // .await?; - // let author = agent::did(); - // let links = links - // .into_iter() - // .filter(|l| l.author == author) - // .collect::<Vec<DecoratedLinkExpression>>(); - //if links.is_empty() { - sdna_links.push(Link { - source: "ad4m://self".to_string(), - predicate: Some(predicate.to_string()), - target: literal_name.clone(), - }); + // Author-scoped idempotency for "self -> class literal" + let my_did = did(); + let existing_self_links = self + .get_links(&LinkQuery { + source: Some("ad4m://self".to_string()), + predicate: Some(predicate.to_string()), + target: Some(literal_name.clone()), + ..Default::default() + }) + .await?; + let have_self_link_for_author = existing_self_links.iter().any(|l| l.author == my_did); + if !have_self_link_for_author { + sdna_links.push(Link { + source: "ad4m://self".to_string(), + predicate: Some(predicate.to_string()), + target: literal_name.clone(), + }); + } - sdna_links.push(Link { - source: literal_name.clone(), - predicate: Some("ad4m://sdna".to_string()), - target: sdna_code, - }); + // Allow multiple versions, but avoid exact duplicates for this author + let existing_sdna_links = self + .get_links(&LinkQuery { + source: Some(literal_name.clone()), + predicate: Some("ad4m://sdna".to_string()), + target: Some(sdna_code.clone()), + ..Default::default() + }) + .await?; + let have_exact_sdna_for_author = existing_sdna_links.iter().any(|l| l.author == my_did); + if !have_exact_sdna_for_author { + sdna_links.push(Link { + source: literal_name.clone(), + predicate: Some("ad4m://sdna".to_string()), + target: sdna_code, + }); + } - self.add_links(sdna_links, LinkStatus::Shared, None).await?; - //added = true; - //} + if sdna_links.is_empty() { + return Ok(false); + } + self.add_links(sdna_links, LinkStatus::Shared, None).await?; // Mutex guard is automatically dropped here - Ok(true) + Ok(true) }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
rust-executor/src/perspectives/perspective_instance.rs(3 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
rust-executor/src/perspectives/perspective_instance.rs (4)
rust-executor/src/agent/mod.rs (2)
create_signed_expression(59-74)create_signed_expression(219-226)rust-executor/src/db.rs (1)
new(59-222)core/src/Literal.ts (1)
Literal(9-84)rust-client/src/literal.rs (2)
from_url(27-36)from_string(38-43)
Contributor
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
rust-executor/src/perspectives/perspective_instance.rs (1)
1196-1210: Allow multiple SDNA impls, but dedupe noise: keep the “self → class-name” link unique and skip exact duplicate sdna linksThis keeps the policy aligned with the PR goal (allow multiple implementations) while avoiding redundant triples and unnecessary fact rebuilds when the same exact link already exists.
Apply:
- sdna_links.push(Link { - source: "ad4m://self".to_string(), - predicate: Some(predicate.to_string()), - target: literal_name.clone(), - }); - - sdna_links.push(Link { - source: literal_name.clone(), - predicate: Some("ad4m://sdna".to_string()), - target: sdna_code, - }); - - self.add_links(sdna_links, LinkStatus::Shared, None).await?; + // Keep "self -> has_* -> <ClassName>" unique to reduce noise + let class_link_exists = !self + .get_links(&LinkQuery { + source: Some("ad4m://self".to_string()), + predicate: Some(predicate.to_string()), + target: Some(literal_name.clone()), + from_date: None, + until_date: None, + limit: Some(1), + }) + .await? + .is_empty(); + if !class_link_exists { + sdna_links.push(Link { + source: "ad4m://self".to_string(), + predicate: Some(predicate.to_string()), + target: literal_name.clone(), + }); + } + + // Allow multiple implementations but skip exact duplicate "<ClassName> -sdna-> <Code>" + let sdna_link_exists = !self + .get_links(&LinkQuery { + source: Some(literal_name.clone()), + predicate: Some("ad4m://sdna".to_string()), + target: Some(sdna_code.clone()), + from_date: None, + until_date: None, + limit: Some(1), + }) + .await? + .is_empty(); + if !sdna_link_exists { + sdna_links.push(Link { + source: literal_name.clone(), + predicate: Some("ad4m://sdna".to_string()), + target: sdna_code, + }); + } + + if !sdna_links.is_empty() { + self.add_links(sdna_links, LinkStatus::Shared, None).await?; + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
rust-executor/src/perspectives/perspective_instance.rs(3 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
rust-executor/src/perspectives/perspective_instance.rs (1)
rust-executor/src/agent/mod.rs (2)
create_signed_expression(59-74)create_signed_expression(219-226)
🔇 Additional comments (3)
rust-executor/src/perspectives/perspective_instance.rs (3)
1159-1161: Good: SDNA mutations are serializedHolding
sdna_change_mutexduring SDNA writes avoids interleaving and duplicate bursts in concurrent callers. Keep it.
1212-1213: No callers depend onadd_sdna’s return value; safe to always returntrue
The only external invocation inmutation_resolvers.rsdiscards the returnedbool(await?;), so changing it to unconditionallyOk(true)has no impact.
6-6: No activeagent::…calls remain in perspective_instance.rs
Only commented-out references toagent::did()were found; the narrowed import is safe.
jhweir
approved these changes
Sep 2, 2025
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
2 participants
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.
We had this check from early to counter-act having apps accidentally write the same subject class many times. But in a situation where a new version of the app adds a new implementation under the same name, this would just silently not write the new code.
Only commented-out in case this causes many duplicated classes again. But should be good to go.
Summary by CodeRabbit