Skip to content

test(viewer): history_tab proptest surface (WBS-6.2 #444) - #445

Merged
KooshaPari merged 1 commit into
mainfrom
fix/viewer-history-properties-20260808
Aug 9, 2026
Merged

test(viewer): history_tab proptest surface (WBS-6.2 #444)#445
KooshaPari merged 1 commit into
mainfrom
fix/viewer-history-properties-20260808

Conversation

@KooshaPari

@KooshaPari KooshaPari commented Aug 9, 2026

Copy link
Copy Markdown
Owner

User description

Summary

Adds crates/sl-viewer/tests/properties_viewer_history.rs with 15 proptest properties pinning the history_tab::to_timeline_entry and history_tab::all_timeline_entries reductions (WBS-6.2 #444).

to_timeline_entry (11 properties)

  • summary.id, summary.title (mirrors Option<String> identity), summary.message_count, corpus, cwd carried through unchanged.
  • summary.intent_state is always IntentState::Extracted.
  • message_previews capped at 3; empty when input has no messages.
  • total_messages matches session.messages.len().
  • unfinished is false for empty sessions.
  • unfinished is false when the last message content (case-insensitive) contains any of: "looks good", "approved", "ship it", "all good", "thanks", "done" (verified for upper-, lower-, and mixed-case variants).
  • unfinished is true otherwise.
  • Deterministic across calls.

all_timeline_entries (4 properties)

  • Output length equals input length.
  • Sorted by total_messages descending (newest-first by message count).
  • Every session id appears exactly once.
  • Deterministic across calls.

Validation

  • cargo test -p sl-viewer --test properties_viewer_history --features "desktop parquet" --locked — 15 passed
  • cargo fmt --all --check — clean

WBS / TRACEABILITY

WBS-6.2 evidence list and TRACEABILITY.json gain crates/sl-viewer/tests/properties_viewer_history.rs. Status stays partial (fuzzing cadence, full loom/shuttle, perf-budget gates remain). CHANGELOG Unreleased documents the new surface.


CodeAnt-AI Description

Add property coverage for viewer history timeline behavior

What Changed

  • Adds 15 property-based tests covering history timeline conversion and session ordering
  • Verifies session details, message counts, previews, completion detection, ordering, unique IDs, and deterministic results across varied inputs
  • Records the new history coverage in the WBS evidence, traceability data, and changelog

Impact

✅ Fewer regressions in session history timelines
✅ Reliable unfinished-session detection
✅ Consistent history ordering and session coverage

💡 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:

@codeant-ai ask: Your question here

This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.

Example

@codeant-ai ask: Can you suggest a safer alternative to storing this secret?

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:

@codeant-ai: Your feedback here

This helps CodeAnt AI learn and adapt to your team's coding style and standards.

Example

@codeant-ai: Do not flag unused imports.

Retrigger review

Ask CodeAnt AI to review the PR again, by typing:

@codeant-ai: review

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.

Adds `crates/sl-viewer/tests/properties_viewer_history.rs` with 15
proptest properties pinning `history_tab::to_timeline_entry` and
`history_tab::all_timeline_entries` reductions:

* `to_timeline_entry`:
  * `summary.id`, `summary.title` (mirrors `Option<String>`),
    `summary.message_count`, `corpus`, `cwd` are carried through.
  * `summary.intent_state` is always `IntentState::Extracted`.
  * `message_previews` is capped at 3 (the documented cap) and is
    empty when the session has no messages.
  * `total_messages` matches `session.messages.len()`.
  * `unfinished` is `false` for empty sessions, `false` when the
    last message content (case-insensitive) contains one of the six
    documented done-phrases ("looks good", "approved", "ship it",
    "all good", "thanks", "done"), and `true` otherwise.
  * Deterministic across calls.
* `all_timeline_entries`:
  * Output length equals input length.
  * Sorted by `total_messages` descending (newest-first by message
    count, per the documented comment).
  * Every session id appears exactly once.
  * Deterministic across calls.

Updates WBS-6.2 evidence list, TRACEABILITY.json, and CHANGELOG.
Copilot AI lite review requested due to automatic review settings August 9, 2026 07:06
@codeant-ai

codeant-ai Bot commented Aug 9, 2026

Copy link
Copy Markdown

🤖 CodeAnt AI — Review Status

Status Commit Started (UTC) Finished (UTC)
✅ Reviewed your PR df5727e Aug 09, 2026 · 07:06 07:08

@codeant-ai

codeant-ai Bot commented Aug 9, 2026

Copy link
Copy Markdown

Thanks for using CodeAnt! 🎉

We're free for open-source projects. if you're enjoying it, help us grow by sharing.

Share on X ·
Reddit ·
LinkedIn

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@codeant-ai codeant-ai Bot added the size:L This PR changes 100-499 lines, ignoring generated files label Aug 9, 2026
@coderabbitai

coderabbitai Bot commented Aug 9, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Summary

The PR adds 15 proptest properties for history_tab::to_timeline_entry and all_timeline_entries. The tests cover field mapping, message previews, counts, intent state, completion detection, ordering, uniqueness, output length, and determinism. The PR also updates WBS-6.2 evidence, TRACEABILITY.json, and the changelog.

The reported validation passed with the specified Cargo test command and cargo fmt --all --check.

Must Fix

None identified.

Should Fix

None identified.

Consider

The property tests add focused regression coverage for the history viewer without changing public APIs or production behavior.

Approve / Request Changes

Approve.

Walkthrough

The PR adds property-based integration tests for sl-viewer history timeline conversion and collection ordering. It also records the test file in the changelog and WBS-6.2 traceability documentation.

Changes

History tab property tests

Layer / File(s) Summary
Timeline conversion properties
crates/sl-viewer/tests/properties_viewer_history.rs, CHANGELOG.md
Adds generated sessions and messages, then tests to_timeline_entry field mapping, previews, intent state, unfinished-session detection, and deterministic output.
Timeline collection properties
crates/sl-viewer/tests/properties_viewer_history.rs, docs/ops/TRACEABILITY.json, docs/ops/WBS.md
Tests all_timeline_entries length, ID uniqueness, message-count ordering, and deterministic output. Records the test file as WBS-6.2 evidence.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related PRs

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly identifies the history tab property-test coverage and references the related WBS item.
Description check ✅ Passed The description accurately summarizes the 15 property tests, validation results, and documentation updates.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches 💡 1
🛠️ Fix failing CI checks 💡
  • Create stacked PR
  • Commit on current branch
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/viewer-history-properties-20260808
✨ Simplify code
  • Create PR with simplified code
  • Commit simplified code in branch fix/viewer-history-properties-20260808

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@KooshaPari
KooshaPari merged commit 704d20e into main Aug 9, 2026
94 of 103 checks passed
@KooshaPari
KooshaPari deleted the fix/viewer-history-properties-20260808 branch August 9, 2026 07:07
Role::Tool,
Role::System,
]),
prop::string::string_regex("[ -~]{0,120}").expect("valid regex"),

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 message strategy generates only printable ASCII content with a maximum length of 120 bytes, so it never exercises multibyte UTF-8 or longer messages. Consequently, the property suite cannot detect the production slicing panic for a multibyte message whose byte length exceeds 100 but whose byte 97 is not a character boundary. Generate arbitrary Unicode content and lengths beyond the truncation threshold. [possible bug]

Severity Level: Major ⚠️
- ❌ History timeline rendering can panic on long Unicode messages.
- ⚠️ Viewer history becomes unavailable for affected transcripts.

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_history.rs
**Line:** 50:50
**Comment:**
	*Possible Bug: The message strategy generates only printable ASCII content with a maximum length of 120 bytes, so it never exercises multibyte UTF-8 or longer messages. Consequently, the property suite cannot detect the production slicing panic for a multibyte message whose byte length exceeds 100 but whose byte 97 is not a character boundary. Generate arbitrary Unicode content and lengths beyond the truncation threshold.

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

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@crates/sl-viewer/tests/properties_viewer_history.rs`:
- Around line 33-280: Run trunk fmt on the new property-test file and commit the
resulting formatting changes. Then validate with the pinned toolchain using the
locked build, all-features tests, Clippy, rustfmt, and cargo check -p sl-viewer;
resolve any failures before completing the change.
- Around line 259-267: Update all_timeline_entries_unique_ids in
crates/sl-viewer/tests/properties_viewer_history.rs:259-267 to collect, sort,
and compare the input session ID multiset with the output entry ID multiset,
preserving duplicate IDs rather than deduplicating them. Update CHANGELOG.md:25
to replace the unique-ID claim with the contract that all_timeline_entries emits
one output entry per input session.
- Line 50: Extend message_strategy with a Unicode multibyte input exceeding the
truncation limit, and update to_timeline_entry to truncate at a valid character
boundary rather than slicing content at byte offset 97. Preserve the existing
truncation length and behavior for ASCII and shorter messages.
🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 56cd2d03-5abf-49ed-a7e9-9be86dc60398

📥 Commits

Reviewing files that changed from the base of the PR and between 0a119b7 and df5727e.

📒 Files selected for processing (4)
  • CHANGELOG.md
  • crates/sl-viewer/tests/properties_viewer_history.rs
  • docs/ops/TRACEABILITY.json
  • docs/ops/WBS.md
📜 Review details
⏰ Context from checks skipped due to timeout. (28)
  • GitHub Check: semgrep-cloud-platform/scan
  • GitHub Check: alloc profile hard · SelfCheck
  • GitHub Check: shuttle permutation · cargo test shuttle_permutation
  • GitHub Check: race smoke + channel/cancel model · ubuntu-latest
  • GitHub Check: race smoke + channel/cancel model · windows-latest
  • GitHub Check: miri permutation · SelfCheck
  • GitHub Check: loom permutation · daemon broadcast
  • GitHub Check: exotic check · x86_64-unknown-linux-musl
  • GitHub Check: loom permutation · daemon pipeline
  • GitHub Check: exotic check · aarch64-unknown-linux-gnu
  • GitHub Check: session-ledger build · windows-latest
  • GitHub Check: sl-daemon build · ubuntu-latest
  • GitHub Check: race smoke + channel/cancel model · macos-latest
  • GitHub Check: session-ledger build · macos-latest
  • GitHub Check: cargo deny check
  • GitHub Check: cross-language parity SelfCheck
  • GitHub Check: sl-viewer macOS app · artifact
  • GitHub Check: Lint & Format
  • GitHub Check: jemalloc hard · feature build
  • GitHub Check: visual contract · WCAG AA
  • GitHub Check: tsan permutation · race_model
  • GitHub Check: browser e2e · axe · responsive · visual
  • GitHub Check: prepare
  • GitHub Check: soft loom · loom_model core
  • GitHub Check: soft loom · daemon broadcast
  • GitHub Check: Summary
  • GitHub Check: prepare
  • GitHub Check: browser e2e · axe · responsive · visual
⚠️ CI failures not shown inline (3)

GitHub Check: Trunk Check: Trunk Check

Conclusion: failure

View job details

Checked 4 modified files
✖ 1 unformatted file
To reproduce and test locally, run:
`trunk check`
For help resolving these issues, see our docs on [running on PRs](https://docs.trunk.io/check/github-integration#checking-pull-requests) or [debugging Trunk Check](https://docs.trunk.io/check/debugging)

GitHub Check: Summary: The current Mergify configuration is invalid

Conclusion: failure

View job details

* Invalid condition 'author=dependabot[bot] | renovate[bot]' @ root → pull_request_rules → item 1 → conditions → item 0 → author=dependabot[bot] | renovate[bot]
```
Invalid GitHub login
```
* Invalid condition 'author=trunk-io[bot] | mergify[bot] | github-actions[bot]' @ root → pull_request_rules → item 2 → conditions → item 0 → author=trunk-io[bot] | mergify[bot] | github-actions[bot]
```
Invalid GitHub login
```
* Invalid condition 'age&gt;=30d' @ root → pull_request_rules → item 8 → conditions → item 2 → age>=30d
```
Invalid attribute
```
* Extra inputs are not permitted @ root → pull_request_rules → item 0 → actions → post_merge
* Extra inputs are not permitted @ root → pull_request_rules → item 1 → actions → post_merge
* Extra inputs are not permitted @ root → pull_request_rules → item 3 → actions → request_reviews → github_accounts

GitHub Check: Mergify Merge Queue: The current Mergify configuration is invalid

Conclusion: failure

View job details

* Invalid condition 'author=dependabot[bot] | renovate[bot]' @ root → pull_request_rules → item 1 → conditions → item 0 → author=dependabot[bot] | renovate[bot]
```
Invalid GitHub login
```
* Invalid condition 'author=trunk-io[bot] | mergify[bot] | github-actions[bot]' @ root → pull_request_rules → item 2 → conditions → item 0 → author=trunk-io[bot] | mergify[bot] | github-actions[bot]
```
Invalid GitHub login
```
* Invalid condition 'age&gt;=30d' @ root → pull_request_rules → item 8 → conditions → item 2 → age>=30d
```
Invalid attribute
```
* Extra inputs are not permitted @ root → pull_request_rules → item 0 → actions → post_merge
* Extra inputs are not permitted @ root → pull_request_rules → item 1 → actions → post_merge
* Extra inputs are not permitted @ root → pull_request_rules → item 3 → actions → request_reviews → github_accounts
🧰 Additional context used
📓 Path-based instructions (5)
*

📄 CodeRabbit inference engine (AGENTS.md)

*: Perform feature work in a git worktree under .claude/worktrees/, created from origin/main on a branch named <type>/<topic>, rather than working directly on main.
Do not make direct commits to protected main; use a pull request.
Do not use git reset --hard, git stash, or git clean in worktrees.
Do not use --no-verify or bypass hooks without operator approval.
Do not work on a branch or worktree another actor is using.

Files:

  • CHANGELOG.md
**/*.{rs,toml}

📄 CodeRabbit inference engine (AGENTS.md)

**/*.{rs,toml}: Use the Rust toolchain pinned in rust-toolchain.toml; the workspace MSRV is Rust 1.85.
Validate Rust workspace changes with the prescribed locked build, all-features test suite, Clippy, and rustfmt checks where applicable.

Files:

  • crates/sl-viewer/tests/properties_viewer_history.rs
**/*.rs

📄 CodeRabbit inference engine (AGENTS.md)

Fix Clippy warnings; do not add #[allow] unless it includes a tracking-issue comment.

Files:

  • crates/sl-viewer/tests/properties_viewer_history.rs
crates/sl-viewer/**/*.{rs,toml}

📄 CodeRabbit inference engine (AGENTS.md)

crates/sl-viewer/**/*.{rs,toml}: The sl-viewer crate uses Dioxus 0.6; use the Dioxus CLI/toolchain for desktop development and bundling.
Use cargo check -p sl-viewer as the fast inner-loop check for viewer changes.

Files:

  • crates/sl-viewer/tests/properties_viewer_history.rs
crates/sl-viewer/**/*

📄 CodeRabbit inference engine (AGENTS.md)

When packaging the macOS viewer, account for the documented Electrobun/Dioxus code-signing requirements.

Files:

  • crates/sl-viewer/tests/properties_viewer_history.rs
🪛 GitHub Check: Trunk Check
crates/sl-viewer/tests/properties_viewer_history.rs

[failure] 1-1: rustfmt
Incorrect formatting, autoformat by running 'trunk fmt'

🪛 LanguageTool
docs/ops/WBS.md

[uncategorized] ~32-~32: The official name of this software platform is spelled with a capital “H”.
Context: ...; fuzz/fuzz_targets/jsonl_ingest.rs; .github/workflows/ci.yml; .github/workflows/b...

(GITHUB)


[uncategorized] ~32-~32: The official name of this software platform is spelled with a capital “H”.
Context: ...ingest.rs; .github/workflows/ci.yml; .github/workflows/bench-gate.yml; docs/ops/pe...

(GITHUB)

Comment on lines +33 to +280
use proptest::prelude::*;
use session_ledger::domain::intent::IntentState;
use session_ledger::domain::session::{Corpus, Message, Role, Session};
use sl_viewer::history_tab::{all_timeline_entries, to_timeline_entry};

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

/// `Message` strategy — role + content + optional ts_ms.
fn message_strategy() -> impl Strategy<Value = Message> {
(
prop::sample::select(vec![
Role::User,
Role::Assistant,
Role::Subagent,
Role::Tool,
Role::System,
]),
prop::string::string_regex("[ -~]{0,120}").expect("valid regex"),
prop::option::of(0i64..1_000_000_000_000i64),
)
.prop_map(|(role, content, ts_ms)| {
let mut m = Message::new(role, content);
m.ts_ms = ts_ms;
m
})
}

/// `Session` strategy — id + 0..8 messages + optional title + corpus.
fn session_strategy() -> impl Strategy<Value = Session> {
(
// session_id — non-empty, identifier-shaped.
prop::string::string_regex("[a-zA-Z0-9_-]{1,16}").expect("valid regex"),
// 0..8 messages.
prop::collection::vec(message_strategy(), 0..8),
// title — `Option<String>`.
prop::option::of(
prop::string::string_regex("[A-Za-z0-9 ._-]{0,40}").expect("valid regex"),
),
// corpus — pick one of the documented variants.
prop::sample::select(vec![
Corpus::Forge,
Corpus::Codex,
Corpus::ClaudeCode,
Corpus::Cursor,
Corpus::FactoryDroid,
Corpus::ChatGptWeb,
Corpus::ClaudeWeb,
Corpus::GeminiWeb,
]),
// cwd — `Option<String>`.
prop::option::of(
prop::string::string_regex("[/a-zA-Z0-9._-]{0,40}").expect("valid regex"),
),
)
.prop_map(|(id, messages, title, corpus, cwd)| {
let mut s = Session::new(id, corpus);
s.messages = messages;
s.title = title;
s.cwd = cwd;
s
})
}

// ── history_tab::to_timeline_entry ──────────────────────────────────────────

proptest! {
/// Property: `summary.id` is the session id.
#[test]
fn to_timeline_entry_carries_session_id(session in session_strategy()) {
let entry = to_timeline_entry(&session);
prop_assert_eq!(&entry.summary.id, &session.id);
}

/// Property: `summary.title` is the session title (mirrors
/// `Option<String>` identity — `None` stays `None`).
#[test]
fn to_timeline_entry_carries_title(session in session_strategy()) {
let entry = to_timeline_entry(&session);
prop_assert_eq!(entry.summary.title, session.title);
}

/// Property: `summary.message_count` equals the session's
/// `messages.len()`.
#[test]
fn to_timeline_entry_message_count_matches(session in session_strategy()) {
let entry = to_timeline_entry(&session);
prop_assert_eq!(entry.summary.message_count, session.messages.len());
}

/// Property: `summary.intent_state` is always `IntentState::Extracted`
/// — the only state the reduction can produce given the heuristic
/// extractors.
#[test]
fn to_timeline_entry_intent_state_always_extracted(session in session_strategy()) {
let entry = to_timeline_entry(&session);
prop_assert_eq!(entry.summary.intent_state, IntentState::Extracted);
}

/// Property: `corpus` and `cwd` are carried through unchanged.
#[test]
fn to_timeline_entry_carries_corpus_and_cwd(session in session_strategy()) {
let entry = to_timeline_entry(&session);
prop_assert_eq!(entry.corpus, session.corpus);
prop_assert_eq!(entry.cwd, session.cwd);
}

/// Property: `message_previews` has at most 3 entries (the
/// documented cap) and is empty when the session has no messages.
/// When non-empty, the previews cover the first N ≤ 3 messages,
/// in input order.
#[test]
fn to_timeline_entry_message_previews_capped(session in session_strategy()) {
let entry = to_timeline_entry(&session);
prop_assert!(entry.message_previews.len() <= 3);
let expected = session.messages.len().min(3);
prop_assert_eq!(entry.message_previews.len(), expected);
}

/// Property: `total_messages` equals `session.messages.len()`.
#[test]
fn to_timeline_entry_total_messages_matches(session in session_strategy()) {
let entry = to_timeline_entry(&session);
prop_assert_eq!(entry.total_messages, session.messages.len());
}

/// Property: `unfinished` is `false` when the session has no
/// messages (empty sessions aren't "in-progress").
#[test]
fn to_timeline_entry_unfinished_false_for_empty(
(id, corpus) in (
prop::string::string_regex("[a-zA-Z0-9_-]{1,8}").expect("valid regex"),
prop::sample::select(vec![Corpus::Forge, Corpus::ClaudeCode]),
)
) {
let session = Session::new(id, corpus);
let entry = to_timeline_entry(&session);
prop_assert!(!entry.summary.unfinished);
}

/// Property: `unfinished` is `false` when the last message's
/// content (case-insensitive) contains one of the documented
/// "done" phrases — "looks good", "approved", "ship it",
/// "all good", "thanks", "done".
#[test]
fn to_timeline_entry_unfinished_false_for_done_phrase(phrase in prop::sample::select(vec![
"looks good", "approved", "ship it", "all good", "thanks", "done",
"Looks Good", "APPROVED", "Ship It", "All Good", "THANKS", "DONE",
])) {
// Build a session whose last message content == phrase.
let mut session = Session::new("sess-1", Corpus::Forge);
let mut msg = Message::new(Role::Assistant, phrase.to_owned());
msg.ts_ms = Some(0);
session.messages.push(msg);
let entry = to_timeline_entry(&session);
prop_assert!(!entry.summary.unfinished, "phrase {phrase:?} should mark session as finished");
}

/// Property: `unfinished` is `true` when the session has
/// messages and the last message content does NOT contain any
/// done-phrase substring (case-insensitive).
#[test]
fn to_timeline_entry_unfinished_true_for_non_done_last(
content in prop::string::string_regex("[A-Za-z0-9 ]{3,40}").expect("valid regex"),
) {
// Filter out any content that happens to match a done phrase.
let lower = content.to_lowercase();
let matches_done = ["looks good", "approved", "ship it", "all good", "thanks", "done"]
.iter().any(|p| lower.contains(p));
prop_assume!(!matches_done);
prop_assume!(!content.is_empty());

let mut session = Session::new("sess-1", Corpus::Forge);
let mut msg = Message::new(Role::User, content.clone());
msg.ts_ms = Some(0);
session.messages.push(msg);
let entry = to_timeline_entry(&session);
prop_assert!(
entry.summary.unfinished,
"session with non-done last message {content:?} should be unfinished",
);
}

/// Property: `to_timeline_entry` is deterministic — applying it
/// twice to the same session yields the same entry.
#[test]
fn to_timeline_entry_is_deterministic(session in session_strategy()) {
let a = to_timeline_entry(&session);
let b = to_timeline_entry(&session);
prop_assert_eq!(a, b);
}
}

// ── history_tab::all_timeline_entries ───────────────────────────────────────

proptest! {
/// Property: output length equals input length.
#[test]
fn all_timeline_entries_length_matches(
sessions in prop::collection::vec(session_strategy(), 0..6),
) {
let entries = all_timeline_entries(&sessions);
prop_assert_eq!(entries.len(), sessions.len());
}

/// Property: the output is sorted by `total_messages` descending
/// (newest-first by message count, per the documented comment).
/// Tied entries remain in stable-sort input order.
#[test]
fn all_timeline_entries_sorted_by_message_count_desc(
sessions in prop::collection::vec(session_strategy(), 1..8),
) {
let entries = all_timeline_entries(&sessions);
for win in entries.windows(2) {
prop_assert!(
win[0].total_messages >= win[1].total_messages,
"entry {} ({} msgs) should sort before entry {} ({} msgs)",
0,
win[0].total_messages,
1,
win[1].total_messages,
);
}
}

/// Property: every session's id appears in the output exactly once.
#[test]
fn all_timeline_entries_unique_ids(
sessions in prop::collection::vec(session_strategy(), 1..6),
) {
let entries = all_timeline_entries(&sessions);
let mut ids: Vec<_> = entries.iter().map(|e| e.summary.id.clone()).collect();
ids.sort();
let mut unique = ids.clone();
unique.dedup();
prop_assert_eq!(ids.len(), unique.len(), "duplicate ids in output: {:?}", ids);
}

/// Property: `all_timeline_entries` is deterministic — applying
/// it twice to the same slice yields the same `Vec<TimelineEntry>`.
#[test]
fn all_timeline_entries_is_deterministic(
sessions in prop::collection::vec(session_strategy(), 0..6),
) {
let a = all_timeline_entries(&sessions);
let b = all_timeline_entries(&sessions);
prop_assert_eq!(a, b);
}
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win

Fix the rustfmt gate.

The supplied Trunk check reports a rustfmt failure for this new Rust file. Run trunk fmt and commit the resulting changes. Then run the required pinned-toolchain validation, including the locked build, all-features tests, Clippy, rustfmt, and cargo check -p sl-viewer.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/sl-viewer/tests/properties_viewer_history.rs` around lines 33 - 280,
Run trunk fmt on the new property-test file and commit the resulting formatting
changes. Then validate with the pinned toolchain using the locked build,
all-features tests, Clippy, rustfmt, and cargo check -p sl-viewer; resolve any
failures before completing the change.

Sources: Coding guidelines, Linters/SAST tools

Role::Tool,
Role::System,
]),
prop::string::string_regex("[ -~]{0,120}").expect("valid regex"),

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== repo files of interest =="
git ls-files | grep -E '(^|/)(properties_viewer_history\.rs|history_tab\.rs|*.rs)$' | grep -E 'crates/sl-viewer/(tests/properties_viewer_history\.rs|src/history_tab\.rs)' || true

