Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThe pull request refactors error handling in the Axone governance contract by replacing a generic Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant QueryHandler as Query Handler
participant CaseGuard as Case Guard
participant Storage as Constitution Storage
participant PrologEngine as Prolog Engine
Client->>QueryHandler: Decide { case, motivated }
QueryHandler->>CaseGuard: case(case)
CaseGuard-->>QueryHandler: ✓ Valid Dict (Ground)
QueryHandler->>Storage: Load Constitution
Storage-->>QueryHandler: Constitution Bytes
QueryHandler->>QueryHandler: Build Prolog Query<br/>(decide/2 or decide/3)
QueryHandler->>PrologEngine: QueryServiceAskRequest
PrologEngine-->>QueryHandler: QueryServiceAskResponse
QueryHandler->>QueryHandler: Extract Substitutions<br/>(Verdict, Motivation)
QueryHandler-->>Client: DecideResponse<br/>{ verdict, motivation }
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
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 |
Codecov Report❌ Patch coverage is
🚀 New features to boost your workflow:
|
size-limit report 📦
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @contracts/axone-gov/src/handlers/query.rs:
- Around line 34-43: The validation is run on the untrimmed input which can
mismatch the later trimmed value used in the query; in function query_decide
ensure you trim the incoming case before calling guards::case by assigning let
case = case.trim(); (or let case = case.trim().to_string() if needed) and then
call guards::case(&case)? so both validation and the formatted query use the
same trimmed string; update any subsequent uses (the query format! and
guards::case invocation) to use this trimmed variable name.
🧹 Nitpick comments (4)
contracts/axone-gov/tests/integration.rs (2)
135-176: Helper responses are good; consider strengthening the mock expectation to validaterequest.query(andlimit) too.
Right now,LogicAskScenario::install()only assertsrequest.program, so a malformed/incorrect query string could still “pass” these tests.
217-470: Excellent decide-path integration coverage (success + broad failure modes); main risk is brittle string matching in a few tests.
For the decide failures, consider using the samemsg + chainapproach you used earlier (instead offormat!("{err:?}")) to reduce sensitivity to debug formatting changes.
Also, per coding guidelines: if this PR changedmsg.rs/ query message types fordecide, ensurecargo make schemawas run and updated schemas were committed.contracts/axone-gov/src/msg.rs (1)
10-11: Clarify "resource AA" reference in documentation.The phrase "on the resource AA" appears to be a placeholder or incomplete reference. Consider specifying what "AA" refers to or removing it if it is extraneous.
contracts/axone-gov/src/guards/constitution.rs (1)
17-20: Consider simplifying the UTF-8 conversion.The
.map(ToString::to_string)call is functional but could be expressed more directly.♻️ Suggested simplification
let program = std::str::from_utf8(constitution.as_slice()) - .map(ToString::to_string) + .map(str::to_owned) .map_err(|err| AxoneGovError::ConstitutionUtf8(err.to_string()))?;
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
docs/axone-gov.mdis excluded by!docs/*.md
📒 Files selected for processing (10)
contracts/axone-gov/src/error.rscontracts/axone-gov/src/gateway/logic.rscontracts/axone-gov/src/guards/case.rscontracts/axone-gov/src/guards/constitution.rscontracts/axone-gov/src/guards/mod.rscontracts/axone-gov/src/handlers/query.rscontracts/axone-gov/src/msg.rscontracts/axone-gov/src/prolog/ast.rscontracts/axone-gov/src/state.rscontracts/axone-gov/tests/integration.rs
🧰 Additional context used
📓 Path-based instructions (5)
**/*.rs
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.rs: Runcargo make schemato generate JSON schemas for each contract after any change tomsg.rsor message types, and commit the updated schemas
Use standard CosmWasm contract entry points with#[cfg_attr(not(feature = "library"), entry_point)]attribute for instantiate, execute, and query functions
Validate all user inputs and apply strict access control usingMessageInfo.sender,Addr, andCanonicalAddrproperly to prevent reentrancy, integer overflows, and unauthorized state changes
Place unit tests insrc/alongside source code usingcosmwasm-std::testingmocks to test contract entry points
Implement amigrateentry point following CosmWasm patterns with migration logic in the contract module when storage or logic changes in a breaking way
Use///for public API documentation and//sparingly for non-obvious reasoning, explaining system invariants, trade-offs, and design intent
Use English for all comments, focusing on 'why' not 'what' when documenting architecture, interfaces, and design decisions
RDF data requires proper namespace/prefix management when working with Cognitarium contract that stores subject-predicate-object triples with SPARQL queries
Optimize for low gas usage by avoiding redundant state reads/writes in smart contract code
Files:
contracts/axone-gov/src/prolog/ast.rscontracts/axone-gov/src/error.rscontracts/axone-gov/src/state.rscontracts/axone-gov/src/gateway/logic.rscontracts/axone-gov/src/guards/constitution.rscontracts/axone-gov/src/guards/mod.rscontracts/axone-gov/tests/integration.rscontracts/axone-gov/src/guards/case.rscontracts/axone-gov/src/msg.rscontracts/axone-gov/src/handlers/query.rs
⚙️ CodeRabbit configuration file
**/*.rs: Review the Rust code, point out issues relative to principles of clean code, expressiveness, and performance.
Suggest idiomatic solutions and best practices.
Files:
contracts/axone-gov/src/prolog/ast.rscontracts/axone-gov/src/error.rscontracts/axone-gov/src/state.rscontracts/axone-gov/src/gateway/logic.rscontracts/axone-gov/src/guards/constitution.rscontracts/axone-gov/src/guards/mod.rscontracts/axone-gov/tests/integration.rscontracts/axone-gov/src/guards/case.rscontracts/axone-gov/src/msg.rscontracts/axone-gov/src/handlers/query.rs
**/error.rs
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Define custom error types in
error.rsusingthiserrorcrate andContractErrorenum for all contract-specific errors
Files:
contracts/axone-gov/src/error.rs
**/state.rs
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Define state in
state.rsusingcw-storage-plustyped storage items with the patternpub const STORE: Item<Store> = Item::new("store");
Files:
contracts/axone-gov/src/state.rs
**/tests/**/*.rs
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Place integration tests in
tests/directory using real contract instantiation with test dependencies
Files:
contracts/axone-gov/tests/integration.rs
**/msg.rs
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Define message types in
msg.rsfiles, includingInstantiateMsg,ExecuteMsg, andQueryMsgenums
Files:
contracts/axone-gov/src/msg.rs
🧠 Learnings (5)
📚 Learning: 2025-12-22T18:47:03.919Z
Learnt from: CR
Repo: axone-protocol/contracts PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-22T18:47:03.919Z
Learning: Applies to **/error.rs : Define custom error types in `error.rs` using `thiserror` crate and `ContractError` enum for all contract-specific errors
Applied to files:
contracts/axone-gov/src/error.rscontracts/axone-gov/src/guards/constitution.rs
📚 Learning: 2025-12-22T18:47:03.919Z
Learnt from: CR
Repo: axone-protocol/contracts PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-22T18:47:03.919Z
Learning: Applies to **/state.rs : Define state in `state.rs` using `cw-storage-plus` typed storage items with the pattern `pub const STORE: Item<Store> = Item::new("store");`
Applied to files:
contracts/axone-gov/src/state.rs
📚 Learning: 2025-12-22T18:47:03.919Z
Learnt from: CR
Repo: axone-protocol/contracts PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-22T18:47:03.919Z
Learning: Applies to **/tests/**/*.rs : Place integration tests in `tests/` directory using real contract instantiation with test dependencies
Applied to files:
contracts/axone-gov/tests/integration.rs
📚 Learning: 2025-12-22T18:47:03.919Z
Learnt from: CR
Repo: axone-protocol/contracts PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-22T18:47:03.919Z
Learning: Applies to **/*.rs : Place unit tests in `src/` alongside source code using `cosmwasm-std::testing` mocks to test contract entry points
Applied to files:
contracts/axone-gov/tests/integration.rs
📚 Learning: 2025-12-22T18:47:03.919Z
Learnt from: CR
Repo: axone-protocol/contracts PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-22T18:47:03.919Z
Learning: Applies to **/msg.rs : Define message types in `msg.rs` files, including `InstantiateMsg`, `ExecuteMsg`, and `QueryMsg` enums
Applied to files:
contracts/axone-gov/src/msg.rs
🧬 Code graph analysis (3)
contracts/axone-gov/src/state.rs (1)
contracts/axone-gov/src/guards/constitution.rs (1)
constitution(17-50)
contracts/axone-gov/src/guards/mod.rs (2)
contracts/axone-gov/src/guards/case.rs (1)
case(6-21)contracts/axone-gov/src/guards/constitution.rs (1)
constitution(17-50)
contracts/axone-gov/src/handlers/query.rs (3)
contracts/axone-gov/src/gateway/logic.rs (3)
query_service_ask(73-79)query_service_ask(122-133)new(16-22)contracts/axone-gov/src/state.rs (1)
load_constitution_as_string(8-13)contracts/axone-gov/src/guards/case.rs (1)
case(6-21)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Analyze (rust)
🔇 Additional comments (18)
contracts/axone-gov/src/prolog/ast.rs (2)
21-35:Term::is_ground()is correct and nicely idiomatic.
The recursive checks forCompound,List(including tail), andDictvalues align with the intended “no variables anywhere” semantics.
37-170: Solid unit coverage foris_ground, including nested and tail-list cases.
The table-driven approach with per-case descriptions will be helpful for future regressions.contracts/axone-gov/src/gateway/logic.rs (1)
126-133: Improved missing-handler signaling; ensure downstream assertions match the newnot_foundwording.
This will change the rendered error string (typicallynot found: ...) compared to the previous generic error.contracts/axone-gov/src/guards/mod.rs (1)
1-5: Guard module wiring looks correct;guards::caseis cleanly exposed.contracts/axone-gov/tests/integration.rs (2)
4-7: Import updates are fine; theResult as LogicResultalias avoids name collisions cleanly.
207-215: Improved assertion strategy by checking both top-level message and error chain.
This makes the test resilient to wrapping (e.g., anyhow) while still asserting the user-facing wording.contracts/axone-gov/src/state.rs (1)
1-13: The code is correct and will compile without issue. TheAxoneGovErrorenum implementsFrom<StdError>via the#[from]attribute on theStdvariant (line 12 oferror.rs), which thethiserrorcrate automatically generates. The?operator on line 9 properly propagates and converts theStdErrorfromCONSTITUTION.load(storage)intoAxoneGovError. Additionally, the function correctly handles the UTF-8 conversion error and maps it to the domain-specificAxoneGovError::ConstitutionUtf8variant. The implementation aligns with established state patterns and error handling conventions.Likely an incorrect or invalid review comment.
contracts/axone-gov/src/msg.rs (3)
20-25: Well-structured instantiate message definition.The documentation clearly explains the constitution requirements including UTF-8 encoding and required predicates. The message structure follows CosmWasm conventions.
49-64: Clear and comprehensive Decide query documentation.The documentation provides an excellent example of the case dict format and clearly explains the behavior differences between
decide/2anddecide/3predicates. Themotivatedflag semantics are well-defined.
73-80: DecideResponse structure is well-designed.The response type correctly models the verdict as mandatory and motivation as optional, aligning with the
motivatedflag semantics in the query.contracts/axone-gov/src/guards/constitution.rs (2)
26-36: Robust handling of missing answers and predicates.The error handling correctly distinguishes between a missing answer envelope (engine failure) and an empty result set (missing predicates). The formatted error message provides actionable context.
38-47: Appropriate error extraction from Prolog results.Using
filter_mapwithnext()efficiently retrieves the first error without unnecessary allocations. The error context is preserved in theConstitutionInvalidvariant.contracts/axone-gov/src/error.rs (1)
29-88: Excellent error variant granularity and documentation.The refactoring from a generic
InvalidConstitutionto specific error variants significantly improves error handling clarity. Each variant is well-documented, explaining the precise failure context. This aligns with coding guidelines for defining custom error types inerror.rsusing thethiserrorcrate.contracts/axone-gov/src/guards/case.rs (2)
6-21: Well-structured case validation guard.The validation logic correctly enforces three requirements: valid syntax, dict type, and groundness. The error messages are descriptive and actionable. The pattern of parsing first, then validating structural constraints is clean.
23-30: Clean helper functions for parsing and error conversion.The separation of
parse_termandparse_errorkeeps the main validation function focused. Including the offset in syntax error messages aids debugging.contracts/axone-gov/src/handlers/query.rs (3)
45-59: Robust Prolog query execution and error handling.The query construction correctly differentiates between
decide/2anddecide/3based on themotivatedflag. Error extraction usingfilter_mapwithnext()is efficient. TheSome(1)limit ensures only the first solution is retrieved, which is appropriate for governance decisions.
61-80: Clear result extraction with appropriate error variants.The extraction logic correctly handles the mandatory verdict and conditionally required motivation. The use of specific error variants (
DecisionNoResult,DecisionMissingVerdict,DecisionMissingMotivation) provides clear failure diagnostics.
82-88: Clean substitution lookup helper.The
find_substitutionfunction is concise and correctly extracts the expression for a given variable name. Consider whether theexpression.clone()could be avoided by returning a reference, though given the usage context, the current approach is reasonable.
2 participants
Adds the
decidequery toaxone-gov- important one, obviously.You can now ask the stored Prolog constitution for a governance verdict (and motivation when requested). Includes input validation, clearer errors, and tests covering the main paths. Docs are intentionally minimal for now. I'll follow up with examples and integration notes once the API is fully stable...
Summary by CodeRabbit
Release Notes
New Features
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.