Eliminate static-analysis nullability and dead-code findings

Merged
UN
3mo ago
+340-159

What changed

  • Removed the supplied redundant graph option spreads while preserving exact optional-property behavior.
  • Replaced analyzer-opaque null checks with record-returning validation guards.
  • Made Sentry, telemetry, and contracts tests fail fast before property access.
  • Removed stale imports, declarations, parameters, arrays, and obsolete bootstrap arguments.
  • Extended the review-loop helper so non-threadable bot feedback can be reacted to and acknowledged with a linked PR comment.
  • Pruned 13 obsolete ESLint suppressions and lowered the enforced ceiling from 127 to 114.

Why

DeepScan and CodeFactor reported nullability, unnecessary allocation, dead-code, and stale-signature findings across runtime and test surfaces. The fixes remove the findings without suppressions or weaker assertions and make future bot-review acknowledgement auditable.

Verification

  • Focused regression suite: 674 passed
  • Full coverage: 6,800 passed
  • Coverage: 100% statements, branches, functions, and lines
  • pnpm typecheck
  • pnpm lint, including static quality, SDK entrypoint costs, and CLI transport floor
  • PM history drift: 0; storage integrity: ok

PM tracking

The links target main so they remain valid after merge.

Summary by Sourcery

Consolidate runtime and test nullability checks, tighten static-analysis gates, and extend the PR review loop helper with auditable acknowledgement workflows.

New Features:

  • Add reusable item metadata record guard to validate object-shaped inputs before field assertions.
  • Extend PR review loop helper with top-level comment and acknowledgement commands that both react to bot feedback and emit linked PR comments describing the decision.

Bug Fixes:

  • Prevent analyzer-opaque null and type assertions in Sentry, telemetry, and contracts tests by failing fast when expected options, requests, or errors are missing or of the wrong type.
  • Simplify graph centrality and articulation invocations by removing redundant options spreading that confused static analyzers.
  • Ensure validate-command tests rely on deterministic closed-item counts instead of accumulating unused IDs.

Enhancements:

  • Tighten static quality thresholds by reducing the maximum allowed ESLint suppressions and updating release automation contracts accordingly.
  • Clarify PR review loop documentation to describe how to acknowledge non-threadable bot feedback with explicit, linked PR comments.
  • Remove unused imports, types, helpers, and obsolete test scaffolding flagged as dead code by static analysis.

Summary by cubic

Cleans up static-analysis findings by fixing nullability issues and removing dead code, makes PR review acknowledgements retry-safe with idempotent top‑level comments, and updates the changelog to record the remediation closeout. Tightens the ESLint suppression ceiling to 114.

  • New Features

    • Added comment and acknowledge commands to scripts/reviews/pr-review-loop.mjs for non-threadable PR feedback; acknowledgements use hidden markers to dedupe on retry, attempt comment and reaction writes independently, and emit clear partial‑completion receipts.
    • Updated docs to show the full acknowledgement workflow, clarify bot identity, and document the implemented‑or‑declined audit contract.

  • Bug Fixes

    • Replaced opaque null checks with record-returning guards in item metadata validation.
    • Switched redundant option spreads to direct conditional option objects in graph centrality/articulation.
    • Hardened Sentry, telemetry, and contracts tests to fail fast before property access.
    • Removed stale imports, parameters, arrays, and obsolete bootstrap arguments, and pruned 13 obsolete ESLint suppressions.