echo
echo "== outline history_tab.rs =="
ast-grep outline crates/sl-viewer/src/history_tab.rs --view expanded || true

echo
echo "== relevant tests =="
wc -l crates/sl-viewer/tests/properties_viewer_history.rs
sed -n '1,120p' crates/sl-viewer/tests/properties_viewer_history.rs

echo
echo "== relevant history_tab.rs =="
wc -l crates/sl-viewer/src/history_tab.rs
sed -n '1,240p' crates/sl-viewer/src/history_tab.rs

echo
echo "== search message_strategy and content slicing =="
rg -n "message_strategy|\\.content\\[\\.\\.\\.97\\]|\\.content\\[\\.\\.\\.|string_regex|to_json|SessionMessage|properties" crates/sl-viewer -S

Repository: KooshaPari/SessionLedger

Length of output: 19237


🏁 Script executed:

#!/bin/bash
set -euo pipefail

python3 - <<'PY'
s = "\u00e9" * 60
byte_len = len(s.encode("utf-8"))
print(f"bytes={byte_len}")
for cut in [96, 97, 98]:
    try:
        print(f"slice ...{cut}: OK len={len(s[:cut])}")
    except Exception as e:
        print(f"slice ...{cut}: ERROR {type(e).__name__}: {e}")
PY

