Skip to content

test(viewer): session-ledger pii redact proptest surface (WBS-6.2 #473) - #492

Closed
KooshaPari wants to merge 0 commit into
mainfrom
fix/viewer-pii-redact-properties-20260809
Closed

test(viewer): session-ledger pii redact proptest surface (WBS-6.2 #473)#492
KooshaPari wants to merge 0 commit into
mainfrom
fix/viewer-pii-redact-properties-20260809

Conversation

@KooshaPari

@KooshaPari KooshaPari commented Aug 11, 2026

Copy link
Copy Markdown
Owner

User description

WBS-6.2 surface 13: session-ledger pii helpers.

Branch off fix/viewer-mock-data-properties-20260809 rebased main. Test file compiles and passes locally.


CodeAnt-AI Description

Add property coverage for session-ledger PII and secret redaction

What Changed

  • Added randomized checks that email addresses are replaced, API keys with supported prefixes are scrubbed, and safe text remains unchanged
  • Verified redaction handles empty, malformed, ASCII, and Unicode input without panicking
  • Verified redaction is idempotent so applying it repeatedly does not alter already-scrubbed content
  • Recorded the new PII redaction coverage in the work plan, traceability records, and changelog

Impact

✅ Safer email and API-key handling
✅ No crashes on arbitrary text
✅ Stable repeated redaction

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

Copilot AI lite review requested due to automatic review settings August 11, 2026 00:05
@codeant-ai

codeant-ai Bot commented Aug 11, 2026

Copy link
Copy Markdown

🤖 CodeAnt AI — Review Status

Status Commit Started (UTC) Finished (UTC)
✅ Incremental review completed d481319 Aug 12, 2026 · 08:21 08:21
✅ Reviewed your PR 1f4b6c9 Aug 11, 2026 · 00:05 00:08

@codeant-ai

codeant-ai Bot commented Aug 11, 2026

Copy link
Copy Markdown

Thanks for using CodeAnt! 🎉

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

Share on X ·
Reddit ·
LinkedIn

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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

@codeant-ai codeant-ai Bot added the size:XXL This PR changes 1000+ lines, ignoring generated files label Aug 11, 2026
@coderabbitai

coderabbitai Bot commented Aug 11, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Summary

This PR adds property-based tests for SessionLedger context extraction, contract extraction, and PII redaction. It also updates the changelog and WBS traceability records.

The test coverage is broad and includes empty inputs, whitespace handling, case-insensitive matching, deduplication, deterministic results, idempotence, Unicode input, and trait-path equivalence.

Must Fix

No blocking issues identified from the provided changes.

Should Fix

No non-blocking issues identified.

Consider

Confirm that cargo fmt, cargo clippy --workspace -- -D warnings, and cargo test --workspace pass in CI.

Approve / Request Changes

Approve.

Walkthrough

Added property-based tests for SessionLedger context extraction, contract extraction, and PII redaction. Updated the changelog and WBS-6.2 traceability records.

Changes

SessionLedger property tests

Layer / File(s) Summary
Context extraction properties
crates/sl-viewer/tests/properties_session_ledger_context.rs
Tests cover metadata mirroring, pattern detection, case handling, deduplication, determinism, trait parity, and empty-state consistency.
Contract extraction properties
crates/sl-viewer/tests/properties_session_ledger_contract.rs
Tests cover contract formatting, trigger detection, case handling, deduplication, determinism, trait parity, and empty-state consistency.
PII redaction properties and traceability
crates/sl-viewer/tests/properties_session_ledger_pii_redact.rs, CHANGELOG.md, docs/ops/TRACEABILITY.json, docs/ops/WBS.md
Tests cover email and API-key redaction, arbitrary input, idempotence, output shape, and Unicode preservation. WBS-6.2 evidence references were updated.

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

Possibly related PRs

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title clearly identifies the session-ledger PII redaction property-test changes, which are the primary objective of the pull request.
Description check ✅ Passed The description directly explains the PII redaction property tests and related documentation updates in the changeset.
✨ Finishing Touches 💡 2
⚔️ Resolve merge conflicts 💡
  • Resolve merge conflict in branch fix/viewer-pii-redact-properties-20260809
🛠️ Fix failing CI checks 💡
  • Create stacked PR
  • Commit on current branch
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/viewer-pii-redact-properties-20260809
✨ Simplify code
  • Create PR with simplified code
  • Commit simplified code in branch fix/viewer-pii-redact-properties-20260809

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.

Comment on lines +182 to +184
let out = redact(&body);
// Sanity: no panic, returns a String.
let _ = out.len();

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Suggestion: This property does not assert any output invariant: it computes out.len() and discards the result. It would pass if redaction returned an empty string or corrupted the input, so it provides no regression protection despite its output-shape description. Assert the intended relationship or expected output for inputs with and without matches. [logic error]

Severity Level: Major ⚠️
- ⚠️ Output corruption can pass the redaction property suite.

Fix in Cursor Fix in VSCode Claude

Prompt for AI Agent 🤖
This is a comment left during a code review.

**Path:** crates/sl-viewer/tests/properties_session_ledger_pii_redact.rs
**Line:** 182:184
**Comment:**
	*Logic Error: This property does not assert any output invariant: it computes `out.len()` and discards the result. It would pass if redaction returned an empty string or corrupted the input, so it provides no regression protection despite its output-shape description. Assert the intended relationship or expected output for inputs with and without matches.

Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
👍 | 👎

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 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_context.rs`:
- Around line 398-405: Update the property assertions around
HeuristicContextExtractor::extract_context in both repeated-extraction checks to
compare each corresponding key_decisions entry’s complete Decision values,
including summary and rationale, rather than only comparing collection lengths.
Preserve the existing assertions for the other context fields.
- Around line 457-462: Replace the two conditional assertions in
crates/sl-viewer/tests/properties_session_ledger_context.rs:457-462 with a
direct prop_assert_eq!(ctx.is_empty(), all_empty) comparison. Apply the same
replacement in
crates/sl-viewer/tests/properties_session_ledger_contract.rs:364-369 using
c.is_empty(), so both tests verify the full equivalence in both directions.
- Around line 149-162: Strengthen the selected-pattern assertions in every
listed coverage property: in
crates/sl-viewer/tests/properties_session_ledger_context.rs at lines 149-162,
assert files_mentioned contains main{ext}; at lines 202-210, assert
key_decisions contains the formatted Session contains '{pat}' language entry;
and at lines 358-366, assert environment_notes contains Session mentions
'{pat}'. In crates/sl-viewer/tests/properties_session_ledger_contract.rs at
lines 151-190, assert the expected formatted entry appears in the appropriate
section for each selected pattern.

In `@crates/sl-viewer/tests/properties_session_ledger_contract.rs`:
- Around line 195-208: The case-insensitivity property currently covers only
CRITERIA_PATTERNS and success_criteria. Add equivalent property tests for
TEST_PATTERNS, CONSTRAINT_PATTERNS, and DO_NOT_TOUCH_PATTERNS, each uppercasing
the selected pattern, extracting from a session via
HeuristicContractExtractor::extract_contract, and asserting the corresponding
contract section is non-empty.

In `@crates/sl-viewer/tests/properties_session_ledger_pii_redact.rs`:
- Around line 176-185: Update the redact_output_is_a_string property test to
generate input containing an email surrounded by arbitrary text, then assert
that redact preserves the surrounding text and replaces only the email with
REDACTED_EMAIL. Remove the no-op out.len() check while retaining coverage for
generated inputs.
- Around line 189-198: Update the redact_preserves_unicode generator so its
inputs cannot match recognized API-key patterns, preferably by constraining
generated text to non-ASCII characters; keep the existing @ filtering and
unchanged-output assertions for the resulting safe string.

In `@docs/ops/WBS.md`:
- Line 32: Update the WBS-6.2 evidence path list to include
crates/sl-viewer/tests/properties_session_ledger_contract.rs alongside the
existing session-ledger property tests. Keep the WBS entry aligned with the
corresponding evidence path in TRACEABILITY.json without changing other paths or
status details.
🪄 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: e8e35299-6963-4742-a515-41fb287062a8

📥 Commits

Reviewing files that changed from the base of the PR and between 5d2c60c and 1f4b6c9.

📒 Files selected for processing (6)
  • CHANGELOG.md
  • crates/sl-viewer/tests/properties_session_ledger_context.rs
  • crates/sl-viewer/tests/properties_session_ledger_contract.rs
  • crates/sl-viewer/tests/properties_session_ledger_pii_redact.rs
  • docs/ops/TRACEABILITY.json
  • docs/ops/WBS.md
📜 Review details
⏰ Context from checks skipped due to timeout. (3)
  • GitHub Check: Summary
  • GitHub Check: prepare
  • GitHub Check: browser e2e · axe · responsive · visual
⚠️ CI failures not shown inline (2)

GitHub Check: Summary: The current Mergify configuration is invalid

Conclusion: failure

View job details

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

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

Conclusion: failure

View job details

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

📄 CodeRabbit inference engine (AGENTS.md)

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

Files:

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

📄 CodeRabbit inference engine (AGENTS.md)

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

Files:

  • crates/sl-viewer/tests/properties_session_ledger_context.rs
  • crates/sl-viewer/tests/properties_session_ledger_contract.rs
  • crates/sl-viewer/tests/properties_session_ledger_pii_redact.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_context.rs
  • crates/sl-viewer/tests/properties_session_ledger_contract.rs
  • crates/sl-viewer/tests/properties_session_ledger_pii_redact.rs
crates/sl-viewer/**/*.{rs,toml}

📄 CodeRabbit inference engine (AGENTS.md)

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

Files:

  • crates/sl-viewer/tests/properties_session_ledger_context.rs
  • crates/sl-viewer/tests/properties_session_ledger_contract.rs
  • crates/sl-viewer/tests/properties_session_ledger_pii_redact.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_context.rs
  • crates/sl-viewer/tests/properties_session_ledger_contract.rs
  • crates/sl-viewer/tests/properties_session_ledger_pii_redact.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 (2)
crates/sl-viewer/tests/properties_session_ledger_pii_redact.rs (1)

9-10: 📐 Maintainability & Code Quality

Run the required Rust validation before merge.

Use the toolchain pinned by rust-toolchain.toml. Run the locked workspace build, all-features test suite, Clippy, rustfmt, and cargo check -p sl-viewer. Fix every Clippy warning without an untracked #[allow].

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_context.rs (1)

12-15: 📐 Maintainability & Code Quality

Run the required Rust validation checks.

Validate both new integration-test targets with the pinned rust-toolchain.toml toolchain. Run the prescribed locked build, all-features test suite, Clippy, and rustfmt checks. Run cargo check -p sl-viewer as the viewer fast check.

  • crates/sl-viewer/tests/properties_session_ledger_context.rs#L12-L15: validate the context property target.
  • crates/sl-viewer/tests/properties_session_ledger_contract.rs#L11-L14: validate the contract property target.

As per coding guidelines, “Use the Rust toolchain pinned in rust-toolchain.toml” and “Validate Rust workspace changes with the prescribed locked build, all-features test suite, Clippy, and rustfmt checks where applicable.”

Source: Coding guidelines

Comment on lines +149 to +162
/// Every documented file extension is detected on a single test message.
#[test]
fn every_documented_file_extension_is_detected(ext in proptest::sample::select(FILE_EXTENSIONS)) {
let owned = ext.to_string();
let ext_str = owned.as_str();
let body = format!("main{ext_str}");
let messages = vec![(Role::User, body.as_str())];
let session = make_session("ext-coverage", &messages);
let ctx = HeuristicContextExtractor::extract_context(&session);
prop_assert!(
!ctx.files_mentioned.is_empty(),
"documented extension {ext} must be detected"
);
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Assert the selected pattern in each coverage property.

A non-empty output does not prove that the selected pattern matched. Overlapping patterns can make these properties pass after the selected pattern is removed.

  • crates/sl-viewer/tests/properties_session_ledger_context.rs#L149-L162: assert that main{ext} appears in files_mentioned.
  • crates/sl-viewer/tests/properties_session_ledger_context.rs#L202-L210: assert that Session contains '{pat}' language appears in key_decisions.
  • crates/sl-viewer/tests/properties_session_ledger_context.rs#L358-L366: assert that Session mentions '{pat}' appears in environment_notes.
  • crates/sl-viewer/tests/properties_session_ledger_contract.rs#L151-L190: assert the expected formatted entry in the section for each selected pattern.
📍 Affects 2 files
  • crates/sl-viewer/tests/properties_session_ledger_context.rs#L149-L162 (this comment)
  • crates/sl-viewer/tests/properties_session_ledger_context.rs#L202-L210
  • crates/sl-viewer/tests/properties_session_ledger_context.rs#L358-L366
  • crates/sl-viewer/tests/properties_session_ledger_contract.rs#L151-L190
🤖 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_context.rs` around lines 149
- 162, Strengthen the selected-pattern assertions in every listed coverage
property: in crates/sl-viewer/tests/properties_session_ledger_context.rs at
lines 149-162, assert files_mentioned contains main{ext}; at lines 202-210,
assert key_decisions contains the formatted Session contains '{pat}' language
entry; and at lines 358-366, assert environment_notes contains Session mentions
'{pat}'. In crates/sl-viewer/tests/properties_session_ledger_contract.rs at
lines 151-190, assert the expected formatted entry appears in the appropriate
section for each selected pattern.

Comment on lines +398 to +405
let a = HeuristicContextExtractor::extract_context(&session);
let b = HeuristicContextExtractor::extract_context(&session);
prop_assert_eq!(a.cwd, b.cwd);
prop_assert_eq!(a.title, b.title);
prop_assert_eq!(a.files_mentioned, b.files_mentioned);
prop_assert_eq!(a.key_symbols, b.key_symbols);
prop_assert_eq!(a.environment_notes, b.environment_notes);
prop_assert_eq!(a.key_decisions.len(), b.key_decisions.len());

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Compare complete Decision values.

Lines 405 and 430 compare only the number of decisions. A change to a decision summary or rationale can pass both properties when the count stays the same. Compare summary and rationale for each decision.

Proposed fix
-        prop_assert_eq!(a.key_decisions.len(), b.key_decisions.len());
+        prop_assert_eq!(
+            a.key_decisions.iter().map(|d| (&d.summary, d.rationale.as_deref())).collect::<Vec<_>>(),
+            b.key_decisions.iter().map(|d| (&d.summary, d.rationale.as_deref())).collect::<Vec<_>>(),
+        );

Also applies to: 422-430

🤖 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_context.rs` around lines 398
- 405, Update the property assertions around
HeuristicContextExtractor::extract_context in both repeated-extraction checks to
compare each corresponding key_decisions entry’s complete Decision values,
including summary and rationale, rather than only comparing collection lengths.
Preserve the existing assertions for the other context fields.

Comment on lines +457 to +462
if all_empty {
prop_assert!(ctx.is_empty());
}
if !ctx.is_empty() {
prop_assert!(!all_empty);
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Test both directions of the is_empty equivalence.

The !is_empty() branch is the contrapositive of the preceding all_empty branch. It does not test is_empty() => all_empty. Compare the values directly.

  • crates/sl-viewer/tests/properties_session_ledger_context.rs#L457-L462: replace both branches with prop_assert_eq!(ctx.is_empty(), all_empty).
  • crates/sl-viewer/tests/properties_session_ledger_contract.rs#L364-L369: replace both branches with prop_assert_eq!(c.is_empty(), all_empty).
📍 Affects 2 files
  • crates/sl-viewer/tests/properties_session_ledger_context.rs#L457-L462 (this comment)
  • crates/sl-viewer/tests/properties_session_ledger_contract.rs#L364-L369
🤖 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_context.rs` around lines 457
- 462, Replace the two conditional assertions in
crates/sl-viewer/tests/properties_session_ledger_context.rs:457-462 with a
direct prop_assert_eq!(ctx.is_empty(), all_empty) comparison. Apply the same
replacement in
crates/sl-viewer/tests/properties_session_ledger_contract.rs:364-369 using
c.is_empty(), so both tests verify the full equivalence in both directions.

Comment on lines +195 to +208
proptest! {
/// All patterns are detected case-insensitively.
#[test]
fn all_patterns_case_insensitive(
pat in proptest::sample::select(CRITERIA_PATTERNS),
) {
let upper = pat.to_uppercase();
let body = format!("we have a {upper} now");
let messages = vec![(Role::User, body.as_str())];
let session = make_session("criteria-case", &messages);
let c = HeuristicContractExtractor::extract_contract(&session);
prop_assert!(!c.success_criteria.is_empty(), "criteria pattern {pat} must match case-insensitively");
}
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Test case-insensitive matching for every contract section.

This property selects only CRITERIA_PATTERNS and checks only success_criteria. A case-folding regression in test, constraint, or do-not-touch extraction will pass. Add equivalent properties for TEST_PATTERNS, CONSTRAINT_PATTERNS, and DO_NOT_TOUCH_PATTERNS.

🤖 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_contract.rs` around lines
195 - 208, The case-insensitivity property currently covers only
CRITERIA_PATTERNS and success_criteria. Add equivalent property tests for
TEST_PATTERNS, CONSTRAINT_PATTERNS, and DO_NOT_TOUCH_PATTERNS, each uppercasing
the selected pattern, extracting from a session via
HeuristicContractExtractor::extract_contract, and asserting the corresponding
contract section is non-empty.

Comment on lines +176 to +185
proptest! {
/// When `redact` produces a non-empty output, it equals the input
/// length plus the difference between placeholder length and matched
/// substring length (sanity: output length is non-negative).
#[test]
fn redact_output_is_a_string(body in "[ -~]{0,40}") {
let out = redact(&body);
// Sanity: no panic, returns a String.
let _ = out.len();
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Replace the no-op assertion with an output-shape property.

This test only reads String::len(). It verifies no output contract and duplicates the no-panic coverage at lines 51-61. Assert that redact preserves text around a generated email and replaces only the email with REDACTED_EMAIL.

🤖 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_pii_redact.rs` around lines
176 - 185, Update the redact_output_is_a_string property test to generate input
containing an email surrounded by arbitrary text, then assert that redact
preserves the surrounding text and replaces only the email with REDACTED_EMAIL.
Remove the no-op out.len() check while retaining coverage for generated inputs.

Comment on lines +189 to +198
fn redact_preserves_unicode(body in "\\PC{1,40}") {
// Use a unicode string that contains no `@` and no api-key prefix.
// Simple heuristic: drop any @ and common prefixes.
let safe: String = body.chars()
.filter(|c| *c != '@')
.collect();
let out = redact(&safe);
// Length is preserved (no character is removed or added).
prop_assert_eq!(out.len(), safe.len());
prop_assert_eq!(out, safe);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Constrain this generator to text that cannot contain an API key.

\\PC{1,40} can generate ASCII. Filtering only @ still permits inputs such as sk-abcdefgh. redact correctly changes that input, but this property expects unchanged output. Generate non-ASCII text instead, or remove every recognized key prefix and token body before the assertion.

Proposed fix
-    fn redact_preserves_unicode(body in "\\PC{1,40}") {
-        // Use a unicode string that contains no `@` and no api-key prefix.
-        // Simple heuristic: drop any @ and common prefixes.
-        let safe: String = body.chars()
-            .filter(|c| *c != '@')
-            .collect();
-        let out = redact(&safe);
-        // Length is preserved (no character is removed or added).
-        prop_assert_eq!(out.len(), safe.len());
-        prop_assert_eq!(out, safe);
+    fn redact_preserves_non_ascii_text(body in "[^\\x00-\\x7F]{1,40}") {
+        prop_assert_eq!(redact(&body), body);
     }
📝 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 redact_preserves_unicode(body in "\\PC{1,40}") {
// Use a unicode string that contains no `@` and no api-key prefix.
// Simple heuristic: drop any @ and common prefixes.
let safe: String = body.chars()
.filter(|c| *c != '@')
.collect();
let out = redact(&safe);
// Length is preserved (no character is removed or added).
prop_assert_eq!(out.len(), safe.len());
prop_assert_eq!(out, safe);
fn redact_preserves_non_ascii_text(body in "[^\\x00-\\x7F]{1,40}") {
prop_assert_eq!(redact(&body), body);
🤖 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_pii_redact.rs` around lines
189 - 198, Update the redact_preserves_unicode generator so its inputs cannot
match recognized API-key patterns, preferably by constraining generated text to
non-ASCII characters; keep the existing @ filtering and unchanged-output
assertions for the resulting safe string.

Comment thread docs/ops/WBS.md Outdated
| 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`; `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 |
| 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`; `crates/sl-viewer/tests/properties_session_ledger_context.rs`; `crates/sl-viewer/tests/properties_session_ledger_pii_redact.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; session-ledger heuristic context extractor properties #464; session-ledger heuristic contract extractor properties #465; session-ledger pii redact properties #473; full loom/shuttle unpaid |

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win

Add the contract property test to the WBS evidence paths.

This list includes properties_session_ledger_context.rs and properties_session_ledger_pii_redact.rs, but omits crates/sl-viewer/tests/properties_session_ledger_contract.rs. docs/ops/TRACEABILITY.json includes that path. Keep both WBS-6.2 evidence lists aligned.

🧰 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 evidence path list to include
crates/sl-viewer/tests/properties_session_ledger_contract.rs alongside the
existing session-ledger property tests. Keep the WBS entry aligned with the
corresponding evidence path in TRACEABILITY.json without changing other paths or
status details.

KooshaPari pushed a commit that referenced this pull request Aug 12, 2026
Shards the unique viewer proptest from #492 onto main without the
obsolete pre-rename daemon source churn.
@KooshaPari
KooshaPari force-pushed the fix/viewer-pii-redact-properties-20260809 branch from 1f4b6c9 to d481319 Compare August 12, 2026 08:21
@codeant-ai

codeant-ai Bot commented Aug 12, 2026

Copy link
Copy Markdown

Thanks for using CodeAnt! 🎉

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

Share on X ·
Reddit ·
LinkedIn

@codeant-ai codeant-ai Bot added size:L This PR changes 100-499 lines, ignoring generated files and removed size:XXL This PR changes 1000+ lines, ignoring generated files labels Aug 12, 2026
@KooshaPari KooshaPari closed this Aug 12, 2026
@KooshaPari
KooshaPari force-pushed the fix/viewer-pii-redact-properties-20260809 branch from d481319 to f445859 Compare August 12, 2026 08:23
@KooshaPari
KooshaPari deleted the fix/viewer-pii-redact-properties-20260809 branch August 12, 2026 08:38
@kilo-code-bot

kilo-code-bot Bot commented Aug 12, 2026

Copy link
Copy Markdown

Code Review Summary

Status: No Issues Found | Recommendation: N/A (PR is closed; no diff between head and base)

Files Reviewed (0 files)
  • No changed files: headRefOid (f445859c) equals baseRefOid (f445859c), so there is no diff to review.

Reviewed by step-3.7-flash · Input: 36.3K · Output: 2.7K · Cached: 211.7K

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

Labels

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants