WBS-6.2: viewer unfinished_tab properties + fuzz/rootless/clippy CI drift fixes by KooshaPari · Pull Request #429 · KooshaPari/SessionLedger

KooshaPari · 2026-08-08T06:18:31Z

User description

Summary

Four bounded cleanups that were visible as soft CI failures during PR #425/#427 but pre-existed on main. Plus a third viewer property-test file.

scripts/fuzz-cadence-check.ps1 drift — the "PR smoke stays short" anchor checked ci.yml for max_total_time=10, but the 10 s fuzz-smoke job no longer lives there (consolidated into fuzz-blocking.yml at 30 s). Re-pointed the check at the actual PR fuzz budget: fuzz-blocking.yml with max_total_time=30.
scripts/rootless-nonet-check.ps1 + .github/workflows/ci.yml drift — the script expected ci.yml to cross-reference rootless-nonet.yml, but that anchor had drifted out of ci.yml while security.yml retained it. Added a small rootless-nonet-policy smoke job to ci.yml (matches the docs/ops/sandbox-boundary.md C04 L40 contract that "ci.yml cross-reference | done") and tightened the script's regex so continue-on-error detection can't bleed across jobs.
cargo clippy --all-targets --all-features -- -D warnings cleanups:
- tests/alloc_profile.rs — clippy::panic_in_if_then: folded the Windows panic! into an explicit else branch.
- tests/replay_breadth.rs — 6 × clippy::unnecessary_trailing_comma in assert! macros.
crates/sl-viewer/tests/properties_viewer_unfinished_tab.rs — 6 proptest properties:
- reason_label_is_non_empty_for_every_variant
- reason_label_is_injective
- unfinished_items_is_deterministic
- unfinished_items_orders_known_timestamps_descending
- unfinished_items_ties_break_by_session_id_ascending
- unfinished_items_is_length_monotonic

Validation

pwsh ./scripts/fuzz-cadence-check.ps1 -SelfCheck — passes.
pwsh ./scripts/rootless-nonet-check.ps1 -SelfCheck — passes.
cargo clippy --all-targets --all-features --locked -- -D warnings — clean.
cargo test -p sl-viewer --test properties_viewer --test properties_viewer_theme_url --test properties_viewer_unfinished_tab --features "desktop parquet" --locked — 16 passed.
cargo test --test alloc_profile --all-features --locked — 2 passed.
cargo test --test replay_breadth --all-features --locked — 5 passed.

WBS / TRACEABILITY

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

Out of scope

Pre-existing tests/sl-viewer/src/settings.rs:351 clippy::len_zero (already on main, unrelated).
Pre-existing tests/sl-viewer/src/web_exports.rs:48 dead_code warning (already on main, unrelated).

CodeAnt-AI Description

Add unfinished-tab property coverage and restore accurate CI drift checks

What Changed

Added property coverage for unfinished-tab labels and item ordering, including non-empty unique reason labels, deterministic results, timestamp ordering, session ID tie-breaking, and length stability.
Restored a pull-request CI smoke check for the rootless/no-network policy and ensured it evaluates only that job.
Updated the fuzz CI check to enforce the current 30-second PR fuzz budget in the blocking workflow.
Fixed test code that prevented clean all-target, all-feature Clippy runs.
Recorded the new viewer coverage and CI safeguards in project tracking and release notes.

Impact

✅ Catches unfinished-tab ordering and labeling regressions
✅ Keeps rootless/no-network policy checks visible in pull-request CI
✅ Prevents drift from weakening the bounded PR fuzz budget

💡 Usage Guide

Checking Your Pull Request

Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.

Talking to CodeAnt AI

Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:

@codeant-ai ask: Your question here

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

Example

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

Preserve Org Learnings with CodeAnt

You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:

@codeant-ai: Your feedback here

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

Example

@codeant-ai: Do not flag unused imports.

Retrigger review

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

@codeant-ai: review

Check Your Repository Health

To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.

…hed_tab properties Four bounded cleanups observed while CI was running on #425/#427: 1) scripts/fuzz-cadence-check.ps1: the "PR smoke stays short" anchor was checking ci.yml for max_total_time=10, but the 10s fuzz-smoke job no longer lives there (consolidated into fuzz-blocking.yml at 30s). Re-pointed the check at the actual PR fuzz budget: fuzz-blocking.yml with max_total_time=30. 2) scripts/rootless-nonet-check.ps1 + .github/workflows/ci.yml: the script expected ci.yml to cross-reference rootless-nonet.yml, but that anchor had drifted out of ci.yml while security.yml retained it. Added a small rootless-nonet-policy smoke job to ci.yml (matches the docs/ops/sandbox-boundary.md C04 L40 contract that 'ci.yml cross-reference | done') and tightened the script's regex so continue-on-error detection can't bleed across jobs. 3) clippy -D warnings under --all-targets --all-features: * tests/alloc_profile.rs: panic-in-if-then — folded the Windows panic into an explicit else branch. * tests/replay_breadth.rs: 6 unnecessary trailing commas in assert! macros. 4) crates/sl-viewer/tests/properties_viewer_unfinished_tab.rs: 6 proptest properties covering reason_label (non-empty, injective) and unfinished_items (deterministic, descending by ts_ms, tiebreak by session_id asc, length-monotonic). Adds to WBS-6.2 viewer property surface alongside #425/#427. WBS-6.2 evidence list and CHANGELOG Unreleased reflect the new property file.

