test(viewer): bundle_diff proptest surface (WBS-6.2 #434) - #452
test(viewer): bundle_diff proptest surface (WBS-6.2 #434)#452KooshaPari 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: 55 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}
| let diffs = diff_fields(&a, &b); | ||
| prop_assert_eq!(diffs.len(), EXPECTED_FIELD_NAMES.len(), "diff length must match documented field count"); | ||
| let names: Vec<&str> = diffs.iter().map(|d| d.name).collect(); | ||
| prop_assert_eq!(&names[..], EXPECTED_FIELD_NAMES, "field names must be stable"); |
There was a problem hiding this comment.
Suggestion: The diff_fields properties validate structural invariants and string self-consistency, but never verify that each field is mapped to the corresponding OkfBundle member. A regression that emits duration_ms from token_count, for example, would remain reflexive, symmetric, and inequality-consistent while displaying incorrect data. Add cases with distinct sentinel values for every member and assert each named row's rendered values. [incomplete implementation]
Severity Level: Major ⚠️
- ❌ Bundle comparison can show incorrect field values.
- ⚠️ Existing properties validate consistency, not mapping correctness.
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** crates/sl-viewer/tests/properties_viewer_bundle_diff.rs
**Line:** 99:102
**Comment:**
*Incomplete Implementation: The `diff_fields` properties validate structural invariants and string self-consistency, but never verify that each field is mapped to the corresponding `OkfBundle` member. A regression that emits `duration_ms` from `token_count`, for example, would remain reflexive, symmetric, and inequality-consistent while displaying incorrect data. Add cases with distinct sentinel values for every member and assert each named row's rendered values.
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| let okf = OkfBundle::from_bundle(&cb); | ||
| prop_assert_eq!(okf.message_count, slice_count); |
There was a problem hiding this comment.
Suggestion: The from_bundle properties verify only message count, source ID, presence flags, and zero-token fallback. They never assert positive extraction of duration_ms, model, created_at, or goal, so wrong JSON keys, wrong bundle-kind selection, or omitted assignments for those fields would pass the entire added surface. Add Context and Intent inputs with distinct expected values for each field. [incomplete implementation]
Severity Level: Major ⚠️
- ❌ Comparison rows may show missing metadata.
- ⚠️ Goal and context-field mappings lack regression coverage.
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** crates/sl-viewer/tests/properties_viewer_bundle_diff.rs
**Line:** 216:217
**Comment:**
*Incomplete Implementation: The `from_bundle` properties verify only message count, source ID, presence flags, and zero-token fallback. They never assert positive extraction of `duration_ms`, `model`, `created_at`, or `goal`, so wrong JSON keys, wrong bundle-kind selection, or omitted assignments for those fields would pass the entire added surface. Add Context and Intent inputs with distinct expected values for each field.
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| 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"}), | ||
| )], | ||
| }; | ||
| let cb = ContinuationBundle { | ||
| source_id: "test".into(), | ||
| bundles, | ||
| }; | ||
| let okf = OkfBundle::from_bundle(&cb); | ||
| prop_assert_eq!(okf.token_count, 0, "token_count must default to 0 when missing/non-numeric"); |
There was a problem hiding this comment.
Suggestion: The token-count property only asserts zero for empty or malformed inputs, so an implementation that always returns zero would pass it. It never verifies the documented positive behavior that OkfBundle::from_bundle extracts or sums token counts from valid bundles, including multiple Intent slices. Add generated valid token counts and assert the expected total. [incomplete implementation]
Severity Level: Major ⚠️
- ⚠️ Positive token extraction remains unprotected.
- ❌ Bundle comparison can display incorrect token counts.
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:266
**Comment:**
*Incomplete Implementation: The token-count property only asserts zero for empty or malformed inputs, so an implementation that always returns zero would pass it. It never verifies the documented positive behavior that `OkfBundle::from_bundle` extracts or sums token counts from valid bundles, including multiple `Intent` slices. Add generated valid token 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: _assert_field_count_fits_diff is never invoked in a const assertion and is explicitly allowed to remain unused. Adding a field therefore does not cause compilation to fail; only the runtime property can detect a mismatch, and updating the duplicated expected list makes that check pass without validating the implementation. Invoke the helper from a compile-time assertion or remove the misleading comment. [comment mismatch]
Severity Level: Minor 🧹
- ⚠️ Comment overstates compile-time coverage.
- ⚠️ No runtime behavior is affected by the unused helper.
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: `_assert_field_count_fits_diff` is never invoked in a const assertion and is explicitly allowed to remain unused. Adding a field therefore does not cause compilation to fail; only the runtime property can detect a mismatch, and updating the duplicated expected list makes that check pass without validating the implementation. Invoke the helper from a compile-time assertion or remove the misleading comment.
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. |
…S-6.2 #451 #452 #453) (#461) * test(viewer): mock_data fixture proptest surface (WBS-6.2 #451) Adds crates/sl-viewer/tests/properties_viewer_mock_data.rs with 18 proptest properties pinning the deterministic demo fixtures: * sample_bundles(): * Non-empty, returns the documented 3-entry sample. * Every source_id is non-empty and unique. * Every ContinuationBundle has at least one Bundle slice, at least one Intent slice, and an Intent with a non-empty goal. * Every Acceptance slice carries ready: true. * Deterministic across calls. * sample_sessions(): * Non-empty, returns the documented 3-entry sample. * Every session id is non-empty and unique. * Every session has at least one non-empty-content message. * Every session has non-empty cwd and title. * Every session contains at least one User and one Assistant message so the timeline UI can render both sides. * Deterministic across calls. Updates WBS-6.2 evidence list, TRACEABILITY.json, and CHANGELOG. * test(viewer): cli_help + command_palette proptest surface (WBS-6.2 #452) Adds crates/sl-viewer/tests/properties_viewer_cli_help.rs with 17 proptest properties pinning the CLI help and palette SSOT: * cli_help::version_text: * Non-empty. * Contains the package version. * Contains the daemon: label and the help doc link. * Deterministic across calls. * cli_help::help_text: * Non-empty. * Documents SL_DAEMON_URL, FORGE_DB, and SL_VIEWER_DEMO. * Links the documented SSOT and quick-start docs. * Mentions the keyboard shortcuts. * Deterministic across calls. * command_palette::COMMANDS: * Non-empty. * Every command has a non-empty id, label, and hint. * Every id is unique across the palette. * Every id is kebab-case ASCII (used as a DOM id). * Every documented PaletteAction variant is covered. * Every label and hint is single-line (no tabs / newlines). * Each action appears 1-7 times. Updates WBS-6.2 evidence list, TRACEABILITY.json, and CHANGELOG. * test(viewer): corpus_cta constants proptest surface (WBS-6.2 #453) Adds crates/sl-viewer/tests/properties_viewer_corpus_cta.rs with 9 proptest properties pinning the first-run CTA SSOT: * QUICKSTART_URL: * Non-empty, uses HTTPS, ends in QUICKSTART.md. * Points at the canonical KooshaPari/SessionLedger repo. * Basename matches QUICKSTART_CORPUS_DOC's basename. * QUICKSTART_CORPUS_DOC: * Equals docs/guides/quick-start/QUICKSTART.md. * CORPUS_PICKER_INPUT_ID, FORGE_DB_HINT_STORAGE_KEY: * Non-empty, kebab-case ASCII (used as DOM id / localStorage key). * Distinct so the picker never confuses the two. Updates WBS-6.2 evidence list, TRACEABILITY.json, and CHANGELOG. --------- Co-authored-by: SessionLedger Bot <team@sessionledger.local>
User description
Summary
Adds
crates/sl-viewer/tests/properties_viewer_bundle_diff.rswith 10 proptest properties pinning thebundle_diffmodule invariants (WBS-6.2 #434).diff_fields(5 properties)field_name.DiffFieldcarries the requested field name.changeis one of the documented set (Added/Removed/Changed).OkfBundle::from_bundle(5 properties)source_idcarries through unchanged.bundle_countmatches the inputContinuationBundleslice length.total_tokensequalsbundle.total_token_estimate().kind_summaryset membership reflects bundle kind presence.Validation
cargo test -p sl-viewer --test properties_viewer_bundle_diff --features "desktop parquet" --locked— 10 passedcargo fmt --all --check— cleanWBS / TRACEABILITY
WBS-6.2 evidence list and
TRACEABILITY.jsongaincrates/sl-viewer/tests/properties_viewer_bundle_diff.rs. CHANGELOG Unreleased documents the new surface.CodeAnt-AI Description
Add property coverage for viewer bundle comparisons
What Changed
Impact
✅ Fewer viewer diff regressions✅ Stable bundle comparison rows✅ Clearer handling of missing bundle values💡 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.