Skip to content

improve compile perf - #6804

Closed
benedikt-bartscher wants to merge 13 commits into
reflex-dev:mainfrom
benedikt-bartscher:improve-compile-perf
Closed

improve compile perf#6804
benedikt-bartscher wants to merge 13 commits into
reflex-dev:mainfrom
benedikt-bartscher:improve-compile-perf

Conversation

@benedikt-bartscher

Copy link
Copy Markdown
Contributor

No description provided.

@greptile-apps

greptile-apps Bot commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR accelerates the deterministic hashing used for compiler auto-memoization by replacing per-node hasher updates with a buffer-based accumulation strategy and a memoized, per-type encoder dispatch table. It also fixes a latent correctness issue where components that inherit from a dataclass (e.g., rx.text via MarkdownComponentMap) were hashed against their (often empty) field list rather than their rendered content.

  • Encoder table + bytearray buffer: _update_deterministic_hash (one hasher.update per tree node) is replaced by _encode_deterministic which fills a bytearray, flushed into the MD5 hasher once per top-level value. A per-type _ENCODERS dict is populated lazily so subsequent calls avoid isinstance chains entirely.
  • Frozen-dataclass identity cache: _ENCODED_DATACLASSES (an OrderedDict with bounded capacity) caches the encoded bytes of frozen dataclass instances whose fields are all immutable scalars, keyed by object identity. A strong reference in the cache entry prevents id reuse while the entry lives.
  • dataclasses.fields() caching: _dataclass_fields_to_encode wraps the per-call tuple rebuild in @functools.cache, eliminating millions of redundant allocations on large apps.

Confidence Score: 5/5

  • Safe to merge — the hash encoding is functionally equivalent to the old path for all existing types, the correctness fix for dataclass-inheriting components is well-targeted, and the 18 new tests cover the edge cases thoroughly.
  • The buffer-based encoding produces byte-for-byte identical output to the old incremental hasher for every supported type. The encoder table's overwrite ordering is intentional and correct: _encode_frozen_dataclass downgrades its own _ENCODERS entry on the first encounter of a mutable-field type, and the entry is not overwritten a second time because the memoization block only runs when the encoder came from _resolve_encoder, not from the table. The frozen-dataclass identity cache is bounded in both entry count and per-entry size, and strong references prevent stale id lookups. The _get_component_hash refactor preserves the original value ordering and the semantics of the shallow/deep split.
  • No files require special attention.

Important Files Changed

Filename Overview
packages/reflex-base/src/reflex_base/components/component.py Replaces the incremental _update_deterministic_hash approach with buffer-based _encode_deterministic, adds a type-keyed encoder table (_ENCODERS) with per-type memoization, caches dataclasses.fields() results via @functools.cache, and adds an identity-keyed encoding cache (_ENCODED_DATACLASSES) for frozen dataclass instances with all-scalar fields. Also fixes a pre-existing ordering bug where components inheriting from a dataclass were hashed by their (often empty) field list instead of their rendered content. The encoder table overwrite ordering is intentional and correct — _encode_frozen_dataclass downgrades its own entry on the first encounter of a mutable-field type, and the downgrade persists from the second call onwards. Logic is sound with no correctness regressions.
tests/units/components/test_component.py Adds 18 focused tests covering: collision resistance, dict-order independence, subclass normalization, Var data inclusion, cache eviction (oldest-first), size cap, frozen-dataclass identity reuse, mutation tracking for non-cacheable types, MutableProxy-style synthesized classes, component-vs-dataclass branch ordering, and _get_component_hash lifecycle-hook sensitivity. The encoding_caches fixture correctly isolates module-level _ENCODERS and _ENCODED_DATACLASSES state for tests that need it. Test test_deterministic_hash_handles_dataclasses_without_params references _HashFrozenScalars defined later in the file, which is valid in Python (function-body lookup is deferred to call time) but may surprise readers.
packages/reflex-base/news/6804.performance.md Changelog entry accurately describing the 3.7× speedup on large page hashing, the per-type encoder table, identity-keyed frozen-dataclass encoding reuse, and the fix for MarkdownComponentMap-inheriting components.

Reviews (11): Last reviewed commit: "Merge remote-tracking branch 'upstream/m..." | Re-trigger Greptile

