Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,10 @@ Follows [Keep a Changelog](https://keepachangelog.com/); versioning is [SemVer](

- CI drift cleanups (WBS-6.2 #432 follow-up): `scripts/rootless-matrix-check.ps1` `^ rootless-matrix-policy:.*?continue-on-error:\s*true` regex was bleeding across jobs into the next `security:` job's `continue-on-error: true`; replaced with `[regex]::Match` + a proper terminator (`(?=^ [A-Za-z][\w-]*:\s|\z)`) so the check scopes to just the policy block. Same script now throws when the policy block is absent (was treating a failed match as success). `.github/workflows/ci.yml` pins `actions/checkout` to the immutable `3d3c42e5` SHA + `persist-credentials: false` for both policy jobs. `.github/workflows/hermetic.yml` aligns its reusable-workflow pin to the documented `ec891654` SHA (was `a8db485` — drift between pin doc + caller). `crates/sl-viewer/src/web_exports.rs` uses `WebExportProvider::default_subdir` to populate the `defaults` array in `web_export_roots_with_env` instead of repeating literal strings, fixing a `dead_code` warning that broke `cargo test cli_help` under `-D warnings`.

- sl-viewer timeline property surface (WBS-6.2 #433): `crates/sl-viewer/tests/properties_viewer_timeline.rs` adds 16 proptest properties — `group_by_day` partitions every entry into exactly one group with no losses, orders groups chronologically, and labels empty-day groups `"(unknown date)"`. `normalize_widths` produces one width per input entry, all in `[MIN_PX, MAX_PX]`, with all-zero inputs collapsing to MIN_PX and the max-tokened entry rendering at MAX_PX. `model_hue` is deterministic and in `[0, 359]`; `model_color` matches `hsl(<hue>, 60%, 55%)`. `TimelineEntry::from_bundle` properties pin: `day` is the leading 10 chars of `created_at` (else empty), `goal` falls back to `"(no goal)"`, `model` falls back to `"unknown"`, `source_id` carries through, `message_count` / `has_acceptance` / `has_contract` match the input, and `token_count` falls back to 0 when no Intent slice carries a numeric `user_turn_count`.

- sl-viewer search/memory property surface (WBS-6.2 #435): `crates/sl-viewer/tests/properties_viewer_search_memory.rs` adds 12 proptest properties — `search_view::build_query` trims each field, emits a field iff its post-trim value is non-empty, encodes the documented break-character set (` `, `,`, `#`, `&`, `=`, `+`), and always emits `limit=` whose value is the parsed input or the documented `"50"` fallback. `advanced_filter_active_count` counts `min_tokens`/`tags` non-empty fields, treats `"50"` as the default for `limit`, and is trim-invariant. `memory_tab::to_wiki_page` carries `session_id` and `title` through unchanged and is deterministic across calls; `all_wiki_pages_from_sessions` produces exactly one page per input session, in input order.

- Wave-44 plan landed: `WAVE44_SCOPE.md` + `docs/ops/WAVE44_PERT.md` enumerate 6 close-out lanes (3 machine, 3 human-gated) for the 6 unpaid residuals from Wave-43 (396/402 → 402/402 target). Theme: stack-stability closure + i18n migration + eval coverage + supply-chain signing.
- Wave-44 reaudit (Wave-44-D): `audit/SCORECARD.md` refresh at commit `13c974f7` (machine-w44-reaudit); `docs/ops/TRACEABILITY.json` overall_audit wave=Wave-44 commit=13c974f7 (conservative hold at 396/402); `docs/ops/GAP_QA_MATRIX.md` C00 + C08 + PLAN-W8-B rows reflect Wave-44 closure (#368 W44-B6 corpus / #372 W44-B1 loom / #373 PERT correction). 2 of 3 machine lanes shipped 2026-07-24; remaining 6 raw pts across C04 L36 / C08 L76 / C11 L110.

Expand Down
280 changes: 280 additions & 0 deletions crates/sl-viewer/tests/properties_viewer_search_memory.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,280 @@
//! Property evidence for sl-viewer's `search_view` and `memory_tab` modules.
//!
//! Integration tests living alongside `properties_viewer*.rs`. The unit
//! tests in those modules pin specific values; these properties pin
//! invariants across the full shape of inputs the helpers receive.
//!
//! `search_view::build_query` invariants:
//! * Trimming: each field's `.trim()` form is what gets serialized —
// leading/trailing whitespace does not leak into the query string.
//! * Empty skipping: empty (post-trim) fields are omitted from the
//! query string entirely; non-empty fields all appear exactly once.
//! * Percent-encoding: characters that break a query string (` `, `,`,
//! `#`, `&`, `=`, `+`) are encoded; other characters pass through.
//! * `limit` is always present and is the parsed form of the input
//! (or `50` when parsing fails).
//!
//! `search_view::advanced_filter_active_count` invariants:
//! * Each non-empty `min_tokens`/`tags` field counts as 1.
//! * `limit` counts as 1 only when it differs from the documented
//! default `"50"` (post-trim). `"50"` does not count.
//!
//! `memory_tab::to_wiki_page` invariants:
//! * `session_id` is carried through unchanged.
//! * `title` is carried through unchanged (mirrors `Option<String>`
//! identity).
//! * Output length matches input: `all_wiki_pages_from_sessions(s)`
//! produces exactly `s.len()` pages.
//!
//! proptest is added to `sl-viewer/[dev-dependencies]` (mirroring the
//! workspace root); see PR #425 for the initial wiring.

use proptest::prelude::*;
use session_ledger::domain::session::{Corpus, Session};
use sl_viewer::memory_tab::{all_wiki_pages_from_sessions, to_wiki_page};
use sl_viewer::search_view::{advanced_filter_active_count, build_query};

// ── strategies ──────────────────────────────────────────────────────────────

/// A string that may contain one of the percent-encoding-relevant chars
/// or a safe ASCII character. We only test against a fixed alphabet so
/// the encoding contract is unambiguous.
fn input_str_strategy() -> impl Strategy<Value = String> {
prop::string::string_regex("[A-Za-z0-9 _,#&=+/]{0,32}").expect("valid regex")
}

/// A `Session` shaped by `Session::new` (which the rest of the
/// codebase uses to construct fixtures). We vary the optional title so
/// we can exercise `to_wiki_page` across `None` / `Some(s)` shapes.
fn session_strategy() -> impl Strategy<Value = Session> {
(
prop::string::string_regex("[a-z0-9-]{1,16}").expect("valid regex"),
prop::option::of(
prop::string::string_regex("[A-Za-z 0-9,_.-]{1,40}").expect("valid regex"),
),
)
.prop_map(|(suffix, title)| {
let id = format!("sess-{suffix}");
let mut s = Session::new(id, Corpus::Forge);
s.title = title;
s
})
}

// ── search_view::build_query ────────────────────────────────────────────────

proptest! {
/// Property: a `since` field that is empty (post-trim) does NOT
/// appear in the query string; a non-empty `since` field appears
/// exactly once (as `since=<urlencoded(since)>`).
#[test]
fn build_query_since_present_iff_nonempty(
since in input_str_strategy(),
until in prop::string::string_regex("[A-Za-z0-9 ,]{0,16}").expect("valid regex"),
model in input_str_strategy(),
min_tokens in input_str_strategy(),
tags in input_str_strategy(),
limit in prop::string::string_regex("[0-9]{0,3}").expect("valid regex"),
) {
let q = build_query(&since, &until, &model, &min_tokens, &tags, &limit);
let has_since = q.split('&').any(|kv| kv.starts_with("since="));
prop_assert_eq!(has_since, !since.trim().is_empty());
}

/// Property: limit is always present in the output. When `limit`
/// is a parseable non-negative integer the value matches the input
/// (post-trim); when unparseable or empty, the value is `"50"`
/// (the documented fallback).
#[test]
fn build_query_limit_always_present(
since in input_str_strategy(),
until in input_str_strategy(),
model in input_str_strategy(),
min_tokens in input_str_strategy(),
tags in input_str_strategy(),
limit in input_str_strategy(),
) {
let q = build_query(&since, &until, &model, &min_tokens, &tags, &limit);
let limit_val = q
.split('&')
.find_map(|kv| kv.strip_prefix("limit="))
.unwrap_or_else(|| panic!("limit missing from query: {q}"));

let expected = limit.trim().parse::<usize>().map(|n| n.to_string()).unwrap_or_else(|_| "50".to_string());
prop_assert_eq!(limit_val, expected.as_str());
}

/// Property: `build_query` is idempotent w.r.t. trimming. Calling
/// it with `" abc "` and `"abc"` for any field produces the same
/// output for that field.
#[test]
fn build_query_trims_field_values(
since in input_str_strategy(),
until in input_str_strategy(),
model in input_str_strategy(),
min_tokens in input_str_strategy(),
tags in input_str_strategy(),
limit in input_str_strategy(),
) {
let pad = |s: &str| format!(" {s} ");
let a = build_query(&since, &until, &model, &min_tokens, &tags, &limit);
let b = build_query(
&pad(&since),
&pad(&until),
&pad(&model),
&pad(&min_tokens),
&pad(&tags),
&pad(&limit),
);
prop_assert_eq!(a, b);
}

/// Property: characters that break query-string parsers (`#`, `&`,
/// `=`, `+`, `,`, ` `) are percent-encoded; plain ASCII
/// alphanumerics pass through unchanged. This is the same alphabet
/// the in-file `urlencoding` helper handles.
#[test]
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:?}",
);
}
}
Comment on lines +137 to +155

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Fix in Cursor Fix in VSCode Claude

(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
👍 | 👎

} else {
// All characters are safe ASCII alphanumerics; they should
// round-trip unchanged.
let model_part = q
.split('&')
.find_map(|kv| kv.strip_prefix("model="))
.unwrap_or_else(|| panic!("model missing from query: {q}"));
prop_assert_eq!(model_part, input.as_str());
}
}
}

