test(viewer): session-ledger worklog projector proptest surface (WBS-6.2 #460) - #476
test(viewer): session-ledger worklog projector proptest surface (WBS-6.2 #460)#476KooshaPari wants to merge 3 commits into
Conversation
) Adds crates/sl-viewer/tests/properties_viewer_async_states.rs with 7 proptest properties pinning the async_states SSOT: * SkeletonLayout::default() is Bundles. * SkeletonLayout exposes exactly three variants (Bundles, ListDetail, StreamFeed). * Every variant's Debug label is non-empty, single-line, and matches one of the documented names. * SkeletonLayout::default() matches the first arm in the match block in ContentSkeleton. * list_rows.clamp(3, 6) lands in [3, 6] for every input. * The clamp is monotonic non-decreasing. * The clamp has the documented fixed points (0/2 -> 3, 6/MAX -> 6). Updates WBS-6.2 evidence list, TRACEABILITY.json, and CHANGELOG.
…(WBS-6.2 #459) Adds crates/sl-viewer/tests/properties_session_ledger_okf.rs with 12 proptest properties pinning the session-ledger OKF SSOT: * OkfDocument::new(b, c) always produces okf = "1.0". * OkfDocument::new(b, c) propagates bundle.source_id into source_id and provenance.source_id. * OkfDocument::new(b, c) propagates c into provenance.corpus. * OkfDocument::new(b, c) starts with empty entities, relations, tags. * validate_okf_document reports exactly one unsupported_version error per non-"1.0" okf (with offending version in message). * validate_okf_document reports exactly one source_id_mismatch error per provenance/source mismatch. * Duplicate entity ids each surface a duplicate_entity_id error. * Dangling relation source / target surface their respective errors. * Every OkfValidationError carries non-empty field / code / message. First property test to exercise session_ledger (the core domain crate) from sl-viewer's test harness, pivoting the bounded lane beyond the viewer-only surface. Updates WBS-6.2 evidence list, TRACEABILITY.json, and CHANGELOG.
…6.2 #460) Adds crates/sl-viewer/tests/properties_session_ledger_worklog.rs with 11 proptest properties pinning the session-ledger worklog projector (crash-recovery / lost-work pipeline): * Empty sessions project None. * Final Role::User turn -> AwaitingAssistantResponse. * Final Role::Tool / Role::Subagent -> InterruptedExecution. * Final assistant turn with one of the 9 documented completion markers (complete / completed / done / [completed] / <completed> / status: complete / status: completed / task complete / task completed) projects None. * Final assistant turn without any marker projects as MissingCompletionMarker. * UnfinishedWorkItem carries the originating session id, corpus, and message_count. * summary never exceeds 241 chars and is single-line. * project_unfinished_work returns one item per unfinished session in input order and is deterministic. * WorklogProjection::from_session carries message_count and matches detect_unfinished exactly. 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 · |
codeant-ai
Bot
added
the
size:XL
📝 WalkthroughSummaryAdds property-based tests for viewer async states, Session Ledger OKF validation, and worklog projection. The tests cover:
Updates No public API changes are introduced. Must FixNone identified. Should FixNone identified. ConsiderThe targeted test suite and formatting check passed. Run the full workspace checks before merge if they are not covered by CI: Approve / Request ChangesApprove. WalkthroughThis change adds property-based tests for viewer async states, OKF document validation, and session worklog projection. It also updates the changelog and WBS-6.2 traceability evidence. ChangesProperty-Test Coverage
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
📝 Generate docstrings
🧪 Generate unit tests (beta)
✨ Simplify code
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 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_session_ledger_okf.rs`:
- Around line 240-260: Update every_error_has_nonempty_components to add a
relation whose source and target IDs are absent from document.entities, ensuring
validation produces both dangling_relation_source and dangling_relation_target
alongside the existing error classes. Keep the fixture’s all-error-shape intent
and existing entity setup unchanged.
In `@crates/sl-viewer/tests/properties_session_ledger_worklog.rs`:
- Around line 11-12: Extend the role-to-reason assertions in the session ledger
worklog tests with a final Role::System case, asserting that it maps to
UnfinishedReason::MissingCompletionMarker. Keep the existing User, Assistant,
Tool, and Subagent cases unchanged.
- Around line 225-231: Update the property test around
WorklogProjection::from_session and detect_unfinished so that when
detect_unfinished returns Some(item), it asserts projection.unfinished equals a
single-item vector containing that exact item, rather than comparing only
lengths. Preserve the existing empty assertion for the None case.
- Around line 99-107: Update the
assistant_without_completion_marker_is_unfinished property so its generated body
cannot equal or contain documented completion markers such as “complete” or
“completed”; use a fixed non-completion prefix with a generated numeric suffix,
while preserving the existing detect_unfinished assertion.
- Around line 140-150: Update the summary_is_single_line property to construct
body from generated prefix and suffix strings with a forced embedded newline
between them, rather than relying on ".{1,200}". Preserve a forced tab in the
generated input as well if tab normalization remains part of the assertions,
while retaining the existing summary checks.
In `@crates/sl-viewer/tests/properties_viewer_async_states.rs`:
- Around line 31-60: Update skeleton_layout_labels_documented and related
coverage to use a production-owned source of SkeletonLayout variants or labels
instead of test-local arrays and select values. Add or reuse a production API or
derive-based metadata that enumerates every variant, then assert each
production-provided variant has the expected stable, non-empty, single-line
label; ensure newly added variants are automatically covered.
- Around line 74-102: Update the tests around list_rows_clamp_in_range,
list_rows_clamp_monotonic, and list_rows_clamp_fixed_points to exercise
ContentSkeleton’s production rendering path rather than usize::clamp directly.
Cover list_rows values below 3, within 3–6, and above 6, asserting the rendered
layout uses the clamped row count; alternatively extract a shared clamp_rows
helper from ContentSkeleton into async_states.rs and test that helper while
ensuring production code calls it.
🪄 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: b1355c5c-71d7-4701-9c4c-901f73163f12
📒 Files selected for processing (6)
CHANGELOG.mdcrates/sl-viewer/tests/properties_session_ledger_okf.rscrates/sl-viewer/tests/properties_session_ledger_worklog.rscrates/sl-viewer/tests/properties_viewer_async_states.rsdocs/ops/TRACEABILITY.jsondocs/ops/WBS.md
📜 Review details
⏰ Context from checks skipped due to timeout. (2)
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: Summary
⚠️ CI failures not shown inline (2)
GitHub Check: Summary: The current Mergify configuration is invalid
Conclusion: failure
* 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>=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
* 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>=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 fromorigin/mainon a branch named<type>/<topic>, rather than working directly onmain.
Do not make direct commits to protectedmain; use a pull request.
Do not usegit reset --hard,git stash, orgit cleanin worktrees.
Do not use--no-verifyor 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 inrust-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_session_ledger_worklog.rscrates/sl-viewer/tests/properties_viewer_async_states.rscrates/sl-viewer/tests/properties_session_ledger_okf.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_session_ledger_worklog.rscrates/sl-viewer/tests/properties_viewer_async_states.rscrates/sl-viewer/tests/properties_session_ledger_okf.rs
crates/sl-viewer/**/*.{rs,toml}
📄 CodeRabbit inference engine (AGENTS.md)
crates/sl-viewer/**/*.{rs,toml}: Thesl-viewercrate uses Dioxus 0.6; use the Dioxus CLI/toolchain for desktop development and bundling.
Usecargo check -p sl-vieweras the fast inner-loop check for viewer changes.
Files:
crates/sl-viewer/tests/properties_session_ledger_worklog.rscrates/sl-viewer/tests/properties_viewer_async_states.rscrates/sl-viewer/tests/properties_session_ledger_okf.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_session_ledger_worklog.rscrates/sl-viewer/tests/properties_viewer_async_states.rscrates/sl-viewer/tests/properties_session_ledger_okf.rs
🪛 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)
🔇 Additional comments (6)
CHANGELOG.md (1)
60-65: LGTM!docs/ops/TRACEABILITY.json (1)
328-330: LGTM!docs/ops/WBS.md (1)
32-32: LGTM!crates/sl-viewer/tests/properties_session_ledger_okf.rs (1)
27-31: 📐 Maintainability & Code QualityRun the required Rust validation.
The recorded validation includes targeted tests and rustfmt only. Before merge, run the pinned toolchain with a locked
cargo check -p sl-viewer, locked all-features tests, and locked Clippy checks.As per coding guidelines, “Validate Rust workspace changes with the prescribed locked build, all-features test suite, Clippy, and rustfmt checks where applicable.”
Source: Coding guidelines
crates/sl-viewer/tests/properties_viewer_async_states.rs (1)
18-103: 📐 Maintainability & Code QualityRun the required Rust validation.
The recorded validation includes targeted tests and rustfmt only. Before merge, run the repository-prescribed locked build, workspace all-features tests, Clippy, rustfmt, and
cargo check -p sl-viewerwith the toolchain pinned byrust-toolchain.toml.As per coding guidelines, “Validate Rust workspace changes with the prescribed locked build, all-features test suite, Clippy, and rustfmt checks where applicable,” and “Use
cargo check -p sl-vieweras the fast inner-loop check for viewer changes.”Source: Coding guidelines
crates/sl-viewer/tests/properties_session_ledger_worklog.rs (1)
14-18: 📐 Maintainability & Code QualityRun the required Rust workspace validation commands.
The
rust-toolchain.toml-pinnedcargo check, all-features suite, Clippy, andcargo fmt --all --checkresults are not available. Run those checks again until all pass before merging.
| fn every_error_has_nonempty_components(_seed in any::<u32>()) { | ||
| let bundle = ContinuationBundle::new("err-shape"); | ||
| let mut document = OkfDocument::new(&bundle, "forge"); | ||
| // Force every error class at once. | ||
| document.okf = "2.0".into(); | ||
| document.provenance.source_id = "other".into(); | ||
| document.entities = vec![ | ||
| OkfEntity { | ||
| id: "x".into(), | ||
| r#type: "intent".into(), | ||
| label: "x".into(), | ||
| properties: serde_json::Value::Null, | ||
| }, | ||
| OkfEntity { | ||
| id: "x".into(), | ||
| r#type: "intent".into(), | ||
| label: "x".into(), | ||
| properties: serde_json::Value::Null, | ||
| }, | ||
| ]; | ||
| let errors = validate_okf_document(&document); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Add dangling relations to the all-error-shape fixture.
The fixture only produces unsupported_version, source_id_mismatch, and duplicate_entity_id. It does not produce dangling_relation_source or dangling_relation_target, despite Line 243 claiming that it forces every error class.
Add a relation whose source and target are absent from document.entities. This makes the non-empty component check cover all validator error classes.
Proposed fix
document.entities = vec![
OkfEntity {
id: "x".into(),
@@
properties: serde_json::Value::Null,
},
];
+ document.relations = vec![OkfRelation {
+ source: "missing-source".into(),
+ target: "missing-target".into(),
+ r#type: "grounds".into(),
+ provenance: document.provenance.clone(),
+ }];
let errors = validate_okf_document(&document);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| fn every_error_has_nonempty_components(_seed in any::<u32>()) { | |
| let bundle = ContinuationBundle::new("err-shape"); | |
| let mut document = OkfDocument::new(&bundle, "forge"); | |
| // Force every error class at once. | |
| document.okf = "2.0".into(); | |
| document.provenance.source_id = "other".into(); | |
| document.entities = vec![ | |
| OkfEntity { | |
| id: "x".into(), | |
| r#type: "intent".into(), | |
| label: "x".into(), | |
| properties: serde_json::Value::Null, | |
| }, | |
| OkfEntity { | |
| id: "x".into(), | |
| r#type: "intent".into(), | |
| label: "x".into(), | |
| properties: serde_json::Value::Null, | |
| }, | |
| ]; | |
| let errors = validate_okf_document(&document); | |
| fn every_error_has_nonempty_components(_seed in any::<u32>()) { | |
| let bundle = ContinuationBundle::new("err-shape"); | |
| let mut document = OkfDocument::new(&bundle, "forge"); | |
| // Force every error class at once. | |
| document.okf = "2.0".into(); | |
| document.provenance.source_id = "other".into(); | |
| document.entities = vec![ | |
| OkfEntity { | |
| id: "x".into(), | |
| r#type: "intent".into(), | |
| label: "x".into(), | |
| properties: serde_json::Value::Null, | |
| }, | |
| OkfEntity { | |
| id: "x".into(), | |
| r#type: "intent".into(), | |
| label: "x".into(), | |
| properties: serde_json::Value::Null, | |
| }, | |
| ]; | |
| document.relations = vec![OkfRelation { | |
| source: "missing-source".into(), | |
| target: "missing-target".into(), | |
| r#type: "grounds".into(), | |
| provenance: document.provenance.clone(), | |
| }]; | |
| let errors = validate_okf_document(&document); |
🤖 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_session_ledger_okf.rs` around lines 240 -
260, Update every_error_has_nonempty_components to add a relation whose source
and target IDs are absent from document.entities, ensuring validation produces
both dangling_relation_source and dangling_relation_target alongside the
existing error classes. Keep the fixture’s all-error-shape intent and existing
entity setup unchanged.
| //! Every `Role` ↔ `UnfinishedReason` mapping, the completion-marker | ||
| //! whitelist, and the `summarize` budget are pinned here. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Add a Role::System reason assertion.
The file states that every Role to UnfinishedReason mapping is pinned. The tests cover User, Assistant, Tool, and Subagent, but not System. The projection test does not cover this gap because it compares WorklogProjection::from_session with detect_unfinished, not with the expected reason.
Add a final Role::System case that asserts UnfinishedReason::MissingCompletionMarker.
Also applies to: 64-78
🤖 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_session_ledger_worklog.rs` around lines 11
- 12, Extend the role-to-reason assertions in the session ledger worklog tests
with a final Role::System case, asserting that it maps to
UnfinishedReason::MissingCompletionMarker. Keep the existing User, Assistant,
Tool, and Subagent cases unchanged.
| fn assistant_without_completion_marker_is_unfinished( | ||
| body in "[a-zA-Z0-9 .,!?]{1,40}", | ||
| ) { | ||
| let mut session = Session::new("a", Corpus::Forge); | ||
| session.messages = vec![ | ||
| Message::new(Role::User, "do it"), | ||
| Message::new(Role::Assistant, body), | ||
| ]; | ||
| let item = detect_unfinished(&session).expect("missing marker is unfinished"); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Restrict this property to non-completion content.
body can be "complete" or "completed". Those are documented completion markers. For those inputs, detect_unfinished returns None, and expect("missing marker is unfinished") fails.
Generate content that cannot contain a completion marker, or use a fixed non-completion prefix and a generated numeric suffix.
🤖 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_session_ledger_worklog.rs` around lines 99
- 107, Update the assistant_without_completion_marker_is_unfinished property so
its generated body cannot equal or contain documented completion markers such as
“complete” or “completed”; use a fixed non-completion prefix with a generated
numeric suffix, while preserving the existing detect_unfinished assertion.
| /// The summary never carries embedded newlines or tab characters | ||
| /// (the content was whitespace-normalized). | ||
| #[test] | ||
| fn summary_is_single_line( | ||
| body in ".{1,200}", | ||
| ) { | ||
| let mut session = Session::new("s", Corpus::Forge); | ||
| session.messages = vec![Message::new(Role::User, body)]; | ||
| let item = detect_unfinished(&session).expect("user work is unfinished"); | ||
| prop_assert!(!item.summary.contains('\n')); | ||
| prop_assert!(!item.summary.contains('\t')); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Generate an embedded newline in this property.
The Rust regex .{1,200} does not generate \n. This property therefore does not test its stated newline-normalization condition.
Build body from generated prefix and suffix values with a forced \n between them. Keep a forced \t if the test must also verify tab normalization.
🤖 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_session_ledger_worklog.rs` around lines 140
- 150, Update the summary_is_single_line property to construct body from
generated prefix and suffix strings with a forced embedded newline between them,
rather than relying on ".{1,200}". Preserve a forced tab in the generated input
as well if tab normalization remains part of the assertions, while retaining the
existing summary checks.
| let projection = WorklogProjection::from_session(&session); | ||
| prop_assert_eq!(projection.message_count, n); | ||
| let detected = detect_unfinished(&session); | ||
| match detected { | ||
| Some(_) => prop_assert_eq!(projection.unfinished.len(), 1), | ||
| None => prop_assert!(projection.unfinished.is_empty()), | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Compare the projected item with detect_unfinished.
This test only compares the number of unfinished items. A regression that preserves the count but changes session_id, summary, reason, metadata, or activity time still passes.
When detect_unfinished returns Some(item), assert that projection.unfinished == vec![item].
🤖 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_session_ledger_worklog.rs` around lines 225
- 231, Update the property test around WorklogProjection::from_session and
detect_unfinished so that when detect_unfinished returns Some(item), it asserts
projection.unfinished equals a single-item vector containing that exact item,
rather than comparing only lengths. Preserve the existing empty assertion for
the None case.
| fn skeleton_layout_has_three_variants(_seed in any::<u32>()) { | ||
| let variants = [ | ||
| SkeletonLayout::Bundles, | ||
| SkeletonLayout::ListDetail, | ||
| SkeletonLayout::StreamFeed, | ||
| ]; | ||
| // Round-trip through Debug to confirm each variant's name | ||
| // survives stable serialisation. | ||
| let mut seen = std::collections::HashSet::new(); | ||
| for v in variants { | ||
| let name = format!("{v:?}"); | ||
| prop_assert!(name.is_ascii(), "variant {name:?} is not ASCII"); | ||
| seen.insert(name); | ||
| } | ||
| prop_assert_eq!(seen.len(), 3, "variant count drifted"); | ||
| } | ||
|
|
||
| /// Every variant's Debug label is non-empty, single-line, and | ||
| /// matches one of the documented variant names. | ||
| #[test] | ||
| fn skeleton_layout_labels_documented(variant in prop::sample::select(vec![ | ||
| SkeletonLayout::Bundles, | ||
| SkeletonLayout::ListDetail, | ||
| SkeletonLayout::StreamFeed, | ||
| ])) { | ||
| let label = format!("{variant:?}"); | ||
| prop_assert!(!label.is_empty()); | ||
| prop_assert!(!label.contains('\n')); | ||
| let valid = label == "Bundles" || label == "ListDetail" || label == "StreamFeed"; | ||
| prop_assert!(valid, "label {label:?} is not a documented variant name"); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Find relevant files"
git ls-files | rg '(^|/)properties_viewer_async_states\.rs$|SkeletonLayout|content.*skeleton|async_states\.rs$' || true
echo
echo "Target test file excerpt"
if [ -f crates/sl-viewer/tests/properties_viewer_async_states.rs ]; then
nl -ba crates/sl-viewer/tests/properties_viewer_async_states.rs | sed -n '1,120p'
fi
echo
echo "Search SkeletonLayout definitions/usages"
rg -n "enum SkeletonLayout|struct ContentSkeleton|SkeletonLayout::|content_rows|list_rows|clamp\\(" crates/sl-viewer -S || true
echo
echo "Module/imports context"
rg -n "use .*SkeletonLayout|SkeletonLayout|AsyncState|ContentSkeleton" crates/sl-viewer/tests crates/sl-viewer/src -S || trueRepository: KooshaPari/SessionLedger
Length of output: 387
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Target test file excerpt"
if [ -f crates/sl-viewer/tests/properties_viewer_async_states.rs ]; then
awk '{printf "%6d\t%s\n", NR, $0}' crates/sl-viewer/tests/properties_viewer_async_states.rs | sed -n '1,130p'
fi
echo
echo "search SkeletonLayout definitions and async_states"
grep -RInE "enum SkeletonLayout|struct ContentSkeleton|SkeletonLayout::|content_rows|list_rows|clamp\\(|derive\\(.*Serialize|derive\\(.*Deserialize|Display|Serialize|Deserialize" crates/sl-viewer -S || true
echo
echo "read async_states relevant lines"
awk '{printf "%6d\t%s\n", NR, $0}' crates/sl-viewer/src/async_states.rs | sed -n '1,140p'Repository: KooshaPari/SessionLedger
Length of output: 11449
Use production-owned variant coverage instead of a test-local list.
The skeleton_layout_labels_documented test still enumerates SkeletonLayout::Bundles, ListDetail, and StreamFeed, so a newly added enum variant can be missed. For stable serialisation or UI labels, define a production API or derive attribute that produces them, and assert coverage from that production source.
🤖 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_async_states.rs` around lines 31 -
60, Update skeleton_layout_labels_documented and related coverage to use a
production-owned source of SkeletonLayout variants or labels instead of
test-local arrays and select values. Add or reuse a production API or
derive-based metadata that enumerates every variant, then assert each
production-provided variant has the expected stable, non-empty, single-line
label; ensure newly added variants are automatically covered.
| /// `list_rows.clamp(3, 6)` lands in `[3, 6]` for every input. | ||
| #[test] | ||
| fn list_rows_clamp_in_range(input in any::<usize>()) { | ||
| let clamped = input.clamp(3, 6); | ||
| prop_assert!((3..=6).contains(&clamped), "clamp produced {clamped} for input {input}"); | ||
| } | ||
|
|
||
| /// The clamp is monotonic non-decreasing. | ||
| #[test] | ||
| fn list_rows_clamp_monotonic( | ||
| a in any::<usize>(), | ||
| b in any::<usize>(), | ||
| ) { | ||
| let (lo, hi) = if a <= b { (a, b) } else { (b, a) }; | ||
| let c_lo = lo.clamp(3, 6); | ||
| let c_hi = hi.clamp(3, 6); | ||
| prop_assert!(c_lo <= c_hi, "clamp not monotonic: {lo}→{c_lo}, {hi}→{c_hi}"); | ||
| } | ||
|
|
||
| /// The clamp has the documented fixed points: `0` and `2` clamp | ||
| /// to `3`; `6` and `u64::MAX` clamp to `6`. | ||
| #[test] | ||
| fn list_rows_clamp_fixed_points(_seed in any::<u32>()) { | ||
| prop_assert_eq!(0_usize.clamp(3, 6), 3); | ||
| prop_assert_eq!(2_usize.clamp(3, 6), 3); | ||
| prop_assert_eq!(3_usize.clamp(3, 6), 3); | ||
| prop_assert_eq!(6_usize.clamp(3, 6), 6); | ||
| prop_assert_eq!(usize::MAX.clamp(3, 6), 6); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== files =="
git ls-files | rg '(^|/)crates/sl-viewer/(tests/properties_viewer_async_states\.rs|src/async_states\.rs)$' || true
echo "== test file relevant section =="
if [ -f crates/sl-viewer/tests/properties_viewer_async_states.rs ]; then
nl -ba crates/sl-viewer/tests/properties_viewer_async_states.rs | sed -n '1,140p'
fi
echo "== async_states relevant section =="
if [ -f crates/sl-viewer/src/async_states.rs ]; then
nl -ba crates/sl-viewer/src/async_states.rs | sed -n '1,140p'
fi
echo "== search ContentSkeleton and list_rows clamp =="
rg -n "ContentSkeleton|list_rows|clamp\(" crates/sl-viewer -SRepository: KooshaPari/SessionLedger
Length of output: 341
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== test file relevant section =="
if [ -f crates/sl-viewer/tests/properties_viewer_async_states.rs ]; then
awk '{printf "%6d\t%s\n", NR, $0}' crates/sl-viewer/tests/properties_viewer_async_states.rs | sed -n '1,140p'
fi
echo "== async_states relevant section =="
if [ -f crates/sl-viewer/src/async_states.rs ]; then
awk '{printf "%6d\t%s\n", NR, $0}' crates/shown_states.rs 2>/dev/null || awk '{printf "%6d\t%s\n", NR, $0}' crates/sl-viewer/src/async_states.rs | sed -n '1,140p'
fi
echo "== search ContentSkeleton and list_rows clamp =="
rg -n "ContentSkeleton|list_rows|clamp\\(" crates/sl-viewer -S || true
echo "== deterministic usize clamp facts =="
python3 - <<'PY'
def rust_usize_clamp(n, lo, hi):
return max(lo, min(hi, n))
for n in [0, 2, 3, 5, 6, 7, 17, 2**64-1 if False else 2**32-1]:
print(n, rust_usize_clamp(n, 3, 6))
PYRepository: KooshaPari/SessionLedger
Length of output: 14708
Test ContentSkeleton clamping through the production path.
The documented crate helper clamp_rows() is not present, and the current tests prove usize::clamp(3, 6) directly. A change inside ContentSkeleton can affect production output without failing these tests.
Cover rendered layouts with list_rows values above and below the [3, 6] bounds, or extract the production clamp helper to crates/sl-viewer/src/async_states.rs and call it from these tests.
🤖 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_async_states.rs` around lines 74 -
102, Update the tests around list_rows_clamp_in_range,
list_rows_clamp_monotonic, and list_rows_clamp_fixed_points to exercise
ContentSkeleton’s production rendering path rather than usize::clamp directly.
Cover list_rows values below 3, within 3–6, and above 6, asserting the rendered
layout uses the clamped row count; alternatively extract a shared clamp_rows
helper from ContentSkeleton into async_states.rs and test that helper while
ensuring production code calls it.
) Shards the unique viewer proptest from #496 onto main without the obsolete pre-rename daemon source churn.
User description
Summary
Adds
crates/sl-viewer/tests/properties_session_ledger_worklog.rswith 11 proptest properties pinningsession_ledger::detect_unfinished,project_unfinished_work, andWorklogProjection::from_session— the crash-recovery / lost-work projector (WBS-6.2 #460).detect_unfinished(8 properties)None.Role::User→AwaitingAssistantResponse.Role::Tool/Role::Subagent→InterruptedExecution.None.MissingCompletionMarker.UnfinishedWorkItemcarries originating session id, corpus, message count.summaryis bounded ≤ 241 chars, single-line.project_unfinished_work(2 properties)WorklogProjection::from_session(1 property)message_countand matchesdetect_unfinishedexactly.Validation
cargo test -p sl-viewer --test properties_session_ledger_worklog --features "desktop parquet" --locked— 11 passedcargo fmt --all --check— cleanWBS / TRACEABILITY
WBS-6.2 evidence list and
TRACEABILITY.jsongaincrates/sl-viewer/tests/properties_session_ledger_worklog.rs. CHANGELOG Unreleased documents the new surface.CodeAnt-AI Description
Add property coverage for viewer loading states and session-ledger validation
What Changed
Impact
✅ Consistent viewer loading states✅ Clearer export validation errors✅ More reliable unfinished-work recovery💡 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.