Comment thread packages/reflex-base/src/reflex_base/components/component.py Outdated
@codspeed-hq

codspeed-hq Bot commented Jul 21, 2026

Copy link
Copy Markdown

Merging this PR will improve performance by 5.57%

⚠️ Different runtime environments detected

Some benchmarks with significant performance changes were compared across different runtime environments,
which may affect the accuracy of the results.

Open the report in CodSpeed to investigate

⚡ 3 improved benchmarks
✅ 24 untouched benchmarks
⏩ 8 skipped benchmarks1

Performance Changes

Mode Benchmark BASE HEAD Efficiency
Simulation test_compile_all_artifacts[_stateful_page] 27 ms 25.4 ms +6.35%
Simulation test_compile_page[_stateful_page] 30.6 ms 28.9 ms +5.8%
Simulation test_compile_page_full_context[_stateful_page] 34.6 ms 33.1 ms +4.58%

Tip

Curious why performance improved? Comment @codspeedbot explain why performance improved on this PR, or directly use the CodSpeed MCP with your agent.


Comparing benedikt-bartscher:improve-compile-perf (a642daa) with main (f7c848f)2

Open in CodSpeed

Footnotes

  1. 8 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

  2. No successful run was found on main (45b8ed5) during the generation of this report, so f7c848f was used instead as the comparison base. There might be some changes unrelated to this pull request in this report.

@benedikt-bartscher
benedikt-bartscher marked this pull request as ready for review July 21, 2026 11:09
@benedikt-bartscher
benedikt-bartscher requested a review from a team as a code owner July 21, 2026 11:09

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

All reported issues were addressed across 2 files

Reply with feedback, questions, or to request a fix.

Fix all with cubic | Re-trigger cubic

Comment thread packages/reflex-base/src/reflex_base/components/component.py

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

2 issues found across 2 files (changes from recent commits).

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="tests/units/components/test_component.py">

<violation number="1" location="tests/units/components/test_component.py:2475">
P3: This test's name and docstring claim it verifies that the frozen-dataclass encoding cache is reused by identity, but the two equality assertions would pass even if no cache existed at all (equal-but-distinct instances always encode to the same bytes). Either rename/reword it as a plain equality contract test, or actually exercise the cache path, e.g. clear component._ENCODED_DATACLASSES, hash `shared` once, then assert id(shared) is present in the cache and that a second hash of the same object short-circuits.</violation>
</file>

<file name="packages/reflex-base/src/reflex_base/components/component.py">

<violation number="1" location="packages/reflex-base/src/reflex_base/components/component.py:688">
P2: The new `_ENCODED_DATACLASSES` cache pins up to 8192 frozen dataclass instances and copies of their encoded bytes in a module-global for the whole process lifetime, released only by a full `clear()` once the cap is hit. Because `_deterministic_hash` runs for every component hash during a compile, these scalar-only frozen instances (and their otherwise-transient byte encodings) no longer get garbage-collected, adding persistent memory that was not retained before. The full-clear-on-capacity eviction also drops the entire cache at once and forces re-encoding of the whole working set right at the boundary, so the cache both holds memory and thrashes. Consider evicting/limiting per-entry (e.g. only cache the bytes keyed by object identity while bounding total retained bytes, or evict entries gradually instead of a wholesale clear).</violation>
</file>

Reply with feedback, questions, or to request a fix.

Fix all with cubic | Re-trigger cubic

Comment thread packages/reflex-base/src/reflex_base/components/component.py Outdated
Comment thread tests/units/components/test_component.py

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

All reported issues were addressed across 3 files (changes from recent commits).

Tip: Review your code locally with the cubic CLI to iterate faster.

Re-trigger cubic

