test(viewer): search_view + memory_tab proptest surface (WBS-6.2 #435) - #435
test(viewer): search_view + memory_tab proptest surface (WBS-6.2 #435)#435KooshaPari wants to merge 2 commits into
Conversation
Adds `crates/sl-viewer/tests/properties_viewer_timeline.rs` with
16 proptest properties pinning the timeline pure-helper reductions:
* `group_by_day` partitions every entry into exactly one group (no
losses), orders groups chronologically, and labels empty-day groups
with the literal `"(unknown date)"` string.
* `normalize_widths` produces one width per input entry, all in
`[MIN_PX, MAX_PX]`; empty or all-zero inputs collapse to MIN_PX;
the entry with the max `token_count` always renders at MAX_PX.
* `model_hue` is deterministic and lands in `[0, 359]`.
* `model_color` is deterministic and matches `hsl(<hue>, 60%, 55%)`.
* `TimelineEntry::from_bundle`:
* `day` is the leading 10 chars of `created_at` (else empty).
* `goal` falls back to `"(no goal)"` when no Intent slice
carries a string `goal` body.
* `model` falls back to `"unknown"` when no Context slice
carries a string `model` body.
* `source_id` carries through unchanged.
* `message_count` equals slice count; `has_acceptance` /
`has_contract` reflect kind presence (any-of).
* `token_count` falls back to 0 when no Intent slice carries a
numeric `user_turn_count`.
Updates WBS-6.2 evidence list, TRACEABILITY.json, and CHANGELOG.
Adds `crates/sl-viewer/tests/properties_viewer_search_memory.rs` with
12 proptest properties pinning `search_view` and `memory_tab`
reductions:
* `search_view::build_query`:
* Each field's post-trim value is what gets serialized — padding
on either side of a field value does not change the output.
* `since` (and by symmetry every other optional field) appears
in the query iff its post-trim value is non-empty.
* The documented break-character set (` `, `,`, `#`, `&`,
`=`, `+`) is percent-encoded; safe ASCII alphanumerics pass
through unchanged.
* `limit=` is always present and equals the parsed input or the
documented `"50"` fallback when parsing fails.
* `search_view::advanced_filter_active_count`:
* `min_tokens` and `tags` count as 1 each when non-empty
(post-trim); 0 otherwise.
* `limit"=="50"` does not count; any other value does.
* Trim-invariant: padded inputs produce the same count.
* `memory_tab::to_wiki_page`:
* `session_id` and `title` are carried through unchanged.
* Deterministic: applying it twice to the same session yields the
same `MemoryWikiPage`.
* `memory_tab::all_wiki_pages_from_sessions`:
* Output length equals input length.
* Order matches input order (page `i` ↔ session `i`).
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: 20 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 (5)
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:XL
![codeant-ai[bot]](https://avatars.githubusercontent.com/in/646884?s=60&v=4){kind=small}
| fn build_query_encodes_break_chars(input in "[-+=#&, a-zA-Z0-9]{1,8}") { | ||
| // Construct a query whose model field carries `input` and | ||
| // verify the encoded form below. | ||
| let q = build_query("", "", &input, "", "", "10"); | ||
| if input.contains([' ', ',', '#', '&', '=', '+']) { | ||
| // The raw character must not appear unescaped in the model | ||
| // value; the percent-encoded form must appear instead. | ||
| let model_part = q | ||
| .split('&') | ||
| .find_map(|kv| kv.strip_prefix("model=")) | ||
| .unwrap_or_else(|| panic!("model missing from query: {q}")); | ||
| for ch in input.chars() { | ||
| if [' ', ',', '#', '&', '=', '+'].contains(&ch) { | ||
| prop_assert!( | ||
| !model_part.contains(ch), | ||
| "raw character {ch:?} present in model value: {model_part:?}", | ||
| ); | ||
| } | ||
| } |
There was a problem hiding this comment.
Suggestion: The encoding property does not verify the required encoded value. It only checks that selected raw characters are absent, so an implementation that deletes delimiters or whitespace instead of encoding them would pass. Assert that the model component contains the expected percent-encoded input, accounting for the query separator when the input contains &. [incomplete implementation]
Severity Level: Major ⚠️
- ⚠️ Search queries can silently lose model characters.
- ⚠️ Delimiter-safe encoding regressions remain undetected.
- ❌ Query semantics can change for affected searches.
(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_search_memory.rs
**Line:** 137:155
**Comment:**
*Incomplete Implementation: The encoding property does not verify the required encoded value. It only checks that selected raw characters are absent, so an implementation that deletes delimiters or whitespace instead of encoding them would pass. Assert that the model component contains the expected percent-encoded input, accounting for the query separator when the input contains `&`.
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 body_changes_default = !min_tokens.trim().is_empty() || !tags.trim().is_empty() || body.trim() != "50"; | ||
| let body_changes_other = !min_tokens.trim().is_empty() || !tags.trim().is_empty() || body.trim() != "50"; | ||
| prop_assert_eq!(default_count < changed_count, body_changes_default && body_changes_other); |
There was a problem hiding this comment.
Suggestion: The assertion requires the count for body to be greater than the default-limit count whenever any other filter is active, even when body.trim() is exactly "50". In that case both calls return the same count, so valid generated inputs with non-empty min_tokens or tags fail the property. The expected condition must isolate whether body changes the limit, rather than including unrelated active filters. [incorrect condition logic]
Severity Level: Major ⚠️
- ❌ Property test fails for valid active-filter inputs.
- ⚠️ CI results depend on generated input combinations.
- ⚠️ Limit-count behavior is obscured by unrelated filters.
(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_search_memory.rs
**Line:** 220:222
**Comment:**
*Incorrect Condition Logic: The assertion requires the count for `body` to be greater than the default-limit count whenever any other filter is active, even when `body.trim()` is exactly `"50"`. In that case both calls return the same count, so valid generated inputs with non-empty `min_tokens` or `tags` fail the property. The expected condition must isolate whether `body` changes the limit, rather than including unrelated active filters.
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 flat: Vec<TimelineEntry> = | ||
| groups.iter().flat_map(|(_, v)| v.iter()).cloned().collect(); | ||
| prop_assert_eq!( | ||
| flat.len(), | ||
| entries.len(), | ||
| "every input entry must appear in exactly one group", | ||
| ); |
There was a problem hiding this comment.
Suggestion: This property only compares flattened length with input length. An implementation that drops one entry and duplicates another still passes, so it does not establish the documented partition invariant or catch the loss/duplication bug described by the test. Compare the flattened entries or their identities and multiplicities against the original input. [incomplete implementation]
Severity Level: Major ⚠️
- ⚠️ Timeline grouping regressions can evade property tests.
- ⚠️ Entries may be silently replaced or duplicated.
- ❌ Displayed timeline data could become incorrect.
(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_timeline.rs
**Line:** 133:139
**Comment:**
*Incomplete Implementation: This property only compares flattened length with input length. An implementation that drops one entry and duplicates another still passes, so it does not establish the documented partition invariant or catch the loss/duplication bug described by the test. Compare the flattened entries or their identities and multiplicities against the original input.
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| heavy_tokens in 1u64..1_000_000, | ||
| light_tokens in 0u64..1000, | ||
| rest in 0usize..6, |
There was a problem hiding this comment.
Suggestion: The generated heavy_tokens value is not guaranteed to exceed light_tokens: it can be 1 while light_tokens is 1000. In that valid case, the first entry is not the maximum and normalize_widths correctly assigns MAX_PX to a later entry, causing this property to fail spuriously. Constrain light_tokens below heavy_tokens or generate the heavy value after the light value. [incorrect condition logic]
Severity Level: Major ⚠️
- ❌ Timeline property fails for valid generated token counts.
- ⚠️ CI can report false failures in width normalization.
- ⚠️ The test does not reliably identify the maximum entry.
(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_timeline.rs
**Line:** 248:250
**Comment:**
*Incorrect Condition Logic: The generated `heavy_tokens` value is not guaranteed to exceed `light_tokens`: it can be `1` while `light_tokens` is `1000`. In that valid case, the first entry is not the maximum and `normalize_widths` correctly assigns `MAX_PX` to a later entry, causing this property to fail spuriously. Constrain `light_tokens` below `heavy_tokens` or generate the heavy value after the light value.
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. |
#442) Adds `crates/sl-viewer/tests/properties_viewer_search_memory.rs` with 12 proptest properties pinning `search_view` and `memory_tab` reductions: * `search_view::build_query`: * Each field's post-trim value is what gets serialized — padding on either side of a field value does not change the output. * `since` (and by symmetry every other optional field) appears in the query iff its post-trim value is non-empty. * The documented break-character set (` `, `,`, `#`, `&`, `=`, `+`) is percent-encoded; safe ASCII alphanumerics pass through unchanged. * `limit=` is always present and equals the parsed input or the documented `"50"` fallback when parsing fails. * `search_view::advanced_filter_active_count`: * `min_tokens` and `tags` count as 1 each when non-empty (post-trim); 0 otherwise. * `limit"=="50"` does not count; any other value does. * Trim-invariant: padded inputs produce the same count. * `memory_tab::to_wiki_page`: * `session_id` and `title` are carried through unchanged. * Deterministic: applying it twice to the same session yields the same `MemoryWikiPage`. * `memory_tab::all_wiki_pages_from_sessions`: * Output length equals input length. * Order matches input order (page `i` ↔ session `i`). 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_search_memory.rswith 12 proptest properties pinning thesearch_viewandmemory_tabpure-helper reductions (WBS-6.2 #435).search_view::build_query(4 properties)since(and by symmetry every other optional field) appears in the query iff its post-trim value is non-empty.,,#,&,=,+) is percent-encoded; safe ASCII alphanumerics pass through unchanged.limit=is always present and equals the parsed input or the documented"50"fallback when parsing fails.search_view::advanced_filter_active_count(3 properties)min_tokensandtagscount as 1 each when non-empty (post-trim); 0 otherwise.limit == "50"does not count; any other value does.memory_tab::to_wiki_page(3 properties)session_idandtitleare carried through unchanged.MemoryWikiPage.memory_tab::all_wiki_pages_from_sessions(2 properties)i↔ sessioni).Validation
cargo test -p sl-viewer --test properties_viewer_search_memory --features "desktop parquet" --locked— 12 passedcargo clippy -p sl-viewer --test properties_viewer_search_memory --features "desktop parquet" --locked -- -D warnings— cleancargo fmt --all --check— cleanWBS / TRACEABILITY
WBS-6.2 evidence list and
TRACEABILITY.jsongaincrates/sl-viewer/tests/properties_viewer_search_memory.rs. Status stayspartial(fuzzing cadence, full loom/shuttle, perf-budget gates remain). CHANGELOG Unreleased documents the new surface.Branch reuses the existing
fix/viewer-timeline-properties-20260808worktree (no conflict — this PR is layered on top of #433's HEAD, butcrates/sl-viewer/tests/properties_viewer_search_memory.rsis the only net-new file; CHANGELOG/WBS/TRACEABILITY edits are additive).CodeAnt-AI Description
Add property coverage for viewer timeline, search, and memory behavior
What Changed
Impact
✅ Fewer timeline grouping and rendering regressions✅ Consistent search filtering and query encoding✅ Reliable session-to-memory page conversion💡 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.