WBS-6.2 evidence list and TRACEABILITY.json gain the new properties_viewer_unfinished_tab.rs and reference #428 for the unfinished-tab properties + CI drift fixups. Status stays partial (fuzzing cadence, full loom/shuttle, perf-budget gates remain).

Copilot

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

codeant-ai · 2026-08-08T06:18:37Z

🤖 CodeAnt AI — Review Status

Status	Commit	Started (UTC)	Finished (UTC)
✅ Reviewed your PR	`cb04180`	Aug 08, 2026 · 06:18	06:21

codeant-ai · 2026-08-08T06:18:37Z

Thanks for using CodeAnt! 🎉

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

Share on X ·
Reddit ·
LinkedIn

coderabbitai · 2026-08-08T06:18:57Z

📝 Walkthrough

Summary

This PR fixes CI drift and adds property-based coverage for viewer unfinished-tab behavior.

Updates fuzz cadence validation to check the 30-second PR fuzz budget in fuzz-blocking.yml.
Restores the rootless-nonet.yml cross-reference in ci.yml.
Restricts continue-on-error detection to the rootless-nonet-policy job.
Fixes Clippy issues in tests/alloc_profile.rs and tests/replay_breadth.rs.
Adds six property tests for unfinished-tab labels, ordering, determinism, tie-breaking, and item retention.
Updates WBS-6.2 evidence, traceability, and the Unreleased changelog.

Must Fix

No blocking issues are reported. The stated validation passed for Clippy, viewer property tests, affected test suites, and updated scripts.

Should Fix

No non-blocking issues are reported.

Consider

WBS-6.2 remains partial. The traceability updates correctly record the additional test evidence.

Approve / Request Changes

Approve.

Walkthrough

The PR adds property-based coverage for unfinished viewer tabs, updates CI self-checks for rootless networking and fuzz cadence, adjusts portable profile validation, and records the new evidence and maintenance changes.

Changes

Viewer and CI validation

Layer / File(s)	Summary
Unfinished-tab property validation `crates/sl-viewer/tests/properties_viewer_unfinished_tab.rs`, `docs/ops/TRACEABILITY.json`, `docs/ops/WBS.md`	Adds property tests for labels, deterministic item projection, timestamp ordering, session-ID tie-breaking, and length monotonicity. Adds the test to WBS-6.2 evidence.
CI policy and fuzz contract checks `.github/workflows/ci.yml`, `scripts/rootless-nonet-check.ps1`, `scripts/fuzz-cadence-check.ps1`, `CHANGELOG.md`	Adds a pull-request rootless non-network smoke job. Scopes policy detection to its job block and validates the 30-second budget in `fuzz-blocking.yml`. Records these changes in the unreleased changelog.
Portable test execution cleanup `tests/alloc_profile.rs`, `tests/replay_breadth.rs`	Uses an explicit non-Windows fallback for profile validation and removes trailing commas from replay-breadth assertions without changing test conditions.

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

Possibly related PRs

KooshaPari/SessionLedger#392: Introduced the shared rootless-nonet-policy CI job.
KooshaPari/SessionLedger#423: Added related rootless/no-network policy checks in the same workflow and script.
KooshaPari/SessionLedger#266: Updated fuzz cadence contract validation in scripts/fuzz-cadence-check.ps1.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly identifies the unfinished-tab property tests and the fuzz, rootless, and Clippy CI drift fixes.
Description check	✅ Passed	The description directly explains the property tests, CI drift fixes, Clippy cleanup, validation, and traceability updates.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches 💡 2

📝 Generate docstrings 💡

Create stacked PR
Commit on current branch

🛠️ Fix failing CI checks 💡

Create stacked PR
Commit on current branch

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch fix/ci-drift-cleanup-20260807

✨ Simplify code

Create PR with simplified code
Commit simplified code in branch fix/ci-drift-cleanup-20260807

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

codeant-ai · 2026-08-08T06:21:35Z

+if ($blockingWorkflow -notmatch 'max_total_time=30') {
+    throw "fuzz-blocking.yml must keep -max_total_time=30 for the PR fuzz budget (do not slow PR CI here)."
 }