// ── search_view::advanced_filter_active_count ───────────────────────────────

proptest! {
/// Property: each non-empty `min_tokens`/`tags` field counts as 1;
/// each empty (post-trim) one counts as 0.
#[test]
fn advanced_filter_count_per_field(
min_tokens in input_str_strategy(),
tags in input_str_strategy(),
limit in input_str_strategy(),
) {
let n = advanced_filter_active_count(&min_tokens, &tags, &limit);
let mut expected = 0usize;
if !min_tokens.trim().is_empty() {
expected += 1;
}
if !tags.trim().is_empty() {
expected += 1;
}
if limit.trim() != "50" {
expected += 1;
}
prop_assert_eq!(n, expected);
}

/// Property: `advanced_filter_active_count` is idempotent w.r.t.
/// trimming: padding the input strings doesn't change the count.
#[test]
fn advanced_filter_count_trim_invariant(
min_tokens in input_str_strategy(),
tags in input_str_strategy(),
limit in input_str_strategy(),
) {
let a = advanced_filter_active_count(&min_tokens, &tags, &limit);
let b = advanced_filter_active_count(
&format!(" {min_tokens} "),
&format!(" {tags} "),
&format!(" {limit} "),
);
prop_assert_eq!(a, b);
}

/// Property: the documented default limit `"50"` does not count.
/// Anything else (parseable, unparseable, padded) counts.
#[test]
fn advanced_filter_count_limit_default(
min_tokens in input_str_strategy(),
tags in input_str_strategy(),
body in input_str_strategy(),
) {
let default_count = advanced_filter_active_count(&min_tokens, &tags, "50");
let changed_count = advanced_filter_active_count(&min_tokens, &tags, &body);
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);
Comment on lines +220 to +222

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Fix in Cursor Fix in VSCode Claude

(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
👍 | 👎

}
}

