Skip to content

Tracking: PET reliability, performance, architecture, and coverage audit implementation plan #528

Description

Goal

Implement the September 21, 2026 PET performance, architecture, refactoring, and test-coverage audit in an evidence-driven order: fix demonstrated reliability failures first, make measurements reflect client behavior next, then simplify and bound concurrency. Do not start with a broad rewrite or an async-runtime migration.

This plan links 12 newly filed implementation issues, reuses #525 for the UTF-8 panic, and tracks #522 as an existing quality-workflow prerequisite.

Evidence and audit scope

The audit examined 4e523bad8bb9be8a84c01800a3e400a6a771cf8e. At filing, main is d586c60ec6b7191256fa3f47a2c866cc5739914e; the intervening change only removes redundant Conda vector drains and does not address these findings.

  • Windows default-feature validation: cargo test --workspace --offline --locked passed 647 tests, with 0 failures and 2 ignored documentation examples. Quality-tooling Python tests: 47 passed.
  • Bounded reproductions confirmed interpreter pipe backpressure, non-UTF-8 worker panic, EOF busy-looping, request-ID/framing defects, and refresh glob expansion blocking the dispatcher. Local timings are Windows debug diagnostics, not release budgets.
  • Exact-revision performance artifacts and coverage artifacts support the measurement/coverage findings. Source-level architectural risks and optimization opportunities are explicitly distinguished from reproduced failures in each ticket.

Existing work: reuse, do not duplicate

#525 / PR #526: existing non-UTF-8 interpreter-output fix. Review/validate and integrate it before overlapping subprocess-runner changes.

#522 / PR #524: existing fork-safe quality-workflow fix. Complete it so fork contributions execute the substantive quality gates. Runtime fixes do not need to wait for comment-publishing changes; never weaken the gates or grant untrusted PRs write credentials as a workaround.

Ordered implementation plan

Order below is the recommended landing sequence, not a requirement to serialize independent investigation or test preparation. P1/P2/P3 are relative priorities within this audit, not estimates of effort. The dependency column distinguishes required foundations from coordination.

Phase 1 - Reliability boundaries

Order Priority Individual issue Prerequisite / coordination Exit evidence
1 P1 #525 - Handle non-UTF-8 interpreter output without panic (existing; PR #526) Existing implementation; no competing ticket Malformed startup output produces a defined result/error, not a panic or abandoned RPC
2 P1 #529 - Exit cleanly on stdin EOF None; can proceed alongside #525 Normal disconnect exits promptly without CPU spin/error flooding; teardown is bounded
3 P1 #530 - Drain subprocess pipes and standardize probe deadlines Coordinate after #525 / #526 in the shared probe code Healthy noisy interpreters finish; deadlines, bounded capture, and cleanup are tested on Windows/Unix
4 P2 #532 - Preserve request IDs and parse bounded multi-header frames #529 transport/test seams String/large IDs round-trip; fragmented/extra-header/invalid/oversized frames have explicit outcomes

Phase exit: reproduced process/transport failures are covered by deterministic regressions; normal request behavior, streaming, and locator priority remain intact.

Phase 2 - Trustworthy measurements, responsiveness, and coverage

Order Priority Individual issue Prerequisite / coordination Exit evidence
5 P1 #531 - Gate client-observed latency and define TTFE boundaries Can start alongside Phase 1; coordinate quality jobs with #522 Pre-discovery/queueing costs appear in gated metrics; schema transition and comparator tests preserve exact-base checks
6 P1 #535 - Move glob expansion off dispatch and bound traversal Measure before/alongside with #531 A blocked expansion does not block a later lightweight RPC; coalescing and error semantics remain correct
7 P1 #534 - Collect subprocess profiles and report production-focused coverage #529; coordinate transport tests with #532 and fork CI with #522 Exercised handlers/server loop/writers register coverage; production/diff and platform gaps are visible
8 P2 #533 - Add same-process, truly concurrent, and scaling workloads #531 and #529; reuse #530 probe fixtures/cleanup Stable identity-based inventory plus client latency/resource measurements across increasing sizes and long-lived churn