-[void](Write-Check -Label "ci.yml fuzz-smoke max_total_time=10" -Ok $true)
+[void](Write-Check -Label "fuzz-blocking.yml PR fuzz max_total_time=30" -Ok $true)


Suggestion: This replacement check no longer validates the separate blocking ci.yml fuzz-smoke lane or its 10-second limit. Because $ci is no longer checked here, removing that job or weakening its budget still passes SelfCheck, despite the documentation continuing to define it as required blocking PR coverage. Keep an explicit ci.yml smoke-job anchor in addition to the sustained-workflow check. [incomplete implementation]

Severity Level: Major ⚠️

- ❌ Required 10-second PR fuzz coverage is not enforced. - ⚠️ CI drift checks ignore the documented `ci.yml` lane. - ⚠️ Fuzz validation changes from short smoke to sustained coverage.

(Use Cmd/Ctrl + Click for best experience)

Prompt for AI Agent 🤖

This is a comment left during a code review. **Path:** scripts/fuzz-cadence-check.ps1 **Line:** 176:179 **Comment:** *Incomplete Implementation: This replacement check no longer validates the separate blocking `ci.yml` `fuzz-smoke` lane or its 10-second limit. Because `$ci` is no longer checked here, removing that job or weakening its budget still passes SelfCheck, despite the documentation continuing to define it as required blocking PR coverage. Keep an explicit `ci.yml` smoke-job anchor in addition to the sustained-workflow check. 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

👍 | 👎