// ── memory_tab::to_wiki_page ────────────────────────────────────────────────

proptest! {
/// Property: `to_wiki_page` carries the session id through
/// unchanged (the wiki page is keyed by session id).
#[test]
fn to_wiki_page_carries_session_id(session in session_strategy()) {
let page = to_wiki_page(&session);
prop_assert_eq!(page.session_id, session.id);
}

/// Property: `to_wiki_page` carries the session title through
/// unchanged. The title field is `Option<String>`; `None` stays
/// `None`, `Some(s)` stays `Some(s)`.
#[test]
fn to_wiki_page_carries_title(session in session_strategy()) {
let page = to_wiki_page(&session);
prop_assert_eq!(page.title, session.title);
}

/// Property: `all_wiki_pages_from_sessions` produces exactly one
/// page per session, in input order. Catches the obvious
/// flatten/filter bug where some sessions are dropped or reordered.
#[test]
fn all_wiki_pages_length_matches_input(
sessions in prop::collection::vec(session_strategy(), 0..8),
) {
let pages = all_wiki_pages_from_sessions(&sessions);
prop_assert_eq!(pages.len(), sessions.len());
}

/// Property: `all_wiki_pages_from_sessions` preserves input order
/// — page `i` corresponds to session `i`.
#[test]
fn all_wiki_pages_order_matches_input(
sessions in prop::collection::vec(session_strategy(), 1..8),
) {
let pages = all_wiki_pages_from_sessions(&sessions);
for (i, page) in pages.iter().enumerate() {
prop_assert_eq!(&page.session_id, &sessions[i].id);
}
}

/// Property: `to_wiki_page` is deterministic — applying it twice
/// to the same session yields the same page (every field, including
/// the `Option<String>` title, matches). This catches drift where
/// the extractors consult a non-deterministic source (e.g.
/// timestamps or RNG).
#[test]
fn to_wiki_page_is_deterministic(session in session_strategy()) {
let a = to_wiki_page(&session);
let b = to_wiki_page(&session);
prop_assert_eq!(a, b);
}
}
Loading
Loading