Phase exit: the roughly 400 ms client / 2 ms reported-duration reproduction is no longer invisible to the performance gate; black-box server testing contributes usable coverage; repeatable workloads establish a baseline for architectural changes.

Phase 3 - Coherent ownership, bounded work, and focused refactoring

Order Priority Individual issue Prerequisite / coordination Exit evidence
9 P2 #536 - Give find/resolve coherent configuration snapshots #533 regression harness and #531 metrics; the small find-lock fix may land earlier if isolated Barrier-driven tests prove old-or-new, never mixed, configuration and no global lock over discovery I/O
10 P2 #539 - Bound scheduling and share failed in-flight probes #530, #533, #536; integrate #535 traversal work Worker/process/queue bounds hold; same-key failures are shared only with current waiters and later calls can retry
11 P2 #540 - Keep output backpressure outside state locks #529/#532 transport seams, #533, #536; coordinate queue policy with #539 Slow consumers do not pin configuration/deduplication locks; buffering, generation filtering, and notification/reply ordering remain correct
12 P3 #537 - Profile Poetry lookup and introduce an alias index only if justified #533 and #536 Before/after operation/allocation measurements justify the optimization, or the issue is explicitly deferred with evidence
13 P3 #538 - Consolidate library module ownership and split orchestration responsibilities #534, #536, #539, #540; guard with #531/#533 Behavior-preserving components replace duplicate module instances and tangled responsibilities without coverage/latency regression

Phase exit: ownership/resource limits are explicit and tested. Optimizations are supported by measurements. The locator framework and public behavior are preserved rather than replaced wholesale.

Parallel work and review boundaries

Transport shutdown/framing and subprocess-runner work can use separate PRs; coordinate the latter with #526. Metric/coverage test preparation can run alongside those fixes. Snapshot, scheduler, and writer changes should not be merged as one large refactor: establish ownership first, then independently validate scheduling and output behavior. Poetry profiling can run in parallel once the workload harness is ready.

Purely mechanical library-module reuse or the find read-guard fix may be isolated earlier; do not smuggle behavioral changes into those cleanups. Do not delay demonstrated reliability fixes for the optional Poetry optimization or final module cleanup.

Shared implementation and verification contract

Each ticket includes its own scope and acceptance tests. Implementations must preserve locator ordering, complete environment/manager information, platform path/symlink behavior, refresh coalescing, generation filtering, and scoped state synchronization. Use explicit errors, not success-shaped fallback data. Measure intentional metric/protocol changes and update directly related documentation.

Run targeted tests first, then relevant workspace/platform/feature jobs. Before committing Rust changes, run cargo fmt --all and cargo clippy --all -- -D warnings; CI's all-targets/all-features lint and the existing exact-base quality gates must also remain green. Concurrency tests should use deterministic barriers/channels rather than timing guesses. Test fixtures must bound and clean up their own subprocesses and files.

Milestones

Close this tracking issue only when each linked item is complete or explicitly deferred with a recorded reason. Check issue/PR state when starting work; links above describe filing-time status, not a substitute for current GitHub state.

