test(viewer): bundle_diff proptest surface (WBS-6.2 #434) - #447
test(viewer): bundle_diff proptest surface (WBS-6.2 #434)#447KooshaPari wants to merge 1 commit into
Conversation
Adds `crates/sl-viewer/tests/properties_viewer_bundle_diff.rs` with
10 proptest properties pinning the `bundle_diff::diff_fields` and
`OkfBundle::from_bundle` reductions:
* `diff_fields` returns the documented 9-field set in stable order
(guards against UI row-count drift when fields are added).
* `diff_fields(a, a)` is reflexive: no fields differ on equal inputs.
* `diff_fields(a, a.clone())` is idempotent: clone-mirror produces no
differences.
* `diff_fields(a, b)` is value-flipped symmetric:
`diff_fields(b, a)` swaps `value_a`/`value_b` per field but the
`differs` set is identical.
* `FieldDiff::differs` matches `value_a != value_b` per field
(catches drift where the boolean is computed independently of values).
* `Option<String>` fields (model, created_at, goal) render the em-dash
fallback (`—`) when both sides are `None`, and the resulting diff
is not a difference.
* `OkfBundle::from_bundle`:
* `message_count` equals the input slice count.
* `has_acceptance`/`has_contract` reflect presence of those kinds
(any-of) in the input continuation.
* `token_count` falls back to 0 when no Intent slice carries a
numeric `user_turn_count` (3-variant: missing slice / missing field
/ non-numeric field).
* `source_id` carries through from the continuation unchanged.
Updates WBS-6.2 evidence list, TRACEABILITY.json, and CHANGELOG.
🤖 CodeAnt AI — Review Status
|
Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
|
Warning Review limit reached
Next review available in: 45 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
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 |
codeant-ai
Bot
added
the
size:L
![codeant-ai[bot]](https://avatars.githubusercontent.com/in/646884?s=60&v=4){kind=small}
| fn from_bundle_token_count_zero_when_no_intent_or_field( | ||
| // Variants: 0 = no Intent bundle at all; 1 = Intent without | ||
| // user_turn_count; 2 = Intent with non-numeric user_turn_count. | ||
| variant in 0u8..3, | ||
| ) { | ||
| let bundles: Vec<Bundle> = match variant { | ||
| 0 => Vec::new(), | ||
| 1 => vec![Bundle::new(BundleKind::Intent, serde_json::json!({"goal": "x"}))], | ||
| _ => vec![Bundle::new( | ||
| BundleKind::Intent, | ||
| serde_json::json!({"user_turn_count": "not-a-number"}), | ||
| )], | ||
| }; |
There was a problem hiding this comment.
Suggestion: The property only generates missing or non-numeric user_turn_count values and never supplies a valid numeric value, so it cannot detect regressions where from_bundle reads the wrong Intent slice or fails to sum multiple Intent slices as documented. Add cases with one and multiple Intent bundles containing numeric counts and assert the expected total. [incomplete implementation]
Severity Level: Major ⚠️
- ❌ Multi-Intent comparisons can display an incorrect token count.
- ⚠️ Current property tests do not detect aggregation regressions.
- ⚠️ The documented sum contract remains unverified.
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** crates/sl-viewer/tests/properties_viewer_bundle_diff.rs
**Line:** 248:260
**Comment:**
*Incomplete Implementation: The property only generates missing or non-numeric `user_turn_count` values and never supplies a valid numeric value, so it cannot detect regressions where `from_bundle` reads the wrong Intent slice or fails to sum multiple Intent slices as documented. Add cases with one and multiple Intent bundles containing numeric counts and assert the expected total.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix| /// Compile-time guarantee that the FieldDiff-derived constants stay in sync. | ||
| /// If the impl adds a field, this test fails to compile until EXPECTED_FIELD_NAMES | ||
| /// is updated, prompting the reviewer to confirm the UI row count. | ||
| #[allow(dead_code)] | ||
| const fn _assert_field_count_fits_diff(diff: &[FieldDiff], expected_len: usize) -> bool { | ||
| diff.len() == expected_len | ||
| } |
There was a problem hiding this comment.
Suggestion: This is not a compile-time guarantee: the function is never invoked, and its parameters are arbitrary rather than tied to diff_fields or EXPECTED_FIELD_NAMES. Consequently, changing the production field list will not cause compilation to fail as the comment claims. Either remove the misleading comment/helper or invoke a real compile-time assertion with fixed values; the runtime property already provides the actual check. [comment mismatch]
Severity Level: Minor 🧹
- ⚠️ Comment inaccurately describes a dead test helper.
- ⚠️ Field-count protection remains runtime-only.
- ⚠️ The existing runtime property still checks field names.
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** crates/sl-viewer/tests/properties_viewer_bundle_diff.rs
**Line:** 284:290
**Comment:**
*Comment Mismatch: This is not a compile-time guarantee: the function is never invoked, and its parameters are arbitrary rather than tied to `diff_fields` or `EXPECTED_FIELD_NAMES`. Consequently, changing the production field list will not cause compilation to fail as the comment claims. Either remove the misleading comment/helper or invoke a real compile-time assertion with fixed values; the runtime property already provides the actual check.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
|
Closing due to merge conflicts. |
User description
Summary
Adds
crates/sl-viewer/tests/properties_viewer_bundle_diff.rswith 10 proptest properties pinning thebundle_diff::diff_fieldsandOkfBundle::from_bundlereductions (WBS-6.2 #434).This branch has been rebased onto
origin/main(which now includes #433 timeline, #442 search/memory, #445 history_tab) after the operator closed the original PR for merge conflicts. Net-new file only; conflicts were isolated to CHANGELOG.md, docs/ops/WBS.md, docs/ops/TRACEABILITY.json and resolved by keeping all entries.diff_fields(6 properties) — full stable field set, reflexive ona == a, idempotent on clone, value-flipped symmetric,differsmatchesvalue_a != value_b, em-dash fallback forNone/None.OkfBundle::from_bundle(4 properties) —message_countequals slice count, kind presence flags match,token_countfalls back to 0 when no Intent slice carries numericuser_turn_count,source_idcarries through.Validation
cargo test -p sl-viewer --test properties_viewer_bundle_diff --features "desktop parquet" --locked— 10 passedcargo fmt --all --check— cleanCodeAnt-AI Description
Add property-test coverage for viewer bundle comparisons and bundle summaries
What Changed
Impact
✅ Fewer regressions in bundle comparison results✅ Consistent viewer diff rows and missing-value display✅ Reliable bundle summary counts and flags💡 Usage Guide
Checking Your Pull Request
Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.
Talking to CodeAnt AI
Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:
This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.
Example
Preserve Org Learnings with CodeAnt
You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:
This helps CodeAnt AI learn and adapt to your team's coding style and standards.
Example
Retrigger review
Ask CodeAnt AI to review the PR again, by typing:
Check Your Repository Health
To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.