Repository navigation
feat(dpp)!: count how many of a group of properties a document sets (countPresent, PV14) - #5332
Conversation
…countPresent, PV14)
A propertyConstraints rule could say a property is present or absent, but
bounding how many of a group are set ("exactly one of email, phone,
handle", "at most two of these five") meant an anyOf plus a not of every
pair, which outgrows the 32-node rule limit at five properties.
countPresent is a new integer expression: how many of two or more
distinct property paths the document holds, each as `present` tests it.
Compared with a number or listed in an `in`, it gives every bound:
{ "equal": [{ "countPresent": ["email", "phone", "handle"] }, 1] }
{ "in": [{ "countPresent": ["email", "phone", "handle"] }, [1, 2]] }
It costs one node plus one per path, never faults, and its paths are
checked like a presence test's (any declared property, none transient, no
system value). A repeated path is refused by the meta-schema and on every
parse. Added in place to meta-schema v3 and parser generation 3, which
only protocol version 14 selects; earlier meta-schemas never admitted the
key, so no stored contract can hold it.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
📖 Book Preview built successfully. Download the preview from the workflow artifacts. Updated at 2026-10-08T13:24:47.604Z |
|
Waiting for bot review — coderabbitai not yet · thepastaclaw not yet. Wait for the missing reviews, or a writer can post |
github-actions
Bot
added
the
waiting-bots
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe change adds ChangescountPresent Property Constraint
Priority: ➖ Normal Estimated code review effort: 3 (Moderate) | ~20 minutes Change: Feature Suggested reviewers: Merge Risk: ⚪ Minimal · up to This change adds a countPresent expression for property constraints, selected only by protocol version 14. No merge-blocking risk was identified in the supplied material. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
|
✅ Final review complete — no blockers (commit bcf982e) · triage: normal |
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Waiting for bot review — coderabbitai ✓ · thepastaclaw not yet. Wait for the missing reviews, or a writer can post |
thepastaclaw
left a comment
thepastaclaw
left a comment
There was a problem hiding this comment.
Final validation — Phase 1 + Phase 2
Verified the supplied findings against head dedd2fd. No blocking defects were confirmed; the remaining feedback is a targeted test-coverage suggestion and a stale Rust API comment. This was a static review: the supplied CI snapshot reports successful Rust workspace, WASM, Swift, and Kotlin checks, with one browser-test shard and PR Hygiene still pending.
🟡 1 suggestion(s) | 💬 1 nitpick(s)
1 finding(s) not shown inline (the lines are not part of this PR's diff)
💬 Nitpick: PropertyRead::Presence documentation omits countPresent
packages/rs-dpp/src/data_contract/document_type/property_constraints/mod.rs:893-894
CountPresent now reports every listed path as PropertyRead::Presence in collect_property_reads, but this variant's documentation still names only present and absent. Include countPresent so the Rust API documentation accurately describes which expressions produce presence reads, matching the updated WASM, Swift, and Kotlin comments.
/// Only whether the document holds it, in a `present`, `absent` or
/// `countPresent`: a property of any type, an object included.
source: muse-spark-1.3-contributor (phase1-reviewer: general, rust-quality)
Review provenance
Source: reviewer 1: muse-spark-1.3-contributor (agent: phase1-reviewer, role: general); reviewer 2: muse-spark-1.3-contributor (agent: phase1-reviewer, role: rust-quality); reviewer 3: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 4: gpt-6.1-sol (agent: phase2-reviewer, role: architecture-layering); reviewer 5: gpt-6.1-sol (agent: phase2-reviewer, role: ffi-engineer); reviewer 6: gpt-6.1-sol (agent: phase2-reviewer, role: platform-versioning); reviewer 7: gpt-6.1-sol (agent: phase2-reviewer, role: rust-quality); reviewer 8: gpt-6.1-sol (agent: phase2-reviewer, role: security-auditor); final verifier: gpt-6.1-sol (agent: sol-verifier, role: final-verifier)
- Triage:
normalbygpt-6.1-sol(effort low) — The consensus-relevant change in property_constraints/mod.rs adds a straightforward, bounded expression with parsing, evaluation and node accounting, but is not large or intricate enough to qualify as critical. - Phase 1 reviewers:
muse-spark-1.3-contributor— general (completed, effort xhigh); agentphase1-reviewer,muse-spark-1.3-contributor— rust-quality (completed, effort xhigh); agentphase1-reviewer - Phase 1 model:
muse-spark-1.3-contributor— not quota-gated; passed overgemini-3.8-flash-high(antigravity below 15% reserve: weekly 15% left, 5h 100% left),glm-5.3-flash(not used above high effort; tier asks max) - Fresh verifier:
gpt-6.1-sol— final-verifier; agentsol-verifier - Phase 2 reviewers:
gpt-6.1-sol— general (completed, effort high); agentphase2-reviewer,gpt-6.1-sol— architecture-layering (completed, effort high); agentphase2-reviewer,gpt-6.1-sol— ffi-engineer (completed, effort high); agentphase2-reviewer,gpt-6.1-sol— platform-versioning (completed, effort high); agentphase2-reviewer,gpt-6.1-sol— rust-quality (completed, effort high); agentphase2-reviewer,gpt-6.1-sol— security-auditor (completed, effort high); agentphase2-reviewer
🤖 Prompt for all review comments with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.
In `packages/rs-dpp/src/data_contract/document_type/class_methods/try_from_schema/v3/property_constraints_tests.rs`:
- [SUGGESTION] packages/rs-dpp/src/data_contract/document_type/class_methods/try_from_schema/v3/property_constraints_tests.rs:974-979: Add countPresent coverage for stored-document conditions
The new tests cover ordinary propertyConstraints parsing and create/replace evaluation, but none exercise countPresent in deleteConstraints, immutable when, or retractedWhen, including the advertised $old paths. These contexts share the expression parser, but differ in stored-read admission and which document supplies the values. Add a focused immutable or retractedWhen test counting $old paths with different stored and replacement values, and assert that the same paths are rejected in propertyConstraints. This would lock down the new expression's interaction with the existing stored-document boundary; no current implementation failure was found.
In `packages/rs-dpp/src/data_contract/document_type/property_constraints/mod.rs`:
- [NITPICK] packages/rs-dpp/src/data_contract/document_type/property_constraints/mod.rs:893-894: PropertyRead::Presence documentation omits countPresent
CountPresent now reports every listed path as PropertyRead::Presence in collect_property_reads, but this variant's documentation still names only present and absent. Include countPresent so the Rust API documentation accurately describes which expressions produce presence reads, matching the updated WASM, Swift, and Kotlin comments.
|
Bots are done — your move: post |
github-actions
Bot
added
waiting-self-review
A condition judging a replace counts the stored document's properties through `$old.` and the written one's without it; the same paths in a propertyConstraints rule are refused. Also names countPresent among the expressions producing a presence read. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Waiting for bot review — coderabbitai ✓ · thepastaclaw not yet. Wait for the missing reviews, or a writer can post |
github-actions
Bot
added
waiting-bots
thepastaclaw
left a comment
thepastaclaw
left a comment
There was a problem hiding this comment.
Re-review — Final validation — Phase 1 + Phase 2
Static verification of the complete PR diff at bcf982e found no actionable in-scope defects: countPresent reuses existing presence semantics, validates every counted path, includes paths in node accounting, and remains isolated behind the PV14 grammar and validation gates. Both prior findings are fixed. No builds or tests were run locally; the supplied CI snapshot still showed Rust workspace tests, JS builds, Kotlin checks, and Docker builds in progress.
🔴 0 blocking | 🟡 0 suggestion(s) | 💬 0 nitpick(s)
Review provenance
Source: reviewer 1: muse-spark-1.3-contributor (agent: phase1-reviewer, role: general); reviewer 2: muse-spark-1.3-contributor (agent: phase1-reviewer, role: rust-quality); reviewer 3: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 4: gpt-6.1-sol (agent: phase2-reviewer, role: architecture-layering); reviewer 5: gpt-6.1-sol (agent: phase2-reviewer, role: ffi-engineer); reviewer 6: gpt-6.1-sol (agent: phase2-reviewer, role: platform-versioning); reviewer 7: gpt-6.1-sol (agent: phase2-reviewer, role: rust-quality); reviewer 8: gpt-6.1-sol (agent: phase2-reviewer, role: security-auditor); reviewer 9: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 10: gpt-6.1-sol (agent: phase2-reviewer, role: architecture-layering); reviewer 11: gpt-6.1-sol (agent: phase2-reviewer, role: ffi-engineer); reviewer 12: gpt-6.1-sol (agent: phase2-reviewer, role: platform-versioning); reviewer 13: gpt-6.1-sol (agent: phase2-reviewer, role: rust-quality); reviewer 14: gpt-6.1-sol (agent: phase2-reviewer, role: security-auditor); final verifier: gpt-6.1-sol (agent: sol-verifier, role: final-verifier)
- Triage:
normalbygpt-6.1-sol(effort low) — The diff changes consensus-relevant parsing and evaluation in property_constraints/mod.rs, but adds a contained, straightforward counting expression rather than large or intricate critical-surface logic. - Phase 1 reviewers:
muse-spark-1.3-contributor— general (completed, effort xhigh); agentphase1-reviewer,muse-spark-1.3-contributor— rust-quality (completed, effort xhigh); agentphase1-reviewer - Phase 1 model:
muse-spark-1.3-contributor— not quota-gated; passed overgemini-3.8-flash-high(antigravity below 15% reserve: weekly 15% left, 5h 100% left),glm-5.3-flash(not used above high effort; tier asks max) - Fresh final gate: an independent Phase-2 review ran after iterative findings were reconciled
- Fresh verifier:
gpt-6.1-sol— final-verifier; agentsol-verifier - Phase 2 reviewers:
gpt-6.1-sol— general (completed, effort high); agentphase2-reviewer,gpt-6.1-sol— architecture-layering (completed, effort high); agentphase2-reviewer,gpt-6.1-sol— ffi-engineer (completed, effort high); agentphase2-reviewer,gpt-6.1-sol— platform-versioning (completed, effort high); agentphase2-reviewer,gpt-6.1-sol— rust-quality (completed, effort high); agentphase2-reviewer,gpt-6.1-sol— security-auditor (completed, effort high); agentphase2-reviewer,gpt-6.1-sol— general (completed, effort high); agentphase2-reviewer,gpt-6.1-sol— architecture-layering (completed, effort high); agentphase2-reviewer,gpt-6.1-sol— ffi-engineer (completed, effort high); agentphase2-reviewer,gpt-6.1-sol— platform-versioning (completed, effort high); agentphase2-reviewer,gpt-6.1-sol— rust-quality (completed, effort high); agentphase2-reviewer,gpt-6.1-sol— security-auditor (completed, effort high); agentphase2-reviewer
🤖 Prompt for all review comments with AI agents
These findings are from an automated code review. Verify the current code and confirm that no unresolved issues remain.
No unresolved findings remain from the prior review on this head.
|
Bots are done — your move: post |
github-actions
Bot
added
waiting-self-review
Basic explanation
What this does: A data contract can already set rules on a document type, such as "this property must be present". This adds
countPresent, a way to count how many properties from a chosen group a document actually fills in. A rule can then say "exactly one of email, phone or handle", "at most two of these five", "at least one" or "one or two".Value: Contract authors get a short, readable way to express "pick one of these" and "at most / at least n of these". Before, this needed one clause for every pair of properties. That was 17 rule nodes for three properties, and the 32-node limit was passed at five. Now it costs one node plus one per property: 6 nodes for "exactly one of three".
Risks: Low. It is a new keyword inside the protocol version 14 grammar, which is not live on mainnet. Earlier meta-schemas never accepted the key, so no stored contract can contain it, and existing rules are evaluated exactly as before. It reads only the document being written, never faults (no overflow or division), and is held to the existing node limit. It is consensus-relevant: nodes must agree on accepting contracts that use it and on judging documents against it, hence the
!.Issue being fixed or feature implemented
propertyConstraintscould test one property at a time withpresentandabsent, but could not bound how many of a group of properties a document sets. Writing "exactly one of N" by hand means ananyOfplus anotof every pair, which grows with the square of N and does not fit the rule limits beyond four properties. "At least k of N" for k of 2 or more needs every combination and is impractical.What was done?
countPresent(ConstraintExpression::CountPresent): how many of two or more distinct property paths the document holds, each counted exactly aspresenttests it (null and an object with no member present count as absent; 0 andfalsecount as present). It works on properties of any type, nested ones and$old.paths included, and is never a fault.equal,lessThanOrEqual,greaterThanOrEqualandin. "Between 1 and 2" is{ "in": [{ "countPresent": [...] }, [1, 2]] }, so nobetweenoperator was added.inrefuses a repeated value) and by the meta-schema'suniqueItemsat registration.presenceread, so parser generation 3 checks it like apresenttest: it must be a declared property, not transient and not a system value.max_property_constraint_nodes.countPresentkey is added to the expression object, and the expression and path descriptions mention it.book/src/contract-keywords/property-constraints.mdwith a table of bounds, a row in the keyword table, a bullet on the deep-dive page and a mention in the js-evo-sdk README. Protocol version 14 note 39 is extended in place.{ countPresent: string[] }. The Swift and Kotlin changes are doc comments only: each counted path already reaches them as apresenceread.Example
A seller must give exactly one way to be reached:
Before, registering this contract failed at the meta-schema (
countPresentwas not an allowed key), and the parser named it as unknown:The only way to say it was the pairwise form, 17 nodes for three properties:
Now the short form registers (6 nodes), and documents are judged like this:
DocumentPropertyConstraintViolatedError(10422), ruleoneContactemailemailandphonehandleto an offer withemailemailand settinghandleOther bounds:
{ "lessThanOrEqual": [{ "countPresent": [...] }, 1] }{ "greaterThanOrEqual": [{ "countPresent": [...] }, 2] }{ "in": [{ "countPresent": [...] }, [1, 2]] }{ "in": [{ "countPresent": ["a", "b", "c"] }, [0, 3]] }In-place changes to shipped generations
Meta-schema v3 and document type parser generation 3 are edited in place. Only protocol version 14 selects them, and it is not live on mainnet; the v3 meta-schema states it stays editable until then. The edit cannot change the outcome for any existing contract or document: meta-schemas v0 to v3 never admitted a
countPresentkey, so registration refused it and no stored contract can carry it. Every existing rule parses, counts nodes and evaluates exactly as before.How Has This Been Tested?
cargo test -p dpp --all-features --lib -- property_constraints: 104 passed, including the newshould_parse_count_present,should_refuse_a_malformed_count_present,should_count_the_properties_a_document_holds,should_bound_how_many_of_a_group_a_document_setsand the generation 3 testshould_count_the_presence_of_properties_the_type_declares(declared, nested and object paths accepted; unknown,$ownerId,$createdAtand transient paths refused on both parse paths; a repeated path refused by the meta-schema and the parser).cargo test -p drive-abci --lib -- property_constraints: 40 passed, including the end-to-endshould_hold_a_document_to_exactly_one_of_a_group_of_properties(the create and replace cases in the table above, throughprocess_raw_state_transitions).cargo clippy -p dpp --all-features --tests: clean.Breaking Changes
Consensus: from protocol version 14, a contract may declare
countPresentinpropertyConstraints,deleteConstraints, animmutableentry'swhenandretractedWhen, and documents are judged against it. Nothing changes before protocol version 14, and no existing contract or rule behaves differently.Checklist:
structure.rs, regeneratedgrovedb-structure.json, and checked the structure viewer link posted on this pull requestFor repository code-owners and collaborators only
🤖 Generated with Claude Code
PR Hygiene ·
bcf982e/self-reviewedjs-wasm-sdk(packages/js-evo-sdk/README.md) — shumkovkotlin-sdk(packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/queries/DocumentPropertyConstraints.kt) — HashEngineeringdpp— you own itrs-drive-abci— you own itswift-sdk(packages/swift-sdk/Sources/SwiftDashSDK/Core/Utils/DocumentPropertyConstraints.swift) — llbartekll or romchornyiWhen every merge requirement is met, the
PR Hygienecheck passes. Reviewer limits do not block merging; other required GitHub checks and protections still apply.Summary by CodeRabbit
New Features
countPresentproperty constraints, which count how many of two or more distinct properties are present. Rules can compare the count against exact values, minimums, maximums, or ranges.Documentation
countPresent, supported property paths, and its contribution to constraint limits.