Activity

  1. karthiknadig commented on Sep 21, 2026

    @karthiknadig
    MemberAuthor

    Implementation progress (local worktrees, not yet published)

    The first independent batch is implemented in separate worktrees based on 114686c. Existing PRs #526 (for #525) and #524 (for #522) were left untouched. No audit issue is being closed by this update.

    Issue Local branch Verified result Remaining gate
    #529 fix/audit-529-eof Clean EOF, typed transport failures, tracked request workers, and safe draining. Full workspace: 651 passed / 0 failed / 2 ignored; after restart, 67 focused tests passed again. A real delayed interpreter completed and was reaped before PET exited successfully. Bounded cancellation of genuinely hung external commands is not solved. This foundation deliberately waits rather than orphaning children; complete that acceptance item with #530/#539 and run Linux/macOS CI.
    #531 fix/audit-531-client-metrics Schema-v3 client round-trip and request-relative TTFE, retained discovery/startup metrics, explicit exact-base transition. 7 Rust timing/client tests and 52 Python quality-tooling tests passed. Comparable repeated Linux/Windows/macOS release baselines are still required for calibration. Existing tolerances are not evidence that new metrics have been calibrated.
    #535 fix/audit-535-glob-dispatch Off-dispatcher expansion, bounded traversal/admission, expanded-path coalescing, and explicit errors. Full workspace: 663 passed / 0 failed / 2 ignored. Real JSONRPC fixture returned exactly three expected environments for duplicate globs, rejected malformed/over-limit patterns with -4, and remained usable for info requests. Linux/macOS CI and integration with #531 metrics; when stacked after #529, retain transport-owned workers rather than reintroducing detached refresh workers.

    All three worktrees pass Windows cargo fmt --all -- --check, cargo clippy --offline --locked --workspace --all-targets --all-features -- -D warnings, and git diff --check.

    Review corrections completed

    • Removed a proposed EOF grace-period exit after reproducing that it orphaned a still-running interpreter. The remaining hung-process cancellation requirement is explicitly documented rather than claimed complete.
    • Fixed glob brace compatibility, case-sensitive Windows wildcard identity, trailing directory-only pattern identity, and glob/literal equivalence for refresh coalescing.
    • Added differential tests against the pinned glob implementation, including terminal **/**/, mixed brace/wildcard forms, dot directories, and 49 two-component combinations. The actual pinned implementation, not an assumption about shell globbing, is the compatibility oracle.
    • Requests without wildcard traversal bypass saturated glob slots. Directory traversal and brace expansion are iterative; intermediate brace work has an explicit cap. Errors never publish a partial refresh inventory/configuration.

    Changes are saved but uncommitted and unpushed. The original checkout remains unchanged. Branches have been validated independently; final stacking/rebase review is still needed before merging overlapping handler changes. Next steps remain publication/CI, the #529 cancellation dependency, and #531 cross-platform calibration rather than starting the measurement-gated optimization/refactor phase prematurely.

  2. karthiknadig commented on Sep 21, 2026

    @karthiknadig
    MemberAuthor

    Published the #535 implementation as draft PR #541 from fix/audit-535-glob-dispatch, commit 56a0d620a56a92538f9fca2c3ba2d436c5820ae1.

    Final review caught swallowed metadata errors; those were fixed and the Reviewer follow-up confirmed resolution. Final Windows validation: 664 workspace tests passed, 0 failed, 2 ignored documentation examples, repository precommit checks and all-target/all-feature Clippy passed. The PR documents intentional glob limits, explicit-error semantics, and OS-call cancellation limitations.

    The draft is pending cross-platform CI, quality-snapshot inspection, and review. #529 and #531 remain in their separate local worktrees and are not included or closed. No merge or auto-merge has been requested.

  3. karthiknadig commented on Sep 21, 2026

    @karthiknadig
    MemberAuthor

    Parallel work update

    #529 stays unpublished until the subprocess cancellation/cleanup dependency is safe. Work continues in separate branches; the original checkout is unchanged.

  4. karthiknadig commented on Sep 24, 2026

    @karthiknadig
    MemberAuthor

    Progress checkpoint: #531 is closed after acceptance-by-acceptance verification of the merged client metrics and 21 Rust/48 comparator tests. #555 (bounded Conda/Poetry probes) is now ready with verified squash auto-merge, a clean current-head Copilot re-review, all 35 automated checks passing, improved coverage, and a documented same-host macOS quality control. It is still waiting for required approval/the external VS Code PR Check; it has not merged. The remaining process-tree/active-shutdown work under #530/#529 is not being marked complete by this manager slice. #520 also remains open because the historical signed Azure artifact/signing logs needed for provenance verification are not accessible in this session. No incomplete tracking issue has been closed.

  5. karthiknadig commented on Sep 28, 2026

    @karthiknadig
    MemberAuthor

    #559 and #560 are merged; #529 and #532 are closed. Work is now proceeding in isolated parallel branches on #534 (coverage integrity/reporting, including native macOS) and #533 (long-lived deterministic session/scale workloads). The #536/#539/#540 ownership/concurrency changes remain behind those measurement prerequisites; no issues are being closed merely because partial prerequisites landed. Existing signing work remains separate, with no signing/release services invoked and no updates posted on #520.

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    debtCode quality issues

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions