Skip to content

perf(sessions): skip unchanged quarantined Pi headers - #4734

Closed
ScriptedAlchemy wants to merge 1 commit into
masterfrom
perf/pi-headers-unchanged-quarantine
Closed

ScriptedAlchemy wants to merge 1 commit into
masterfrom
perf/pi-headers-unchanged-quarantine

Conversation

@ScriptedAlchemy

Copy link
Copy Markdown
Owner

Root cause

Refs #4481 (repeated Pi discovery on invalid session headers).

The existing durable PathMemo records invalid Pi headers and already prevents repeated admission, but discovery opens and parses each unchanged invalid file before that memo can be consulted. The shared per-source ledger could skip the read, but its old boolean "already seen" result did not distinguish a fully covered file from a quarantined one. Blindly skipping invalid inputs would turn partial coverage into false success.

Change

  • Keep the canonical durable Pi memo as the authority for invalid-header dispositions. Only after an existing matching memo or a successful compare-and-swap of a newly revalidated invalid header do we seed the shared file-witness quarantine.
  • Expose the existing ledger's skipped-file disposition to Pi discovery. A matching quarantined witness is skipped without reopening/parsing the header, while still incrementing quarantined_headers and preserving partial provider coverage.
  • A changed settled file witness no longer matches that entry and is inspected/admitted normally. No new store, timer, file format, source cursor, admission bypass, or error suppression.
  • Preserve behavior of other ledger-backed hosts: ordinary observe_file_blocking still returns None for already-seen files.

Regression

The new Pi test uses a real initialized discovery hub, actual malformed JSONL file, durable in-memory host admission, repeated scanner/capture paths, and then a repaired valid Pi fixture. It checks first refusal, subsequent skip without header-parsing evidence, continued partial coverage, and normal recovery to the nine retained observations.

Validation and scope

The isolated three-file patch was reviewed against the current master code, including the source-claim and memo CAS boundaries; the resulting commit is a non-force fast-forward on a new branch. Rust compilation, formatting and regression execution have not run because this environment has no supplied repository checkout, Bazel or Rust toolchain. Hosted CI must execute the owning sessions tests on this exact PR head before merging. No CPU or file-open benchmark has been run; this is source-confirmed redundant work, not measured overall idle-CPU improvement.

This improves repeated malformed-header scanning only while the discovery hub retains its in-memory ledger. After daemon restart the durable memo continues to prevent repeated admission, but the first discovery still inspects an invalid header once before warming the ledger. #4481 remains open for its separate scheduler CPU, storage/corruption, resource-usage and end-to-end macOS acceptance. It is not a full resolution.

The separate PR-repair automation owns CI repair and merges; this issue sweep does not modify its branches.

Preserve the exact file-witness quarantine disposition in the shared source
ledger. Once durable Pi memo admission confirms an invalid header, avoid
reopening it on later discovery passes without treating skipped evidence as
complete coverage. A changed file witness re-enables header validation.

Add a real Pi discovery and capture regression for skip, partial coverage,
and recovery on a repaired file.

Refs #4481
@changeset-bot

changeset-bot Bot commented Nov 3, 2026

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: 7ee5d21

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@ScriptedAlchemy

Copy link
Copy Markdown
Owner Author

Consolidated into #4742 (integrate/open-pull-requests) at the captured head shown in its description, as requested. Continue review and further fixes on #4742. This closure does not assert CI readiness; the replacement remains draft. Source branch retained.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant