Skip to content

Type-soundness and formatting cleanup on top of #80#81

Open
scottmessinger wants to merge 1 commit into
copilot/reduce-kernel-allocationsfrom
claude/fix-base-correctness
Open

Type-soundness and formatting cleanup on top of #80#81
scottmessinger wants to merge 1 commit into
copilot/reduce-kernel-allocationsfrom
claude/fix-base-correctness

Conversation

@scottmessinger
Copy link
Copy Markdown
Member

Summary

Correctness and formatting cleanup on top of the kernel-allocations work in #80. No perf changes — strictly type-soundness, formatting, and one accidental indentation regression.

What this fixes

1. as any casts regressing #76's type-soundness sweep

Three casts to any were introduced when adding intrusive symbol storage:

  • packages/kernel/src/collections.tscreateReactiveMap and createReactiveSet use (rawTarget as any)[$PROXY] to read the intrusive cache.
  • packages/kernel/src/read.tsgetMutatorCache uses (target as any)[$MUTATORS].

The codebase already has a ReactiveTagged interface in core.ts for exactly this purpose, and #76 deliberately removed as any from this surface. Reintroducing them here was a regression.

Fix: route all three through ReactiveTagged, and add [$MUTATORS]? to the interface so the cast resolves to a real type instead of needing another as any to land somewhere.

2. packages/js-krauset/package.json indent regression

The base reformatted this file from 2-space to 4-space indentation. Every other package.json in the workspace uses 2-space, so this looks accidental. Reverted.

3. perf-stats.ts formatting

Two stats(...) calls added with --trim exceed the line width. pnpm format reformats them on a clean run, so committing them in formatted shape avoids future churn.

What this is not

  • No perf changes. No allocation refactors. No structural rewrites.
  • The intrusive-symbol-cache work, the per-array $MUTATORS cache, the pre-created Map/Set methods — all kept exactly as the base has them.

Validation

  • pnpm test — kernel 175 / mill 52 pass
  • pnpm run typecheck — clean
  • pnpm lint — clean
  • pnpm format — clean (no further reformatting needed)
  • pnpm run test:validate — clean

https://claude.ai/code/session_01VDVFSh8xJqvYp61dZQ8q4o


Generated by Claude Code

Three issues introduced by the kernel-allocations PR (commits 11bbbee,
94dba65, 08a1f25), no perf changes:

1. Replace `(target as any)[…]` casts with proper typing through
   `ReactiveTagged`:
   - `collections.ts`: two casts in `createReactiveMap` /
     `createReactiveSet` for the intrusive `$PROXY` lookup.
   - `read.ts`: one cast in `getMutatorCache` for the `$MUTATORS`
     lookup.
   These regressed the type-soundness sweep landed in #76. Fixing them
   required adding `[$MUTATORS]?` to the `ReactiveTagged` interface in
   `core.ts` so the cast resolves to a real type.

2. `package.json`: revert the accidental 2-space → 4-space indent in
   `packages/js-krauset/package.json` (the rest of the workspace's
   package.json files use 2-space).

3. `perf-stats.ts`: oxfmt formatting cleanup of two `stats(...)` calls
   added with the `--trim` feature in 08a1f25 — they exceeded the line
   width and `pnpm format` reformats them on a clean run.

No behavior changes, no perf claims.
Copilot AI review requested due to automatic review settings May 1, 2026 14:15
@codecov
Copy link
Copy Markdown

codecov Bot commented May 1, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
⚠️ Please upload report for BASE (copilot/reduce-kernel-allocations@08a1f25). Learn more about missing BASE report.

Additional details and impacted files
@@                         Coverage Diff                          @@
##             copilot/reduce-kernel-allocations      #81   +/-   ##
====================================================================
  Coverage                                     ?   99.46%           
====================================================================
  Files                                        ?       31           
  Lines                                        ?     1317           
  Branches                                     ?      260           
====================================================================
  Hits                                         ?     1310           
  Misses                                       ?        2           
  Partials                                     ?        5           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

This PR removes newly introduced as any casts by routing intrusive symbol-cache access through the existing ReactiveTagged type, and applies formatting-only fixes (JSON indentation + line wrapping) to reduce churn.

Changes:

  • Replace as any access to [$PROXY] / [$MUTATORS] with ReactiveTagged-typed access and extend ReactiveTagged to include [$MUTATORS]?.
  • Reformat perf-stats.ts to match the formatter’s wrapping for long stats(...) calls.
  • Revert packages/js-krauset/package.json to consistent 2-space indentation.

Reviewed changes

Copilot reviewed 4 out of 5 changed files in this pull request and generated no comments.

Show a summary per file
File Description
packages/kernel/src/read.ts Removes as any from $MUTATORS access by using ReactiveTagged.
packages/kernel/src/core.ts Extends ReactiveTagged to include optional [$MUTATORS] for type-safe symbol access.
packages/kernel/src/collections.ts Removes as any from $PROXY intrusive cache reads via ReactiveTagged.
packages/js-krauset/perf-stats.ts Wraps long stats(...) calls to match formatter output and avoid future reformat churn.
packages/js-krauset/package.json Restores workspace-consistent 2-space JSON indentation.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

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.

3 participants