Repository: KooshaPari/SessionLedger

Length of output: 241


🏁 Script executed:

#!/bin/bash
set -euo pipefail

python3 - <<'PY'
s = "é" * 60
byte_len = len(s.encode("utf-8"))
print(f"bytes={byte_len}")
for cut in range(90, 106):
    try:
        print(f"slice ...{cut}: OK len={len(s[:cut])}")
    except Exception as e:
        print(f"slice ...{cut}: ERROR {type(e).__name__}: {e}")
PY

Repository: KooshaPari/SessionLedger

Length of output: 546


🏁 Script executed:

#!/bin/bash
set -euo pipefail

python3 - <<'PY'
import unicodedata

s = "".join(chr(ord("é") + i % 8) for i in range(80))
byte_len = len(s.encode("utf-8"))
print(f"bytes={byte_len}")
print(f"{s[45:55]!r}")
print([((ord(c), unicodedata.name(c, "?"))) for c in s[45:55]])
for cut in range(80, 106):
    try:
        print(f"slice ...{cut}: OK len={len(s[:cut])}")
    except Exception as e:
        print(f"slice ...{cut}: ERROR {type(e).__name__}: {e}")

# Find slices that do not end on codepoint boundaries while content is longer than 100 bytes.
bad = []
for cut in range(80, 106):
    prefix = s[:cut]
    if prefix.isascii() is False and (len(s.encode("utf-8")) > 100 and not prefix.isascii() or cut > len(s)):
        pass