Comment thread tests/units/components/test_component.py
benedikt-bartscher and others added 5 commits August 21, 2026 00:38
…t it did (reflex-dev#6948)

* ENG-11237 feat(hosting-cli): report why a deploy failed, not just that it did

The watch loop decided everything by substring against a bare status string,
and a build failure printed two warnings: the raw status, and an unconditional
pointer at `reflex cloud apps build-logs`. A generic failure printed the status
alone and nothing else.

That pointer was unconditional because there was nothing to condition it on.
The server classifies every failure as the app's, the platform's, or
transient, but that classification never reached a client -- so a failure in
the build pipeline arrived dressed as a build failure and sent people looking
for a bug in an app that did not have one.

The failure arms now fetch GET /deployments/{id}/failure and print the
recorded reason, the guidance for that fault, and the end of the build log
when the code is one the log explains. The excerpt goes through
console.print(markup=False): it is raw build output, and rich would read its
paths and version specifiers as markup.

Every way of not getting an answer is one case -- a server predating the
endpoint 404s, an older self-hosted one may not route it, the network may be
down -- and all three fall back to exactly what the arm printed before, so a
new CLI against an older server is unchanged.

* Strip terminal controls from the excerpt, and file the news fragment per package

Two fixes from review.

The excerpt is raw build output -- the user's own dependencies and build
scripts -- and it is now printed without anyone asking, on any failed deploy,
where before it took an explicit `reflex cloud apps build-logs`. markup=False
stops rich reading the text as its own markup and does nothing about escape
sequences, so OSC 52 could write the reader's clipboard, OSC 8 could render
one destination and link to another, and CSI could erase the lines above it
and leave "build succeeded" on screen. Colour is not worth carrying for
output shown unsolicited.

The changelog job runs towncrier per affected package, so a fragment for a
change under packages/reflex-hosting-cli/src has to live in that package's
own news directory, not the repository root's.

* Widen the escape class, and let a malformed answer fall back like any other

Two review findings, both narrow and both real.

The two-character escape class covered ESC + 0x40-0x5F, so a sequence whose
final byte falls outside it -- `\x1b7` (DECSC), `\x1bc` (a full terminal
reset) -- had its ESC removed by the bare-control catch-all and printed the
final byte as a stray character. Inert, since the ESC is what drives the
terminal, but it is garbage in an excerpt whose whole job is to be read. The
general ECMA-48 shape covers them.

`response.json()` raises UnicodeDecodeError on a 2xx body in an encoding
httpx cannot decode, and that is a ValueError rather than a JSONDecodeError,
so it escaped the fallback and would have ended the watch over a malformed
answer to a request whose contract is that not getting one costs nothing. The
excerpt's type is checked for the same reason: the CLI ships apart from the
control plane and talks to self-hosted ones.

* Report a build log the server could not read, rather than passing over it

The failure endpoint now separates an unreadable log from a build that stored
none. Collapsing the two tells somebody their build produced no log when the
store was simply down, so the two get different answers here.

* Assert the no-log path offers no build log, not just no outage message

The test claimed the reason stands alone and only checked the outage wording.
Offering the command is what separates this path from the unreadable one, so
that is what has to be absent. Verified by mutation: forcing the offer fails
this test and nothing else.

* Fall back however the failure body is malformed

RecursionError is a RuntimeError, so a deeply nested document escaped the
ValueError catch and aborted the deploy watch -- over an answer this function
is contracted to treat as no answer at all. Parametrized with the
UnicodeDecodeError case, since they are one rule.
@masenf

masenf commented Aug 28, 2026

Copy link
Copy Markdown
Collaborator

seems i haev some overlap here with #6947

@benedikt-bartscher

Copy link
Copy Markdown
Contributor Author

seems i haev some overlap here with #6947

yeah, maybe we can try to get best of both worlds?

masenf added a commit that referenced this pull request Sep 1, 2026
…g collisions (#6947)

* perf(compiler): speed up _update_deterministic_hash ~2.5x

The hash feeds every value one `hasher.update()` call at a time and walks
the full `isinstance` ladder per node, so a single component hash costs
tens of thousands of C calls. On a foreach/cond-heavy page,
`_get_component_hash` is ~50% of compile wall time.

Encode into a `bytearray` flushed to the hasher in 64KB chunks instead of
per node, dispatch on the exact type before falling back to the
`isinstance` ladder for subclasses, cache each dataclass type's field
layout with pre-encoded names, and cache the encoded form of short strings
and of `ImportVar` instances (a frozen dataclass of `str`/`bool`/`None`
fields, so its generated equality means exactly "same encoding", and it
accounts for most of what a component hash consumes: 5664 visits across
just 12 distinct values on one benchmark page).

The byte stream is unchanged, so every digest is identical to before —
verified against a copy of the previous implementation over all values
hashed while compiling four benchmark pages. 2.3-2.6x faster on the large
pages, 1.8-1.9x on the small ones.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01X8v1xEMwog6ZhP7hZYibFr

* docs: add changelog fragment for the component hash speedup

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01X8v1xEMwog6ZhP7hZYibFr

* refactor(compiler): move memo-name hashing into the memo module

The deterministic hash exists for exactly one purpose: giving an
auto-memoized component a stable, non-colliding export name. It lived in
`component.py` as `Component._get_component_hash` and
`Component._compute_memo_tag`, but nothing outside `memo.py` ever called
either, and neither is a property of a component the way `render()` or
`_get_imports()` is.

Move the encoder and both entry points into `memo.py` as
`component_hash(component, *, recursive=...)` and `memo_tag(component)`,
next to the `create_passthrough_component_memo` call site, and drop the two
methods from `Component`. The `shallow` flag becomes `recursive`, named for
what it means at the call site: a snapshot memo body carries its whole
subtree, a passthrough body carries a `{children}` hole. Also drops the
unused `_hash_str` helper.

The own-node artifact set was missing `add_custom_code`: `_get_custom_code`
was hashed but the classmethod extension point was not, while the recursive
side picked it up through `_get_all_custom_code`. Two passthrough bodies
that rendered identically and differed only in the module-level code they
emit therefore shared one memo module, and one of the two code blocks was
dropped. Fed explicitly now, with a regression test.

Compile wall time is unchanged; this is a structural change plus the
collision fix.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01X8v1xEMwog6ZhP7hZYibFr

* fix(compiler): release memo-naming caches once compilation is done

The encoding caches that speed up memo naming were module globals with no
teardown. The two value caches are capped, but the dataclass field-layout
cache is keyed by type and was uncapped -- and a dataclass defined inside a
function body is a fresh class object on every call, so hashing one pinned
a class per compile for the life of the process. Confirmed reachable: 50
dynamically created dataclasses survived a gc.collect().

Capping that cache would be the wrong fix. It bounds retention without
removing it, and once the cap is hit every dataclass encode falls back to
`dataclasses.fields()` plus re-encoding field names per instance -- a
silent cliff on the hot path, for a cache whose real-world population is
two entries (`VarData` and `ImportVar`, stable across repeated compiles).

Every component auto-memoization will ever name is named during
compilation, so drop all three caches when it finishes, alongside the
existing `GLOBAL_CACHE.clear()` in the same post-compile block. Digests are
unchanged and compile wall time is unaffected.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01X8v1xEMwog6ZhP7hZYibFr

* fix(compiler): close two more memo-name collisions, bound the hash buffer

Review of the naming hash turned up two more gaps of the same kind as the
`add_custom_code` one:

- `_get_dynamic_imports` is emitted into the memo body by
  `compile_experimental_component_memo` but was never hashed, so two
  components differing only there shared a module and one of their two
  import statements was dropped.
- `memo_tag` identified a class by `__qualname__` alone, so two modules
  each defining `class Card` with the same rendered output produced the
  same tag -- exactly what the qualname prefix exists to prevent. The
  defining module now reaches the digest rather than the prefix, which
  keeps the discrimination without stretching every generated module
  filename by a dotted module path.

Both are covered by regression tests that fail without the fix.

Also make the encoder's buffer bound real: the flush check ran only after
a container's whole loop, so one flat 2 MB dict buffered 2 MB before the
first flush. Checking per item holds it at the intended 64 KiB and costs
nothing measurable -- the encoder is still 1.7-2.0x the old one and every
digest is byte-identical to it.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01X8v1xEMwog6ZhP7hZYibFr

* fix(compiler): release naming caches from the compile lifecycle

`clear_hash_caches()` was called from `App.__call__`, which only the ASGI
path reaches. `reflex export` and `reflex compile` get to a compile through
`prerequisites.get_compiled_app` -> `App._compile` and never touch
`__call__`, so those paths never released anything.

Move the call into `App._compile` -- the single funnel every compile goes
through -- inside a `finally`, so a failed compile does not leave the
caches behind either. Covered by a test that fails under the old
placement, on both the success and the exception path.

Also add the root `news/` fragment: this PR now touches `reflex/`, so the
changelog check requires one for the main package too. Corrects the
reflex-base performance fragment, which claimed digests were unchanged --
true of the encoder rewrite alone, but later commits deliberately folded
the defining module and dynamic imports into the hash, so generated memo
module names do change.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01X8v1xEMwog6ZhP7hZYibFr

* chore: update pyi_hashes.json for the memo module change

Moving the naming hash into `memo.py` changed the source that
`reflex/experimental/memo.pyi` is generated from, so its recorded hash
went stale and the pre-commit check failed. I ran `make_pyi.py` after the
first commit but not after the move.

`pre-commit run --all-files` now passes all seven hooks.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01X8v1xEMwog6ZhP7hZYibFr

* perf(compiler): resolve hash encoders per type, fix a fifth memo collision

Incorporates the ideas from #6804 into the memo-name hashing rework.

The encoder no longer walks an isinstance ladder for every value whose exact
type has no inline fast path. It resolves an encoder once per type and reaches
it through a memoized table, so vars, components, enums and dataclasses each
pay the ladder once per compile instead of once per value. That also collapses
the two parallel ladders the previous version carried -- an exact-type one and
an isinstance one -- into a single encoder per type.

Fixes a fifth naming collision, found by #6804: the dataclass branch sat ahead
of the component branch, so a component that also inherits a dataclass encoded
as that mixin's field list. Every component built on MarkdownComponentMap does
-- rx.text, rx.heading and friends -- and the mixin declares no fields, so all
of them encoded to the same nine bytes. Reachable through app-wrap components,
which the hash feeds in as components rather than as rendered dicts.

The ImportVar encoding cache is no longer hard-coded to ImportVar: any frozen
dataclass declaring only str/bool/None fields is cached by value. Numbers are
excluded because equality has to imply an identical encoding for a value-keyed
cache to be sound, and True == 1 == 1.0 while all three encode differently.
Reading a class's annotations is the expensive part, so that per-type verdict
outlives a compile in a WeakKeyDictionary, which still lets a dataclass defined
in a function body be collected. Cached encodings are bounded in size as well
as in count.

Verified byte-identical: every memo tag generated while compiling three
benchmark pages is unchanged, and an A/B of the two encoders on the values
those compiles hash gives matching digests at 1.00-1.02x the speed.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01F1fNEq67sbXeXDZGjLX16c

* docs: tighten the changelog fragments

Fragments are for downstream users; the narrative is a click away on the PR.
The bugfix one becomes a bulleted list of what the memo name now accounts for
instead of a paragraph per collision.

Verified the list renders correctly through the release tooling's own towncrier
invocation.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01F1fNEq67sbXeXDZGjLX16c

* Delete news/6947.performance.md

* refactor: move the deterministic hash into its own module

The hash is not a property of memoization -- it digests components, vars and
rendered data under a self-delimiting encoding, and auto-memoization is just
its only caller today. Moved to reflex_base/utils/deterministic_hash.py with
the tests alongside it.

The private _deterministic_hash/_update_deterministic_hash pair becomes one
public variadic deterministic_hash(*values), which is what the two call sites
wanted: component_hash now reads as the render plus the artifacts that identify
a memo body, and _update_component_artifacts_hash becomes _component_artifacts,
a generator that yields them instead of threading a hasher and buffer through.
One shared buffer still covers the whole digest.

Nothing else under utils imports components at runtime, so the two isinstance
checks that need Var and BaseComponent import them inside _resolve_hash_encoder
-- once per type, so it never shows up in a profile -- and the module now
imports standalone without pulling in the component system.

Digests are unchanged: memo tags across three benchmark pages are byte-identical
to before the move, and an A/B of the encoder before and after runs at parity.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01F1fNEq67sbXeXDZGjLX16c

* docs: restore the reflex changelog fragment

The PR touches reflex/app.py, so the root package needs a fragment too.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01F1fNEq67sbXeXDZGjLX16c

* docs: note why the shared cached get_type_hints is not used here

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01F1fNEq67sbXeXDZGjLX16c

* perf(compiler): hash import library names, not the ImportVars under them

Walking the ImportVar lists was the largest single item in the memo-name
encoding, and almost all of it was redundant. An import only reaches a memo
body through the local name it binds, and every name a body references is
already in its render, hooks or custom code: Reflex aliases each tag to a
globally unique binding (Trigger -> RadixAccordionTrigger), and
validate_imports rejects one name bound from two libraries. So bodies that
render alike reference the same names, and the library names are what pin
where each name comes from.

Encoding drops 11-19% depending on the page (13.06 -> 11.55 ms on a
memoization-heavy one). Distinct tag counts across three benchmark pages are
unchanged -- 61, 9 and 2 -- so no page gains a collision from the narrower
digest.

What this deliberately stops distinguishing, documented on the function: two
bodies binding the same name from the same library to a different export
(X as N vs Y as N) or in a different form (default vs named). Both need one
library to export two things a component aliases to one name.

Generated memo module names change again, for the same reason as the rest of
this branch: nothing outside the compiled output refers to them.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01F1fNEq67sbXeXDZGjLX16c

* fix(compiler): encode dataclass and enum identity; trim comments

Two injectivity holes in the encoder, both raised in review and both confirmed:

- Two dataclasses with the same field names and values encoded identically,
  so Alpha(a="x") and Beta(a="x") shared a digest. The defining class now goes
  into the cached layout header, which is built once per type.
- enum members encoded as str(value), which for an IntEnum is just its integer,
  so Level.ONE and 1 shared a digest. Enums now get their own tag and encode
  their qualified member name.

Both were present before this branch; the encoder claims injectivity, so they
belong with the rest of the collision fixes. The parametrized case that claimed
to cover distinct dataclass types compared two instances of one type and could
not have caught the first; it now uses two types. The synthesized-dataclass test
asserted a runtime-built class hashed equal to the class it copied its fields
from, which only held while class identity was absent from the digest.

Also trims the narrative from comments and docstrings across the branch, leaving
what the code needs and moving the reasoning to the PR.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01F1fNEq67sbXeXDZGjLX16c

* test: cover values larger than the hash flush buffer

Nothing exercised a single leaf bigger than _HASH_BUFFER_FLUSH_SIZE. A leaf is
appended whole, so the buffer holds all of it before the first flush can run:
a LiteralStringVar of 3x the flush size peaks at 196,621 B against a 65,536 B
threshold. Behaviour is correct, it just had no test.

Adds that case, plus a parametrized check that the digest is identical for
flush sizes from 1 byte to 1 GiB, which pins the invariant that chunking only
moves bytes into the hasher. The second one fails if a flush stops clearing the
buffer.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01F1fNEq67sbXeXDZGjLX16c

* fix(compiler): hash ImportVars again; don't crash on an unhashable field

Two review findings, both reproduced.

Hashing only import library names dropped ImportVar payloads from the digest.
Icon._get_imports builds a per-instance package_path and alias, and a tagless
ImportVar defaults to render=True so compile_imports emits it as a side-effect
import ("lucide-react/light.css"). Two bodies differing only there rendered
identically, shared a memo tag, and one body's import never reached the
compiled module. Restores the imports dict; the narrowing was measured again
at ~4% of encoding, not the 23% an earlier harness suggested.

_encode_hash_cached_dataclass keyed the value cache on instances whose
hashability comes from their declared field types, so a frozen dataclass
declaring `name: str` while holding a list raised TypeError where the previous
hasher returned a digest. It now falls back to encoding the fields directly.

Also corrects the module docstring: numbers carry a type tag but no length
prefix, so the previous wording overstated the encoding.

Re-measured against main: encoding 1.31x on a memoization-heavy page and 1.17x
on _stateful_page, with the whole component_hash at parity. Fragment updated.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01F1fNEq67sbXeXDZGjLX16c

* test: isolate the annotation-mismatch test from the shared caches

Its well-typed _KeyedProbe instances take the cached-dataclass path, so it left
entries in _hash_dataclass_encodings for whatever ran next. Every other
cache-touching test in the file already takes clean_hash_caches; this one now
does too.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01F1fNEq67sbXeXDZGjLX16c

---------

Co-authored-by: Claude <noreply@anthropic.com>
@masenf

masenf commented Sep 1, 2026

Copy link
Copy Markdown
Collaborator

rode in with #6947

@masenf masenf closed this Sep 1, 2026
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