test(viewer): session-ledger heuristic intent extractor proptest surface (WBS-6.2 #463) - #479
Merged
Merged
Conversation
added 6 commits
August 9, 2026 19:37
) 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.
…-6.2 #461) Adds crates/sl-viewer/tests/properties_session_ledger_export.rs with 8 proptest properties pinning session_ledger::export_to_okf (the OKF v1 export pipeline entry point): * export_to_okf always produces okf = "1.0" and propagates bundle.source_id into source_id + provenance.source_id. * export_to_okf propagates the corpus arg into provenance.corpus. * Empty bundles yield zero entities / relations / tags. * Every exported document passes validate_okf_document. * export_to_okf is deterministic across calls. * Intent bundles emit exactly one goal entity (label = goal), one acceptance entity per acceptance signal, one constraint entity per constraint. * Context bundles emit exactly one resource entity when cwd is present. * Acceptance bundles emit exactly one gate entity with label = "resume-gate" and properties.ready = true / scope_sized = true. * The exporter never produces duplicate entity ids across mixed intent / context / acceptance / contract bundles. Updates WBS-6.2 evidence list, TRACEABILITY.json, and CHANGELOG.
#462) Adds crates/sl-viewer/tests/properties_session_ledger_distill.rs with 9 proptest properties pinning session_ledger::distill::compile and compile_and_store: * compile(session) always produces a bundle whose source_id equals session.id. * compile(session) always produces an injectable bundle (carries an Acceptance slice) — the load-bearing contract for resume. * compile(session) always emits one slice for every documented kind (Acceptance / Intent / Context / Contract / Provenance / Worklog) — even when the session is empty. * compile(session) always returns a bundle whose total_token_estimate() equals the sum of per-slice token_estimate values. * compile(session) is deterministic across calls. * The Worklog slice body deserializes to a WorklogProjection whose message_count equals session.messages.len(). * compile_and_store returns an injectable bundle with the input source_id. * compile_and_store writes exactly 3 episodic memories (intent / contract / context) to the memory store. * compile_and_store is deterministic across fresh stores. Updates WBS-6.2 evidence list, TRACEABILITY.json, and CHANGELOG.
…ace (WBS-6.2 #463) Adds crates/sl-viewer/tests/properties_session_ledger_intent.rs with 10 proptest properties pinning session_ledger::distill::extractor::HeuristicIntentExtractor::extract_intent (the P1 SSOT for what the user wants — drives resume prompt, search index, and wiki/docs view): * user_turn_count always equals the count of Role::User messages and ignores Assistant / Subagent / Tool / System messages. * Empty sessions produce an empty Intent. * Repeated acceptance / constraint patterns are deduplicated. * Every documented acceptance pattern (16) and every documented constraint pattern (19) is recognized in any user message. * Labeled Goal: / Objective: / Task: lines win over preamble. * Labeled Constraint: / Requirement: / Boundary: lines carry their full text. * extract_intent is deterministic across calls. 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 · |
📝 WalkthroughSummaryThis PR adds property-based tests for viewer async states and session-ledger behavior. The tests cover intent extraction, worklog projection, distillation, OKF document construction and validation, and OKF export. The PR also updates WBS-6.2 traceability and adds Unreleased changelog entries. The stated focused test and formatting checks pass. Must FixNone identified. Should FixNone identified. ConsiderThe PR adds substantial test coverage without changing public APIs or production behavior. Confirm that the full workspace checks also pass before merge:
Approve / Request ChangesApprove. WalkthroughThe PR adds property-based tests for viewer async states and SessionLedger intent, worklog, OKF, export, and distillation behavior. It also updates the changelog and WBS-6.2 traceability evidence. ChangesWBS-6.2 Property-Test Coverage
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 2📝 Generate docstrings 💡
🛠️ Fix failing CI checks 💡
🧪 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 |
codeant-ai
Bot
added
the
size:XXL
![codeant-ai[bot]](https://avatars.githubusercontent.com/in/646884?s=60&v=4){kind=small}
Comment on lines
+183
to
+184
| prop_assert_eq!(out_a.bundle, out_b.bundle); | ||
| prop_assert_eq!(out_a.memories.len(), out_b.memories.len()); |
There was a problem hiding this comment.
Suggestion: The determinism property compares bundles and only the number of memories, not the memory records themselves. Changes to memory IDs, keys, kinds, ordering, or stored content could therefore make compile_and_store nondeterministic while this test still passes; compare the complete memories values and store contents. [logic error]
Severity Level: Major ⚠️
- ⚠️ Memory identifiers and keys can regress undetected.
- ⚠️ Episodic-memory ordering/content is not covered.
- ❌ Determinism evidence can approve inconsistent stored facts.
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** crates/sl-viewer/tests/properties_session_ledger_distill.rs
**Line:** 183:184
**Comment:**
*Logic Error: The determinism property compares bundles and only the number of memories, not the memory records themselves. Changes to memory IDs, keys, kinds, ordering, or stored content could therefore make `compile_and_store` nondeterministic while this test still passes; compare the complete `memories` values and store contents.
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
Comment on lines
+134
to
+138
| let matching: Vec<&OkfValidationError> = errors | ||
| .iter() | ||
| .filter(|e| e.code == "unsupported_version" && e.field == "okf") | ||
| .collect(); | ||
| prop_assert_eq!(matching.len(), 1); |
There was a problem hiding this comment.
Suggestion: The property claims that an invalid version produces exactly one validation error, but it only counts matching errors and ignores every other error in the returned vector. Additional diagnostics or an unrelated regression would therefore still satisfy the assertion; assert the total length and the complete expected error shape. [logic error]
Severity Level: Major ⚠️
- ⚠️ Unexpected OKF diagnostics can remain undetected.
- ⚠️ Consumers may receive changed validation error shapes.
- ⚠️ Version-validation contract is only partially pinned.
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** crates/sl-viewer/tests/properties_session_ledger_okf.rs
**Line:** 134:138
**Comment:**
*Logic Error: The property claims that an invalid version produces exactly one validation error, but it only counts matching errors and ignores every other error in the returned vector. Additional diagnostics or an unrelated regression would therefore still satisfy the assertion; assert the total length and the complete expected error shape.
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
Comment on lines
+208
to
+210
| let errors = validate_okf_document(&document); | ||
| prop_assert!(errors.iter().any(|e| e.code == "dangling_relation_source" | ||
| && e.field == "relations[0].source")); |
There was a problem hiding this comment.
Suggestion: The dangling-relation properties only require that an expected error appears somewhere. They do not reject extra diagnostics or verify that the returned error list has the expected exact shape, so a validator emitting both endpoint errors or unrelated errors could pass these tests. [logic error]
Severity Level: Major ⚠️
- ⚠️ Extra relation diagnostics can go unnoticed.
- ⚠️ OKF error ordering and shape remain weakly tested.
- ⚠️ Downstream diagnostic rendering may regress silently.
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** crates/sl-viewer/tests/properties_session_ledger_okf.rs
**Line:** 208:210
**Comment:**
*Logic Error: The dangling-relation properties only require that an expected error appears somewhere. They do not reject extra diagnostics or verify that the returned error list has the expected exact shape, so a validator emitting both endpoint errors or unrelated errors could pass these tests.
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[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🤖 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_distill.rs`:
- Around line 181-184: The property around compile_and_store must compare
generated memory values, not only memories.len(); compare the ordered records or
their stable identifiers and content fields between out_a.memories and
out_b.memories. In CHANGELOG.md lines 68-68, retain the deterministic
fresh-store claim after the property validates memory equivalence.
In `@crates/sl-viewer/tests/properties_session_ledger_export.rs`:
- Around line 128-161: Update intent_bundle_emits_goal_entity to generate at
least two Intent bundles with distinct goals and add each to the
ContinuationBundle before export. Assert the exported intent entity labels
include every generated goal, while retaining acceptance and constraint count
assertions against the combined signals and constraints.
- Around line 183-211: Update the acceptance_bundle_emits_gate_entity property
test to generate ready and scope_sized as any::<bool>() values, include those
values in the Acceptance payload, and assert the exported gate properties match
the generated values instead of always asserting true. Keep the existing gate
count and fixed label assertions unchanged.
In `@crates/sl-viewer/tests/properties_session_ledger_intent.rs`:
- Around line 151-152: Update both acceptance and constraint assertions in
crates/sl-viewer/tests/properties_session_ledger_intent.rs at lines 151-152 and
166-167 to count entries equal to pat and assert the count is exactly one,
replacing the current contains checks so duplicates fail the properties.
In `@crates/sl-viewer/tests/properties_session_ledger_okf.rs`:
- Around line 240-259: Add a relation with nonexistent source and target entity
IDs to the document setup in every_error_has_nonempty_components before
validation, so the existing diagnostic loop also exercises
dangling_relation_source and dangling_relation_target while preserving the
current error-shape assertions.
In `@crates/sl-viewer/tests/properties_session_ledger_worklog.rs`:
- Around line 208-231: Update the `worklog_projection_matches_detect` property
test to compare `projection.unfinished` against the item returned by
`detect_unfinished`, rather than asserting only its length. Preserve the
existing empty-result assertion when detection returns `None`, and assert the
projected collection contains the exact detected unfinished item when it returns
`Some`.
- Around line 99-108: Update assistant_without_completion_marker_is_unfinished
to constrain the generated body so it cannot equal any value in
COMPLETION_MARKERS, while preserving the existing assertion for non-marker
bodies. Reuse the existing completion-marker definition or filtering approach
rather than changing detect_unfinished.
In `@crates/sl-viewer/tests/properties_viewer_async_states.rs`:
- Around line 31-71: Update the tests around skeleton_layout_has_three_variants,
skeleton_layout_default_is_first_arm, and the related clamp properties to
exercise production behavior rather than duplicated local matches and constants.
Move the list-row clamp into a production-owned ContentSkeleton helper with an
internal unit test, or render ContentSkeleton and assert its rendered row count;
ensure enum coverage and default behavior are likewise derived from the
production implementation so new variants or changed bounds cannot leave tests
passing.
In `@docs/ops/WBS.md`:
- Line 32: Update the WBS-6.2 documentation entry to replace the misspelled
“hmetic-pin cleanups” phrase with “hermetic-pin cleanups,” preserving the
surrounding milestone references and wording.
🪄 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: 50214487-a329-4832-86eb-e03b7b35412b
📒 Files selected for processing (9)
CHANGELOG.mdcrates/sl-viewer/tests/properties_session_ledger_distill.rscrates/sl-viewer/tests/properties_session_ledger_export.rscrates/sl-viewer/tests/properties_session_ledger_intent.rscrates/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. (8)
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: race smoke + channel/cancel model · macos-latest
- GitHub Check: race smoke + channel/cancel model · windows-latest
- GitHub Check: session-ledger build · windows-latest
- GitHub Check: sl-daemon build · macos-latest
- GitHub Check: sl-daemon build · windows-latest
- GitHub Check: sl-viewer macOS app · artifact
- 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_viewer_async_states.rscrates/sl-viewer/tests/properties_session_ledger_export.rscrates/sl-viewer/tests/properties_session_ledger_distill.rscrates/sl-viewer/tests/properties_session_ledger_worklog.rscrates/sl-viewer/tests/properties_session_ledger_okf.rscrates/sl-viewer/tests/properties_session_ledger_intent.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_async_states.rscrates/sl-viewer/tests/properties_session_ledger_export.rscrates/sl-viewer/tests/properties_session_ledger_distill.rscrates/sl-viewer/tests/properties_session_ledger_worklog.rscrates/sl-viewer/tests/properties_session_ledger_okf.rscrates/sl-viewer/tests/properties_session_ledger_intent.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_viewer_async_states.rscrates/sl-viewer/tests/properties_session_ledger_export.rscrates/sl-viewer/tests/properties_session_ledger_distill.rscrates/sl-viewer/tests/properties_session_ledger_worklog.rscrates/sl-viewer/tests/properties_session_ledger_okf.rscrates/sl-viewer/tests/properties_session_ledger_intent.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_async_states.rscrates/sl-viewer/tests/properties_session_ledger_export.rscrates/sl-viewer/tests/properties_session_ledger_distill.rscrates/sl-viewer/tests/properties_session_ledger_worklog.rscrates/sl-viewer/tests/properties_session_ledger_okf.rscrates/sl-viewer/tests/properties_session_ledger_intent.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)
crates/sl-viewer/tests/properties_viewer_async_states.rs (1)
18-103: 📐 Maintainability & Code QualityRun the required Rust validation.
The supplied validation evidence shows one selected test target and
cargo fmt --all --check. It does not show the prescribed locked build, all-features test suite, or Clippy check. Run those checks with the pinned toolchain and attach the results.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_session_ledger_distill.rs (1)
10-15: 📐 Maintainability & Code QualityRun the required Rust validation.
The reported validation runs one feature-specific test target and
cargo fmt --all --check. It does not show the required locked build, workspace all-features tests, or Clippy run. Run those checks with the toolchain pinned inrust-toolchain.tomlbefore merge.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_session_ledger_intent.rs (2)
1-135: LGTM!Also applies to: 171-231
1-231: 📐 Maintainability & Code QualityRun the required Rust validation before merge. Add results for the locked
cargo check -p sl-viewer, all-featurescargo test -p sl-viewer, Clippy, and rustfmt checks required by the Rust workspace guidelines.crates/sl-viewer/tests/properties_session_ledger_worklog.rs (1)
1-95: LGTM!Also applies to: 111-200
crates/sl-viewer/tests/properties_session_ledger_okf.rs (1)
27-31: 📐 Maintainability & Code QualityRun the required Rust validation set with a Rust-capable toolchain.
The provided environment does not have
timeout,pkg-config, orcc/cc1, so these commands cannot produce a valid Rust validation result:
cargo check -p sl-viewer --lockedcargo test --workspace --all-features --lockedcargo clippy --workspace --all-targets --all-features --locked -- -D warningscargo fmt --all --checkRun these from the workspace root with the pinned Rust toolchain and provide the results before merging.
Comment on lines
+181
to
+184
| let out_a = compile_and_store(&session, &store_a).expect("compile_and_store a"); | ||
| let out_b = compile_and_store(&session, &store_b).expect("compile_and_store b"); | ||
| prop_assert_eq!(out_a.bundle, out_b.bundle); | ||
| prop_assert_eq!(out_a.memories.len(), out_b.memories.len()); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Assert deterministic memory values.
Line 184 only compares memory counts. Different episodic memories with the same count pass this property. Compare the ordered memory records, or their stable identifier and content fields, across out_a and out_b.
crates/sl-viewer/tests/properties_session_ledger_distill.rs#L181-L184: compare the generated memory values, not onlymemories.len().CHANGELOG.md#L68-L68: retain the deterministic fresh-store claim only after the property compares memory values.
📍 Affects 2 files
crates/sl-viewer/tests/properties_session_ledger_distill.rs#L181-L184(this comment)CHANGELOG.md#L68-L68
🤖 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_distill.rs` around lines 181
- 184, The property around compile_and_store must compare generated memory
values, not only memories.len(); compare the ordered records or their stable
identifiers and content fields between out_a.memories and out_b.memories. In
CHANGELOG.md lines 68-68, retain the deterministic fresh-store claim after the
property validates memory equivalence.
Comment on lines
+128
to
+161
| /// Every Intent bundle contributes at least one entity | ||
| /// (the goal entity) and the goal's label is the bundle's goal. | ||
| #[test] | ||
| fn intent_bundle_emits_goal_entity( | ||
| goal in "[a-zA-Z0-9 .,!?-]{1,40}", | ||
| n_signals in 0_usize..4, | ||
| n_constraints in 0_usize..4, | ||
| ) { | ||
| let signals: Vec<String> = (0..n_signals).map(|i| format!("signal-{i}")).collect(); | ||
| let constraints: Vec<String> = (0..n_constraints).map(|i| format!("constraint-{i}")).collect(); | ||
| let mut bundle = ContinuationBundle::new("intent-test"); | ||
| bundle.push(Bundle::new( | ||
| BundleKind::Intent, | ||
| serde_json::json!({ | ||
| "goal": goal, | ||
| "acceptance_signals": signals, | ||
| "constraints": constraints, | ||
| "user_turn_count": 1_u64, | ||
| }), | ||
| )); | ||
| let doc = export_to_okf(&bundle, "forge"); | ||
| let intent_entities: Vec<&session_ledger::OkfEntity> = doc | ||
| .entities | ||
| .iter() | ||
| .filter(|e| e.r#type == "intent") | ||
| .collect(); | ||
| prop_assert_eq!(intent_entities.len(), 1); | ||
| prop_assert_eq!(&intent_entities[0].label, &goal); | ||
| // acceptance + constraint entity counts match. | ||
| let acceptance_count = doc.entities.iter().filter(|e| e.r#type == "acceptance").count(); | ||
| let constraint_count = doc.entities.iter().filter(|e| e.r#type == "constraint").count(); | ||
| prop_assert_eq!(acceptance_count, n_signals); | ||
| prop_assert_eq!(constraint_count, n_constraints); | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Test multiple Intent bundles.
This property creates one Intent bundle. It does not verify that each Intent bundle emits its own goal entity.
Generate two or more Intent bundles with distinct goals. Assert that the exported intent labels contain every generated goal. Keep the acceptance and constraint count assertions for the combined input.
🤖 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_export.rs` around lines 128
- 161, Update intent_bundle_emits_goal_entity to generate at least two Intent
bundles with distinct goals and add each to the ContinuationBundle before
export. Assert the exported intent entity labels include every generated goal,
while retaining acceptance and constraint count assertions against the combined
signals and constraints.
Comment on lines
+183
to
+211
| /// Every Acceptance bundle contributes exactly one gate entity | ||
| /// with `ready` and `scope_sized` properties. | ||
| #[test] | ||
| fn acceptance_bundle_emits_gate_entity( | ||
| _seed in any::<u32>(), | ||
| ) { | ||
| let mut bundle = ContinuationBundle::new("acc-test"); | ||
| bundle.push(Bundle::new( | ||
| BundleKind::Acceptance, | ||
| serde_json::json!({ | ||
| "ready": true, | ||
| "scope_sized": true, | ||
| "label": "resume", | ||
| }), | ||
| )); | ||
| let doc = export_to_okf(&bundle, "forge"); | ||
| let gates: Vec<&session_ledger::OkfEntity> = doc | ||
| .entities | ||
| .iter() | ||
| .filter(|e| e.r#type == "gate") | ||
| .collect(); | ||
| prop_assert_eq!(gates.len(), 1); | ||
| // The gate entity's label is fixed ("resume-gate") regardless | ||
| // of the input `label` field — it's a gate, not a per-input | ||
| // accept signal. | ||
| prop_assert_eq!(gates[0].label.as_str(), "resume-gate"); | ||
| prop_assert_eq!(&gates[0].properties["ready"], &serde_json::json!(true)); | ||
| prop_assert_eq!(&gates[0].properties["scope_sized"], &serde_json::json!(true)); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Generate both gate-property values.
_seed does not vary the Acceptance payload. This test passes if export_to_okf always writes true for ready and scope_sized.
Generate ready in any::<bool>() and scope_sized in any::<bool>(). Put both values in the bundle payload. Assert that the gate properties equal the generated values.
🤖 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_export.rs` around lines 183
- 211, Update the acceptance_bundle_emits_gate_entity property test to generate
ready and scope_sized as any::<bool>() values, include those values in the
Acceptance payload, and assert the exported gate properties match the generated
values instead of always asserting true. Keep the existing gate count and fixed
label assertions unchanged.
Comment on lines
+151
to
+152
| prop_assert!(intent.acceptance_signals.contains(&pat.to_string()), | ||
| "acceptance pattern {pat:?} not recognized in {intent:?}"); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Assert the claimed “exactly once” contract. Both properties use contains, which only proves that a pattern appears at least once. A duplicate acceptance or constraint entry can pass these tests.
crates/sl-viewer/tests/properties_session_ledger_intent.rs#L151-L152: count entries equal topatand assert that the count is one.crates/sl-viewer/tests/properties_session_ledger_intent.rs#L166-L167: count entries equal topatand assert that the count is one.
📍 Affects 1 file
crates/sl-viewer/tests/properties_session_ledger_intent.rs#L151-L152(this comment)crates/sl-viewer/tests/properties_session_ledger_intent.rs#L166-L167
🤖 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_intent.rs` around lines 151
- 152, Update both acceptance and constraint assertions in
crates/sl-viewer/tests/properties_session_ledger_intent.rs at lines 151-152 and
166-167 to count entries equal to pat and assert the count is exactly one,
replacing the current contains checks so duplicates fail the properties.
Comment on lines
+240
to
+259
| 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, | ||
| }, | ||
| ]; |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Exercise the dangling-relation error shapes.
This setup does not force every error class. It creates no relation, so it does not test dangling_relation_source or dangling_relation_target.
Add a relation with missing source and target IDs before validation. The existing loop will then verify non-empty fields for both relation diagnostics.
Proposed fix
document.entities = vec![
OkfEntity {
id: "x".into(),
@@
},
];
+ 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.
Suggested change
| 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, | |
| }, | |
| ]; | |
| 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(), | |
| }]; |
🤖 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 -
259, Add a relation with nonexistent source and target entity IDs to the
document setup in every_error_has_nonempty_components before validation, so the
existing diagnostic loop also exercises dangling_relation_source and
dangling_relation_target while preserving the current error-shape assertions.
Comment on lines
+99
to
+108
| 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"); | ||
| prop_assert_eq!(item.reason, UnfinishedReason::MissingCompletionMarker); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Exclude completion markers from this strategy.
body can equal "done" or another value in COMPLETION_MARKERS. In that case, detect_unfinished correctly returns None, but this property calls expect and fails.
Proposed fix
fn assistant_without_completion_marker_is_unfinished(
body in "[a-zA-Z0-9 .,!?]{1,40}",
) {
+ prop_assume!(
+ !COMPLETION_MARKERS
+ .iter()
+ .any(|marker| body.to_lowercase().contains(marker))
+ );
let mut session = Session::new("a", Corpus::Forge);📝 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.
Suggested change
| 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"); | |
| prop_assert_eq!(item.reason, UnfinishedReason::MissingCompletionMarker); | |
| fn assistant_without_completion_marker_is_unfinished( | |
| body in "[a-zA-Z0-9 .,!?]{1,40}", | |
| ) { | |
| prop_assume!( | |
| !COMPLETION_MARKERS | |
| .iter() | |
| .any(|marker| body.to_lowercase().contains(marker)) | |
| ); | |
| 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"); | |
| prop_assert_eq!(item.reason, UnfinishedReason::MissingCompletionMarker); |
🤖 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
- 108, Update assistant_without_completion_marker_is_unfinished to constrain the
generated body so it cannot equal any value in COMPLETION_MARKERS, while
preserving the existing assertion for non-marker bodies. Reuse the existing
completion-marker definition or filtering approach rather than changing
detect_unfinished.
Comment on lines
+208
to
+231
| fn worklog_projection_matches_detect( | ||
| n in 0_usize..5, | ||
| final_role in role_choice(), | ||
| ) { | ||
| let mut session = Session::new("wp", Corpus::Forge); | ||
| session.messages = (0..n) | ||
| .map(|i| { | ||
| // Alternate user / assistant so the message list is | ||
| // meaningful; final role is whatever the strategy picks. | ||
| let role = if i == n - 1 { final_role } else if i % 2 == 0 { Role::User } else { Role::Assistant }; | ||
| let content = match role { | ||
| Role::Assistant => "complete".to_string(), | ||
| _ => format!("msg {i}"), | ||
| }; | ||
| Message::new(role, content) | ||
| }) | ||
| .collect(); | ||
| 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.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Assert the projected unfinished item, not only its count.
The current assertion passes if WorklogProjection::from_session produces one incorrect item. Compare projection.unfinished with the detect_unfinished result.
Proposed fix
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()),
- }
+ let expected = detect_unfinished(&session).into_iter().collect::<Vec<_>>();
+ prop_assert_eq!(projection.unfinished, expected);📝 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.
Suggested change
| fn worklog_projection_matches_detect( | |
| n in 0_usize..5, | |
| final_role in role_choice(), | |
| ) { | |
| let mut session = Session::new("wp", Corpus::Forge); | |
| session.messages = (0..n) | |
| .map(|i| { | |
| // Alternate user / assistant so the message list is | |
| // meaningful; final role is whatever the strategy picks. | |
| let role = if i == n - 1 { final_role } else if i % 2 == 0 { Role::User } else { Role::Assistant }; | |
| let content = match role { | |
| Role::Assistant => "complete".to_string(), | |
| _ => format!("msg {i}"), | |
| }; | |
| Message::new(role, content) | |
| }) | |
| .collect(); | |
| 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()), | |
| } | |
| fn worklog_projection_matches_detect( | |
| n in 0_usize..5, | |
| final_role in role_choice(), | |
| ) { | |
| let mut session = Session::new("wp", Corpus::Forge); | |
| session.messages = (0..n) | |
| .map(|i| { | |
| // Alternate user / assistant so the message list is | |
| // meaningful; final role is whatever the strategy picks. | |
| let role = if i == n - 1 { final_role } else if i % 2 == 0 { Role::User } else { Role::Assistant }; | |
| let content = match role { | |
| Role::Assistant => "complete".to_string(), | |
| _ => format!("msg {i}"), | |
| }; | |
| Message::new(role, content) | |
| }) | |
| .collect(); | |
| let projection = WorklogProjection::from_session(&session); | |
| prop_assert_eq!(projection.message_count, n); | |
| let expected = detect_unfinished(&session).into_iter().collect::<Vec<_>>(); | |
| prop_assert_eq!(projection.unfinished, expected); |
🤖 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 208
- 231, Update the `worklog_projection_matches_detect` property test to compare
`projection.unfinished` against the item returned by `detect_unfinished`, rather
than asserting only its length. Preserve the existing empty-result assertion
when detection returns `None`, and assert the projected collection contains the
exact detected unfinished item when it returns `Some`.
Comment on lines
+31
to
+71
| 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"); | ||
| } | ||
|
|
||
| /// `SkeletonLayout::default()` matches the first arm in the | ||
| /// `match` block in `ContentSkeleton` so adding a new variant | ||
| /// forces a deliberate `default()` change. | ||
| #[test] | ||
| fn skeleton_layout_default_is_first_arm(_seed in any::<u32>()) { | ||
| let first = match () { | ||
| () => SkeletonLayout::Bundles, // mirrors the first match arm in ContentSkeleton | ||
| }; | ||
| prop_assert_eq!(SkeletonLayout::default(), first); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect whether the properties invoke the production layout and clamp paths.
rg -n -C 6 'pub enum SkeletonLayout|pub fn ContentSkeleton|list_rows\.clamp|match layout' \
crates/sl-viewer/src/async_states.rs
rg -n -C 6 'skeleton_layout_has_three_variants|skeleton_layout_default_is_first_arm|list_rows_clamp' \
crates/sl-viewer/tests/properties_viewer_async_states.rsRepository: KooshaPari/SessionLedger
Length of output: 3924
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
import re
src = Path("crates/sl-viewer/src/async_states.rs").read_text()
tests = Path("crates/sl-viewer/tests/properties_viewer_async_states.rs").read_text()
checks = {
"ContentSkeleton.clamps list_rows locally": "list_rows.clamp(3, 6)" in src,
"test imports ContentSkeleton": bool(re.search(r"\bContentSkeleton\b", tests)),
"tests invoke list_rows.clamp(3, 6)": bool(re.search(r"\.clamp\(3,\s*6\)", tests)),
"tests create local variant array": bool(re.search(r"let variants = \[[^\]]*SkeletonLayout::Bundles[^\]]*\]", tests)),
"tests mirror first match arm via hard-coded Bundles": "SkeletonLayout::Bundles" in tests and "skeleton_layout_default_is_first_arm" in tests,
}
for name, ok in checks.items():
print(f"{name}: {ok}")
# Deterministic evidence that changed production range would not be observed by the current clamp test inputs.
for (min_row, max_row) in [(4, 5), (5, 6), (3, 7), (3, 10)]:
def product(values, minn=min_row, maxx=max_row):
return sorted(set(max(minn, min(x, maxx)) for x in values))
def property(values, minn=min_row, maxx=max_row):
clamped = [max(minn, min(x, maxx)) for x in values]
return (
min(clamped) >= minn <= max(clamped) <= maxx
and all(x <= y or all(max(minn, min(a, maxx)) <= max(minn, min(b, maxx)) for b in values) for x in clamped for a in values if values.index(a) is None or True) # no-op keep style simple below
)
PYRepository: KooshaPari/SessionLedger
Length of output: 382
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
import re
src_path = Path("crates/sl-viewer/src/async_states.rs")
test_path = Path("crates/sl-viewer/tests/properties_viewer_async_states.rs")
src = src_path.read_text()
tests = test_path.read_text()
print("contains ContentSkeleton.clamp:", "list_rows.clamp(3, 6)" in src)
print("test imports ContentSkeleton:", bool(re.search(r"\bContentSkeleton\b", tests)))
print("test imports list_rows:" , bool(re.search(r"\blast_rows\b|list_rows\b", tests)))
print("tests directly clamp locally:", bool(re.search(r"\bclamp\(3,\s*6\)", tests)))
print("local variant names in properties test:", bool(re.search(r"let variants = \[\s*\n", tests)))
print("default test mirrors Bundles:", "SkeletonLayout::Bundles" in tests.re find and "SKeletonLayout::Default()"))
samples = list(range(12))
clamped = [max(3, min(x, 6)) for x in samples]
print("samples clamped under 3-6:", clamped)
print("samples clamped under 4-5:", [max(4, min(x, 5)) for x in samples])
print("samples clamped under 3-10:", [max(3, min(x, 10)) for x in samples])
print("property would still pass under 4-5 for range inclusion:", min(max(4, min(x,5)) for x in clamped) >= 4 and max(max(4,min(x,5)) for x in clamped) <=5)
print("property would still pass under 3-10 for range inclusion:", min(max(3, min(x,10)) for x in clamped) >= 3 and max(max(3,min(x,10)) for x in clamped) <=10)
PYRepository: KooshaPari/SessionLedger
Length of output: 419
Test the production contract instead of local copies.
The enum-count, first-arm, and clamp properties duplicate production behavior inside the test. A new SkeletonLayout variant can leave the enum-count assertion passing, and a changed list_rows.clamp(3, 6) range in ContentSkeleton can leave the clamp assertions passing. Move the rows clamp into a production-owned helper and assert it from an internal unit test, or render ContentSkeleton and assert its row count.
🤖 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 -
71, Update the tests around skeleton_layout_has_three_variants,
skeleton_layout_default_is_first_arm, and the related clamp properties to
exercise production behavior rather than duplicated local matches and constants.
Move the list-row clamp into a production-owned ContentSkeleton helper with an
internal unit test, or render ContentSkeleton and assert its rendered row count;
ensure enum coverage and default behavior are likewise derived from the
production implementation so new variants or changed bounds cannot leave tests
passing.
| | WBS-5.1 | P5 deterministic dedup merge and crash/lost-work recovery E2E | done | machine | `src/domain/merge.rs`; `src/domain/worklog.rs`; `tests/merge_recovery.rs` | FR-011; T-024, T-035; C03 | | ||
| | WBS-6.1 | P6 85% coverage gate and deterministic golden corpus | done | machine | `.github/workflows/ci.yml`; `tests/okf_golden.rs`; `tests/fixtures/okf/` | T-037, T-038; C01, C08 | | ||
| | WBS-6.2 | P6 property tests, fuzzing, race checks, and enforced performance budgets | partial | machine | `tests/properties.rs`; `crates/sl-viewer/tests/properties_viewer.rs`; `crates/sl-viewer/tests/properties_viewer_theme_url.rs`; `crates/sl-viewer/tests/properties_viewer_unfinished_tab.rs`; `crates/sl-viewer/tests/properties_viewer_timeline.rs`; `crates/sl-viewer/tests/properties_viewer_search_memory.rs`; `crates/sl-viewer/tests/properties_viewer_history.rs`; `crates/sl-viewer/tests/properties_viewer_web_exports.rs`; `crates/sl-viewer/tests/properties_viewer_bundle_detail.rs`; `crates/sl-viewer/tests/properties_viewer_bundle_diff.rs`; `crates/sl-viewer/tests/properties_viewer_mock_data.rs`; `crates/sl-viewer/tests/properties_viewer_cli_help.rs`; `crates/sl-viewer/tests/properties_viewer_corpus_cta.rs`; `crates/sl-viewer/tests/properties_viewer_theme.rs`; `crates/sl-viewer/tests/properties_viewer_settings.rs`; `crates/sl-viewer/tests/properties_viewer_corpus_paths.rs`; `crates/sl-viewer/tests/properties_viewer_help_overlay.rs`; `crates/sl-viewer/tests/properties_viewer_settings_tab.rs`; `crates/sl-viewer/tests/properties_viewer_menu.rs`; `fuzz/fuzz_targets/okf_roundtrip.rs`; `fuzz/fuzz_targets/jsonl_ingest.rs`; `.github/workflows/ci.yml`; `.github/workflows/bench-gate.yml`; `docs/ops/perf-baseline.json`; `scripts/bench-gate.ps1`; `benches/pipeline.rs`; `tests/loom_model.rs` | DESIGN P6 residual; C00 L6-L8; C07 L66-L68; C08 L74; perf-budget enforced Wave-26 #223; p95 latency enforced Wave-30 #256; FSM properties Wave-31 #261; soft loom Wave-31 #264; viewer corpus_paths/parquet/settings properties #425; viewer theme + daemon_url properties #427; viewer unfinished_tab properties + fuzz/rootless CI drift fixes #428; viewer bundle_diff + timeline properties + web_exports/hmetic-pin cleanups #432; viewer bundle_diff properties #434; viewer search/memory properties #435; viewer history_tab properties #444; viewer web_exports properties #437; viewer bundle_list + detail_pane properties #436; viewer mock_data fixture properties #451; viewer cli_help / command_palette properties #452; viewer corpus_cta constants properties #453; viewer theme + settings properties #454; viewer corpus_paths round-trip properties #446; viewer help_overlay shortcuts properties #455; viewer settings_tab HealthStatus properties #456; viewer menu id taxonomy properties #457; full loom/shuttle unpaid | | ||
| | WBS-6.2 | P6 property tests, fuzzing, race checks, and enforced performance budgets | partial | machine | `tests/properties.rs`; `crates/sl-viewer/tests/properties_viewer.rs`; `crates/sl-viewer/tests/properties_viewer_theme_url.rs`; `crates/sl-viewer/tests/properties_viewer_unfinished_tab.rs`; `crates/sl-viewer/tests/properties_viewer_timeline.rs`; `crates/sl-viewer/tests/properties_viewer_search_memory.rs`; `crates/sl-viewer/tests/properties_viewer_history.rs`; `crates/sl-viewer/tests/properties_viewer_web_exports.rs`; `crates/sl-viewer/tests/properties_viewer_bundle_detail.rs`; `crates/sl-viewer/tests/properties_viewer_bundle_diff.rs`; `crates/sl-viewer/tests/properties_viewer_mock_data.rs`; `crates/sl-viewer/tests/properties_viewer_cli_help.rs`; `crates/sl-viewer/tests/properties_viewer_corpus_cta.rs`; `crates/sl-viewer/tests/properties_viewer_theme.rs`; `crates/sl-viewer/tests/properties_viewer_settings.rs`; `crates/sl-viewer/tests/properties_viewer_corpus_paths.rs`; `crates/sl-viewer/tests/properties_viewer_help_overlay.rs`; `crates/sl-viewer/tests/properties_viewer_settings_tab.rs`; `crates/sl-viewer/tests/properties_viewer_menu.rs`; `crates/sl-viewer/tests/properties_viewer_async_states.rs`; `crates/sl-viewer/tests/properties_session_ledger_okf.rs`; `crates/sl-viewer/tests/properties_session_ledger_worklog.rs`; `crates/sl-viewer/tests/properties_session_ledger_export.rs`; `crates/sl-viewer/tests/properties_session_ledger_distill.rs`; `crates/sl-viewer/tests/properties_session_ledger_intent.rs`; `fuzz/fuzz_targets/okf_roundtrip.rs`; `fuzz/fuzz_targets/jsonl_ingest.rs`; `.github/workflows/ci.yml`; `.github/workflows/bench-gate.yml`; `docs/ops/perf-baseline.json`; `scripts/bench-gate.ps1`; `benches/pipeline.rs`; `tests/loom_model.rs` | DESIGN P6 residual; C00 L6-L8; C07 L66-L68; C08 L74; perf-budget enforced Wave-26 #223; p95 latency enforced Wave-30 #256; FSM properties Wave-31 #261; soft loom Wave-31 #264; viewer corpus_paths/parquet/settings properties #425; viewer theme + daemon_url properties #427; viewer unfinished_tab properties + fuzz/rootless CI drift fixes #428; viewer bundle_diff + timeline properties + web_exports/hmetic-pin cleanups #432; viewer bundle_diff properties #434; viewer search/memory properties #435; viewer history_tab properties #444; viewer web_exports properties #437; viewer bundle_list + detail_pane properties #436; viewer mock_data fixture properties #451; viewer cli_help / command_palette properties #452; viewer corpus_cta constants properties #453; viewer theme + settings properties #454; viewer corpus_paths round-trip properties #446; viewer help_overlay shortcuts properties #455; viewer settings_tab HealthStatus properties #456; viewer menu id taxonomy properties #457; viewer async_states SkeletonLayout properties #458; session-ledger OKF document validator properties #459; session-ledger worklog projector properties #460; session-ledger OKF export adapter properties #461; session-ledger distill compiler properties #462; session-ledger heuristic intent extractor properties #463; full loom/shuttle unpaid | |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Correct the hmetic-pin typo.
Replace hmetic-pin cleanups with hermetic-pin cleanups. This matches the hermetic.yml cleanup documented in CHANGELOG.md.
🧰 Tools
🪛 LanguageTool
[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)
🤖 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 `@docs/ops/WBS.md` at line 32, Update the WBS-6.2 documentation entry to
replace the misspelled “hmetic-pin cleanups” phrase with “hermetic-pin
cleanups,” preserving the surrounding milestone references and wording.
Source: Linters/SAST tools
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
User description
Summary
Adds
crates/sl-viewer/tests/properties_session_ledger_intent.rswith 10 proptest properties pinningsession_ledger::distill::extractor::HeuristicIntentExtractor::extract_intent(WBS-6.2 #463) — the P1 SSOT for what the user wants (drives resume prompt, search index, wiki/docs view).extract_intent(10 properties)user_turn_countalways equals the count ofRole::Usermessages and ignoresAssistant/Subagent/Tool/System.Intent.Goal:/Objective:/Task:lines win over preamble.Constraint:/Requirement:/Boundary:lines carry their full text.extract_intentis deterministic across calls.Validation
cargo test -p sl-viewer --test properties_session_ledger_intent --features "desktop parquet" --locked— 10 passedcargo fmt --all --check— cleanWBS / TRACEABILITY
WBS-6.2 evidence list and
TRACEABILITY.jsongaincrates/sl-viewer/tests/properties_session_ledger_intent.rs. CHANGELOG Unreleased documents the new surface.CodeAnt-AI Description
Add property-based coverage for session-ledger pipelines and viewer async layouts
What Changed
Impact
✅ More reliable resume recovery✅ Valid OKF exports for search and wiki consumers✅ Consistent intent and constraint extraction✅ Stable viewer loading layouts💡 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.