print("bad slices near 100-byte check:", [cut for cut in range(80, 106) if 'slice ...' in None])
PY

Repository: KooshaPari/SessionLedger

Length of output: 1398


🏁 Script executed:

#!/bin/bash
set -euo pipefail

python3 - <<'PY'
import random
random.seed(1)
samples = []
for _ in range(10000):
    n = random.randint(1, 100)
    s = "".join(chr(0x80 + random.randint(0, 0x7F)) for _ in range(n))
    if len(s.encode("utf-8")) > 100:
        samples.append(s)
        if len(samples) == 10:
            break
print(len(samples), "samples with over-100-byte UTF-8 messages")
fails = []
for s in samples:
    if len(s.encode("utf-8")) >= 100:
        for cut in range(80, 106):
            try:
                s[:cut]
            except UnicodeDecodeError as e:
                fails.append((s.encode("utf-8").hex(), cut, str(e)))
print("finds:", len(fails))
for f in fails[:5]:
    print(f)
PY

Repository: KooshaPari/SessionLedger

Length of output: 216


🌐 Web query:

Rust String indexing panic byte boundary &str[..n] documentation

💡 Result:

In Rust, string slicing using the &s[..n] or &s[start..end] syntax is designed to operate on byte offsets rather than character positions [1][2][3]. Because Rust strings are UTF-8 encoded, characters can occupy between 1 and 4 bytes [3][4]. To ensure memory safety and maintain valid UTF-8 sequences, the Rust runtime will panic if the specified byte indices do not fall on a valid character boundary [1][5][2]. Key details regarding this behavior include: Why it panics: Rust requires that any string slice represents a valid UTF-8 string [1][3]. If an index falls inside a multi-byte character sequence, returning a slice would result in an invalid, non-UTF-8 string. To prevent this, the language enforces that slices must begin and end at valid character boundaries [5][4]. How to check for boundaries: You can use the is_char_boundary(index) method on a &str or String to verify if a specific byte index is a valid starting point for a character [1][6]. Non-panicking alternatives: - The .get() method: If you want to avoid a panic, use the get method (e.g., s.get(start..end)), which returns an Option<&str>. It returns Some(&str) if the range is valid and None if the indices are out of bounds or not on character boundaries [1][6][3]. - Finding safe boundaries: You can use floor_char_boundary(index) or ceil_char_boundary(index) to find the closest valid byte offsets to your desired position [1][6][7]. Under the hood, slicing is implemented via the SliceIndex trait, which dictates that indexing must be constant-time [1][5]. This is why byte indexing is used instead of character indexing—counting characters would require iterating through the string, which is an O(n) operation [1][8]. The indexing syntax is essentially syntactic sugar for calling the index method, which is why it panics when the safety constraints of SliceIndex are violated [5][9][10].

