-
Notifications
You must be signed in to change notification settings - Fork 31
Feat: querying other agents source chains #606
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughDocumentation-only updates to querying-source-chains.md: clarified query filters span data sources; restructured "query another agent's source chain" into coordinator/validation subsections; documented that get_agent_activity (ActivityRequest::Full) returns chain status, warrants, and action hashes only (no actions/entries); added example, DHT-retrieval guidance, warnings, and TODOs. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✨ Finishing touches🧪 Generate unit tests
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. 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. Comment |
c55eb80 to
c58933d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (3)
src/pages/build/querying-source-chains.md (3)
85-96: Call out that this is not the primary use of get_agent_activity.Per issue #571, add a brief note stating that the primary use is chain status/warrants and that a separate doc will cover that.
Apply this diff to insert a short note after the introductory sentence:
In your coordinator zome functions, you can use the [`get_agent_activity`](https://docs.rs/hdk/latest/hdk/chain/fn.get_agent_activity.html) host function, which works a lot like `query` if you pass [`ActivityRequest::Full`](https://docs.rs/hdk/latest/hdk/prelude/enum.ActivityRequest.html#variant.Full) to it. The main differences are: +::: note +`get_agent_activity` is primarily intended for inspecting chain status (including invalid/forked regions) and collecting warrants. Querying another agent’s authored content is a secondary pattern; a dedicated doc will cover the primary use case. +:::
96-96: Fix typo: “Ths” → “This”.-Ths example gets the action hashes of all the movie entries authored by an arbitrary agent. +This example gets the action hashes of all the movie entries authored by an arbitrary agent.
98-101: Unify admonition syntax (“!!! info”) with site style (“::: info”).Earlier you used “::: intro”. Recommend consistent syntax to avoid rendering issues.
-!!! info All DHT requests can fail +::: info All DHT requests can fail Because a DHT request often goes out to a remote node, it can fail to connect to that node, or they may not have the data you're looking for. It's up to you to build the retry logic that works best for your app. If you have program flow that requires multiple queries, it's often a good practice to define zome functions as single-query functions that return DHT hashes so that the client can retrieve the rest of the data in follow-up zome calls. This way, work isn't wasted if one query fails. -!!! +:::If the docs site standardizes on a different admonition syntax, adjust accordingly.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/pages/build/querying-source-chains.md(2 hunks)
🔇 Additional comments (2)
src/pages/build/querying-source-chains.md (2)
18-18: Good clarification on filter scope.Clear and useful lead‑in to the filter section.
125-127: Nice addition pointing to must_get_agent_activity for validation.
c58933d to
da19c2a
Compare
da19c2a to
1551636
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/pages/build/querying-source-chains.md (2)
63-67: Fix invalid Rust import keyword.Replace
includewithuse.-include hdk::prelude::*; +use hdk::prelude::*;
71-83: Extern must return ExternResult and bring UnitEntryTypes into scope.
queryreturnsExternResult<Vec<Record>>; function signature and imports need updating.use hdk::prelude::*; +use movies::prelude::*; #[hdk_extern] -pub fn get_all_movies_i_authored() -> Vec<Record> { - query(ChainQueryFilter::new() +pub fn get_all_movies_i_authored() -> ExternResult<Vec<Record>> { + query(ChainQueryFilter::new() .entry_type(EntryType::App(UnitEntryTypes::Movie.into())) .include_entries(true) ) }
♻️ Duplicate comments (1)
src/pages/build/querying-source-chains.md (1)
104-121: Rust snippet won’t compile: “using” keyword and missing import.Replace
usingwithuseand import your entry enum; also minor readability tweak on tuple destructuring.-```rust -using hdk::prelude::*; +```rust +use hdk::prelude::*; +use movies::prelude::*; @@ - // The action hash is the second element in each tuple. - Ok(activity.valid_activity.into_iter().map(|a| a.1).collect()) + // The action hash is the second element in each tuple. + Ok(activity.valid_activity.into_iter().map(|(_, h)| h).collect()) }</blockquote></details> </blockquote></details> <details> <summary>🧹 Nitpick comments (3)</summary><blockquote> <details> <summary>src/pages/build/querying-source-chains.md (3)</summary><blockquote> `89-96`: **Clarify why include_entries is ignored for get_agent_activity.** Add the explicit reason to preempt confusion. ```diff * It returns action hashes but no action or entry data. <!-- TODO: hopefully this behavior will be fixed one day --> + The `include_entries` filter is ignored because agent-activity authorities do not respect `EntryType::cache_at_agent_activity` and do not store entry data alongside actions.
96-96: Typo: “Ths” → “This”.-Ths example gets the action hashes of all the movie entries authored by an arbitrary agent. +This example gets the action hashes of all the movie entries authored by an arbitrary agent.
98-101: Admonition syntax likely inconsistent with site.If the site uses
:::(as used for the intro), switch from!!!to:::. Please confirm rendering.-!!! info All DHT requests can fail +::: info All DHT requests can fail Because a DHT request often goes out to a remote node, it can fail to connect to that node, or they may not have the data you're looking for. It's up to you to build the retry logic that works best for your app. If you have program flow that requires multiple queries, it's often a good practice to define zome functions as single-query functions that return DHT hashes so that the client can retrieve the rest of the data in follow-up zome calls. This way, work isn't wasted if one query fails. -!!! +:::
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/pages/build/querying-source-chains.md(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-08T21:51:44.027Z
Learnt from: pdaoust
PR: holochain/docs-pages#597
File: src/pages/build/getting-an-agents-status.md:141-141
Timestamp: 2025-09-08T21:51:44.027Z
Learning: In the holochain_zome_types::query::AgentActivity struct, the correct field name is `valid_activity`, not `valid_actions`. This field contains the valid activities for an agent according to the official Rust documentation.
Applied to files:
src/pages/build/querying-source-chains.md
🔇 Additional comments (1)
src/pages/build/querying-source-chains.md (1)
118-120: Good: correct field namevalid_activity.Matches the current
AgentActivityAPI. Nice catch.
jost-s
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
I left suggestions and request the addition of a description of the first value of the tuple.
|
✔️ 72bf9e6...2264c46 - Conventional commits check succeeded. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/pages/build/querying-source-chains.md (2)
63-67: Rust import typo:include→use.This snippet won’t compile as written.
Apply this diff:
-include hdk::prelude::*; +use hdk::prelude::*;
71-84: Return type mismatch: needExternResult<Vec<Record>>and?.
query(...)returnsExternResult<Vec<Record>>. The extern should returnExternResultand propagate errors.Apply this diff:
#[hdk_extern] -pub fn get_all_movies_i_authored() -> Vec<Record> { - query(ChainQueryFilter::new() - .entry_type(EntryType::App(UnitEntryTypes::Movie.into())) - .include_entries(true) - ) -} +pub fn get_all_movies_i_authored() -> ExternResult<Vec<Record>> { + query( + ChainQueryFilter::new() + .entry_type(EntryType::App(UnitEntryTypes::Movie.into())) + .include_entries(true), + ) +}
🧹 Nitpick comments (3)
src/pages/build/querying-source-chains.md (3)
20-32: Field name: usedescending, notorder_descending.The struct field is
descending: bool. Update the docs to match the actual API.Apply this diff:
-* `order_descending: bool`: Return the results in reverse chronological order, newest first and oldest last. +* `descending: bool`: Return the results in reverse chronological order, newest first and oldest last.
48-52: Confirm builder semantics for multipleentry_type(...)calls.If
entry_typeappends to the filter, this is fine; if it overwrites, only the last type will be used. Please confirm against the current API.If it overwrites, use a single call with both types:
-let filter_only_cap_grants_and_claims_newest_first = ChainQueryFilter::new() - .entry_type(EntryType::CapGrant) - .entry_type(EntryType::CapClaim) +let filter_only_cap_grants_and_claims_newest_first = ChainQueryFilter::new() + .entry_type(vec![EntryType::CapGrant, EntryType::CapClaim]) .include_entries(true) .descending();
105-124: Example looks good; minor enhancement on ordering.
valid_activityitems are(seq, hash). If consumers expect order, consider sorting byseqexplicitly before mapping, as ordering isn’t guaranteed by the example.Apply this diff inside the extern:
- Ok(activity.valid_activity.into_iter().map(|(_, h)| h).collect()) + let mut items = activity.valid_activity; + items.sort_by_key(|(seq, _)| *seq); + Ok(items.into_iter().map(|(_, h)| h).collect())
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/pages/build/querying-source-chains.md(3 hunks)
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-09-11T13:19:23.020Z
Learnt from: pdaoust
PR: holochain/docs-pages#597
File: src/pages/resources/glossary.md:933-939
Timestamp: 2025-09-11T13:19:23.020Z
Learning: In the holochain/docs-pages repository, pdaoust prefers glossary definitions to be "future-correct" rather than strictly reflecting current implementation status, to reduce maintenance burden when features are implemented.
Applied to files:
src/pages/build/querying-source-chains.md
📚 Learning: 2025-07-31T17:04:48.450Z
Learnt from: pdaoust
PR: holochain/docs-pages#594
File: src/pages/resources/howtos/running-network-infrastructure.md:5-7
Timestamp: 2025-07-31T17:04:48.450Z
Learning: In the holochain/docs-pages repository, pdaoust prefers "howto" (unhyphenated) over "how-to" (hyphenated) as it's the established pattern used consistently throughout the codebase for section titles, navigation, and documentation references.
Applied to files:
src/pages/build/querying-source-chains.md
📚 Learning: 2025-09-08T21:51:44.027Z
Learnt from: pdaoust
PR: holochain/docs-pages#597
File: src/pages/build/getting-an-agents-status.md:141-141
Timestamp: 2025-09-08T21:51:44.027Z
Learning: In the holochain_zome_types::query::AgentActivity struct, the correct field name is `valid_activity`, not `valid_actions`. This field contains the valid activities for an agent according to the official Rust documentation.
Applied to files:
src/pages/build/querying-source-chains.md
🔇 Additional comments (4)
src/pages/build/querying-source-chains.md (4)
18-18: Good clarification about filter scope.Reads well and sets context.
86-94: Section restructure: clear and accurate.The split and the note that
get_agent_activity(.., ActivityRequest::Full)returns hashes (not actions/entries) looks correct.
99-101: Solid reliability guidance.The “All DHT requests can fail” admonition is helpful and pragmatic.
128-131: Incorrect —must_get_agent_activityis not validation‑only.
It is deterministic and intended/safe for use invalidate/genesis_self_check, but it’s also a regular host function exported in the HDI/HDK prelude and callable from normal zome externs (so the doc’s claim that it can be used outside validation is valid; instead clarify that its deterministic, bounded semantics make it especially suitable for validation). (developer.holochain.org)Likely an incorrect or invalid review comment.
jost-s
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great stuff!
Closes #571 . Includes a simple, single-query example meant to be used in conjunction with another imaginary zome function that makes a single
getcall on each of the returned action hashes, and explains why this is a useful pattern.Summary by CodeRabbit