codeant-ai · 2026-08-08T06:21:35Z

            if cfg!(target_os = "windows") {
                panic!("failed to spawn pwsh for self-check: {error}");
+            } else {
+                let (max_bytes, total_blocks) = load_profile();
+                println!(
+                    "pwsh unavailable; running portable alloc-profile SelfCheck fallback.\nSelf-check passed\nMax bytes ceiling: {max_bytes}\nTotal blocks ceiling: {total_blocks}\nProfiler: dhat"


Suggestion: The non-Windows fallback converts every pwsh spawn failure—including permission errors, invalid invocation failures, and other environment problems—into a fabricated successful SelfCheck result. This means the test can pass without executing scripts/alloc-profile-check.ps1 or validating its documented workflow and path anchors. Only use the fallback for an explicitly detected missing executable, or fail the test for other spawn errors. [possible bug]

Severity Level: Major ⚠️

- ⚠️ Linux tests can pass without executing alloc-profile SelfCheck. - ⚠️ PowerShell workflow-anchor regressions may be missed. - ⚠️ Non-missing-executable spawn failures are misreported as success.

(Use Cmd/Ctrl + Click for best experience)

Prompt for AI Agent 🤖

This is a comment left during a code review. **Path:** tests/alloc_profile.rs **Line:** 90:95 **Comment:** *Possible Bug: The non-Windows fallback converts every `pwsh` spawn failure—including permission errors, invalid invocation failures, and other environment problems—into a fabricated successful SelfCheck result. This means the test can pass without executing `scripts/alloc-profile-check.ps1` or validating its documented workflow and path anchors. Only use the fallback for an explicitly detected missing executable, or fail the test for other spawn errors. 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

Actionable comments posted: 5

🤖 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 @.github/workflows/ci.yml:
- Around line 203-206: Update the actions/checkout step in the rootless / no-net
SelfCheck job to use an immutable commit SHA instead of the floating v7 tag, and
configure checkout with persist-credentials disabled.

In `@crates/sl-viewer/tests/properties_viewer_unfinished_tab.rs`:
- Around line 120-123: Update the test around the with_ts filtering in the
unfinished-work sorting assertions to retain all items, including those with
last_activity_ms == None. Add an assertion that no item with
Some(last_activity_ms) appears after an item with None, while preserving the
existing descending-order check for known timestamps.
- Around line 35-55: Update the property generator around the session message
construction to generate an independent Option<i64> timestamp for each message
instead of one session-wide value, while preserving the None unknown-timestamp
case. Track the maximum known timestamp per session and assert the projected
unfinished_items last_activity_ms matches that maximum, following the contract
used by unfinished_items.

In `@scripts/rootless-nonet-check.ps1`:
- Around line 134-145: Update the validation around $policyBlockMatch in
rootless-nonet-check.ps1 to throw when the rootless-nonet-policy block is
absent, rather than treating a failed match as success. Keep the existing
continue-on-error validation for a present block and only call Write-Check with
Ok $true after confirming the block exists and is blocking.

In `@tests/alloc_profile.rs`:
- Around line 85-96: Update the non-Windows error handling around the pwsh
`Command::output()` call so the portable fallback runs only when `error.kind()
== std::io::ErrorKind::NotFound`; panic with the existing failure context for
all other spawn errors, including permission failures. Preserve the Windows
behavior and the successful output path.

🪄 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: 8959c577-3a15-42af-a795-3598bf1728d5

📥 Commits

Reviewing files that changed from the base of the PR and between f8c3f33 and cb04180.

📒 Files selected for processing (9)

.github/workflows/ci.yml
CHANGELOG.md
crates/sl-viewer/tests/properties_viewer_unfinished_tab.rs
docs/ops/TRACEABILITY.json
docs/ops/WBS.md
scripts/fuzz-cadence-check.ps1
scripts/rootless-nonet-check.ps1
tests/alloc_profile.rs
tests/replay_breadth.rs

📜 Review details

⏰ Context from checks skipped due to timeout. (28)

GitHub Check: semgrep-cloud-platform/scan
GitHub Check: load macro gate · macro routes smoke
GitHub Check: update check hard · root SelfCheck wrapper
GitHub Check: CVE feed subscription smoke (soft)
GitHub Check: update check hard · sl-daemon tests
GitHub Check: Socket posture SelfCheck
GitHub Check: gitleaks
GitHub Check: tsan permutation · race_model
GitHub Check: cargo deny check
GitHub Check: SLSA protected-environment SelfCheck
GitHub Check: env.example hygiene
GitHub Check: jemalloc default-on · SelfCheck
GitHub Check: jemalloc default-on · windows default build
GitHub Check: trufflehog
GitHub Check: fuzz blocking · sustained 30s
GitHub Check: cargo audit
GitHub Check: browser e2e · axe · responsive · visual
GitHub Check: visual contract · WCAG AA
GitHub Check: prepare
GitHub Check: shuttle permutation · cargo test shuttle_permutation
GitHub Check: daemon graph hard · tokio graph
GitHub Check: sl-daemon build · windows-latest
GitHub Check: sl-daemon build · macos-latest
GitHub Check: sl-viewer help · unit tests
GitHub Check: sl-viewer macOS app · artifact
GitHub Check: Summary
GitHub Check: browser e2e · axe · responsive · visual
GitHub Check: prepare

⚠️ CI failures not shown inline (2)

GitHub Actions: Trunk Check / 0_Lint & Format.txt: WBS-6.2: viewer unfinished_tab properties + fuzz/rootless/clippy CI drift fixes

Conclusion: failure

View job details

##[group]Run cat >>$GITHUB_ENV <<EOF
 �[36;1mcat >>$GITHUB_ENV <<EOF�[0m
 �[36;1mGITHUB_***REDACTED_SECRET_ASSIGNMENT***
 �[36;1mTRUNK_LAUNCHER_QUIET=false�[0m
 �[36;1mEOF�[0m
 �[36;1m�[0m
 �[36;1m# First arg is field to fetch, second arg is default value or empty�[0m
 �[36;1mpayload() {�[0m
 �[36;1m  if [ $# -lt 2 ]; then�[0m
 �[36;1m    DEFAULT_VALUE=empty�[0m
 �[36;1m  else�[0m
 �[36;1m    DEFAULT_VALUE=\"$2\"�[0m
 �[36;1m  fi�[0m
 �[36;1m  if command -v jq >/dev/null; then�[0m
 �[36;1m    jq -r ".inputs.payload | fromjson | .$1 // ${DEFAULT_VALUE}" ${TEST_GITHUB_EVENT_PATH:-${GITHUB_EVENT_PATH}}�[0m
 �[36;1m  else�[0m
 �[36;1m    echo "::error::jq not installed on system!"�[0m

GitHub Actions: Trunk Check / Lint & Format: WBS-6.2: viewer unfinished_tab properties + fuzz/rootless/clippy CI drift fixes

Conclusion: failure

View job details

##[group]Run cat >>$GITHUB_ENV <<EOF
 �[36;1mcat >>$GITHUB_ENV <<EOF�[0m
 �[36;1mGITHUB_***REDACTED_SECRET_ASSIGNMENT***
 �[36;1mTRUNK_LAUNCHER_QUIET=false�[0m
 �[36;1mEOF�[0m
 �[36;1m�[0m
 �[36;1m# First arg is field to fetch, second arg is default value or empty�[0m
 �[36;1mpayload() {�[0m
 �[36;1m  if [ $# -lt 2 ]; then�[0m
 �[36;1m    DEFAULT_VALUE=empty�[0m
 �[36;1m  else�[0m
 �[36;1m    DEFAULT_VALUE=\"$2\"�[0m
 �[36;1m  fi�[0m
 �[36;1m  if command -v jq >/dev/null; then�[0m
 �[36;1m    jq -r ".inputs.payload | fromjson | .$1 // ${DEFAULT_VALUE}" ${TEST_GITHUB_EVENT_PATH:-${GITHUB_EVENT_PATH}}�[0m
 �[36;1m  else�[0m
 �[36;1m    echo "::error::jq not installed on system!"�[0m

🧰 Additional context used

📓 Path-based instructions (5)

**/*.{rs,toml}