Citations:


🏁 Script executed:

#!/bin/bash
set -euo pipefail

rustc --version || true

tmp="$(mktemp -d)"
cat > "$tmp/slice_boundary.rs" <<'RS'
fn main() {
    let s = "é".repeat(50);
    println!("bytes={}", s.len());
    for n in 94..99 {
        std::panic::set_hook(Box::new(|_| {}));
        let r = std::panic::catch_unwind(|| {
            let trimmed = &s[..n];
            println!("safe_n={n} trimmed_byte_len={}", trimmed.len());
        });
        if r.is_err() {
            println!("panic_at_byte={n}");
        }
    }
}
RS
rustc "$tmp/slice_boundary.rs" -o "$tmp/bin"
"$tmp/bin"

Repository: KooshaPari/SessionLedger

Length of output: 505


Exercise Unicode truncation inputs.

message_strategy only generates ASCII content. to_timeline_entry truncates oversized message bodies with &m.content[..97], which panics on out-of-range UTF-8 multibyte boundaries. Add a multibyte over-limit proptest case and update the converter to truncate using a character boundary instead of a raw byte offset.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/sl-viewer/tests/properties_viewer_history.rs` at line 50, Extend
message_strategy with a Unicode multibyte input exceeding the truncation limit,
and update to_timeline_entry to truncate at a valid character boundary rather
than slicing content at byte offset 97. Preserve the existing truncation length
and behavior for ASCII and shorter messages.

Comment on lines +259 to +267
fn all_timeline_entries_unique_ids(
sessions in prop::collection::vec(session_strategy(), 1..6),
) {
let entries = all_timeline_entries(&sessions);
let mut ids: Vec<_> = entries.iter().map(|e| e.summary.id.clone()).collect();
ids.sort();
let mut unique = ids.clone();
unique.dedup();
prop_assert_eq!(ids.len(), unique.len(), "duplicate ids in output: {:?}", ids);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Define the duplicate-ID contract consistently.

The generator permits duplicate input IDs. all_timeline_entries produces one entry for each input session and does not deduplicate IDs.

  • crates/sl-viewer/tests/properties_viewer_history.rs#L259-L267: Sort and compare input and output ID vectors to verify ID-multiset preservation.
  • CHANGELOG.md#L25-L25: Replace the unique-ID claim with one output entry per input session.
📍 Affects 2 files
  • crates/sl-viewer/tests/properties_viewer_history.rs#L259-L267 (this comment)
  • CHANGELOG.md#L25-L25
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/sl-viewer/tests/properties_viewer_history.rs` around lines 259 - 267,
Update all_timeline_entries_unique_ids in
crates/sl-viewer/tests/properties_viewer_history.rs:259-267 to collect, sort,
and compare the input session ID multiset with the output entry ID multiset,
preserving duplicate IDs rather than deduplicating them. Update CHANGELOG.md:25
to replace the unique-ID claim with the contract that all_timeline_entries emits
one output entry per input session.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:L This PR changes 100-499 lines, ignoring generated files

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants