Skip to content

ENG-9148: feat(compiler): incremental compile cache & warm hot-reload daemon (REFLEX_COMPILE_CACHE) - #6688

Open
FarhanAliRaza wants to merge 63 commits into
reflex-dev:mainfrom
FarhanAliRaza:reflex-hmr
Open

FarhanAliRaza wants to merge 63 commits into
reflex-dev:mainfrom
FarhanAliRaza:reflex-hmr

Conversation

@FarhanAliRaza

@FarhanAliRaza FarhanAliRaza commented Jun 28, 2026 •

Copy link
Copy Markdown
Contributor

An experimental, flag-gated frontend compile pipeline that recompiles only the pages whose source actually changed and reuses the rest. Off by default; enabled by REFLEX_COMPILE_CACHE. The compile path is completely unchanged when the flag is unset.

What it does

On-disk incremental cache (disk_cache.py + page_cache.py)

A fresh compile process reuses the previous build already on disk in .web and recompiles only the pages whose source changed. Each page is invalidated by a precise dependency set rather than a global timestamp:

  • the page's first-party Python import closure,
  • files read while the page is evaluated (markdown/data, including runtime importlib imports and data read at import time),
  • the component modules in its rendered tree, and
  • the fine-grained state files it references.

Pages are keyed by the content hashes of that set plus a small global epoch — Reflex version, rxconfig/lockfiles, and the app entrypoint's config-only modules (theme, app-wraps, stylesheets, head components, plus config the app imports dynamically at load time). A change to a genuinely-global input forces a full recompile; per-file edits invalidate only the pages that depend on them. Any unsafe condition falls back to a full compile, so a cache miss is never a correctness risk.

Warm fork-per-compile hot-reload daemon (compile_daemon.py)

reflex run dev imports the app once, then compiles each edit in an isolated child — forking on POSIX so third-party imports stay warm (falling back to a fresh process elsewhere). Combined with the disk cache, a hot reload skips the cold reimport and rebuilds only what changed.

Supporting changes required for safe page reuse: deterministic compile-time ref-name generation, own-before-mutate page-metadata injection, and mirroring memo output paths to their Python source modules.

All Submissions:

  • Have you followed the guidelines stated in CONTRIBUTING.md file?
  • Have you checked to ensure there aren't any other open Pull Requests for the desired changed?

Type of change

  • New feature (non-breaking change which adds functionality)

New Feature Submission:

  • Does your submission pass the tests?
  • Have you linted your code locally prior to submission?

Changes To Core Features:

  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests for your core changes, as applicable?
  • Have you successfully ran tests with your changes locally?

Add an experimental, flag-gated incremental frontend compile cache that
recompiles only the pages whose source actually changed and reuses the rest.

Two layers, both off by default and enabled by REFLEX_COMPILE_CACHE:

- In-process per-page cache (page_cache.py): a Salsa-style dependency graph
  records the exact set of source files each page reads, so editing one file
  invalidates only the pages that depend on it. Pages are keyed by a small
  genuinely-global epoch (Reflex version + rxconfig + lockfile) plus the
  content hashes of their dependency set. Speeds up repeat compiles within a
  single process.

- On-disk manifest (disk_cache.py): persists each page's serializable
  contribution and dependency hashes to .web/reflex_compile_cache.json so a
  fresh process — notably a `reflex run` hot-reload worker, which respawns on
  every edit — recompiles only changed pages and reuses the rest. Falls back
  to a full compile on any unsafe condition.

REFLEX_COMPILE_CACHE_VERIFY runs a full compile alongside the cached one and
asserts byte-identical output, falling back on mismatch — the backstop for
gaps a static dependency graph cannot see (runtime importlib imports, data
read at module-import time).

Supporting changes required for safe page reuse: deterministic compile-time
ref-name generation, and own-before-mutate page metadata injection.
@FarhanAliRaza
FarhanAliRaza requested a review from a team as a code owner June 28, 2026 15:37
@codspeed

codspeed Bot commented Jun 28, 2026 •

Copy link
Copy Markdown

Merging this PR will degrade performance by 4.61%

2 warnings

⚠️ 15 benchmarks are dominated by system calls

System calls cannot be simulated consistently and are left out of the measure. The reported value understates the real cost. Switch to the Walltime instrument to measure them.

Learn more about system calls

See affected benchmarks

⚠️ Base and head ran on different runner types

Some benchmarks with significant performance changes were compared across different runner types, so the changes might not come from the code. Run base and head on the same runner type, or use CodSpeed Macro Runners for more stable results.

Learn more about Macro Runners

Open the report in CodSpeed to investigate

❌ 1 regressed benchmark
✅ 147 untouched benchmarks

Warning

Please fix the performance issues or acknowledge them on CodSpeed.

Performance Changes

Benchmark BASE HEAD Efficiency
❌ test_computed_dependency_fanout[10] 477.6 µs 500.7 µs -4.61%

Tip

Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.


Comparing FarhanAliRaza:reflex-hmr (9752de1) with main (be0c685)

Open in CodSpeed

@FarhanAliRaza
FarhanAliRaza marked this pull request as draft June 28, 2026 15:40
@greptile-apps

greptile-apps Bot commented Jun 28, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerView in GreptileConfidence Score: 5/5

[Medium impact] The recent changes appear safe to merge, with a non-blocking duplicate-telemetry issue.

Findings

  1. P2 Compile events get counted twice ▶

Summary

Adds an experimental compile cache and warm reload daemon behind REFLEX_COMPILE_CACHE.

  • Changed source rebuilds only the frontend pages that depend on it.
  • Development reloads compile edits in an isolated child process.
  • Memo files and generated state names stay consistent across cached compiles.

Previous-thread context:

  • FarhanAliRaza declined extracting manifest keys into constants because all keys are local to the same module; they accepted keeping that pattern and deferred the broader cleanup.
  • FarhanAliRaza dismissed the preloaded-app findings because supported compile paths import under the recorder or clear first-party modules first. Greptile withdrew both disputed findings.
  • The supplied threads are unnumbered, so none require numbered previous-finding entries.

Reviews (44) · Last reviewed commit: "test(memo): let React Compiler compile t..." · Reviewed by Greptile

Comment thread reflex/compiler/disk_cache.py
Comment thread reflex/compiler/disk_cache.py Outdated
Comment thread reflex/compiler/page_cache.py

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a453ccbd69

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread reflex/compiler/disk_cache.py Outdated
Comment thread reflex/compiler/disk_cache.py Outdated
Comment thread reflex/compiler/page_cache.py Outdated
Add a persistent compile daemon (REFLEX_COMPILE_CACHE) that imports the
world once and forks a throwaway child per source change, instead of the
reloader respawning a worker that cold-imports on every edit. The child
re-imports first-party code fresh and runs the incremental rebuild, so
correctness matches a respawn while the cold import is paid once.

Supporting changes that make the daemon safe and complete:

- write_file now writes atomically (temp + os.replace) so a reader (vite,
  a concurrent compile) never sees a half-written file, even when a forked
  child is killed mid-compile.
- _run_dev launches the daemon and sets REFLEX_SKIP_COMPILE on the backend
  so it only evaluates pages to register state.
- The daemon watches what the compiler reads (incl. sibling-dir markdown
  from the manifest); uvicorn reload_includes also covers *.md/*.mdx, so
  markdown edits finally trigger a reload.
- Drop the per-rebuild console.info now that progress is shown inline.
Comment thread reflex/compiler/disk_cache.py Outdated
Comment thread reflex/compiler/disk_cache.py Outdated
multi_docs built every component's prop tables at module import, so the
whole library reference was reconstructed on every import of the docs
tree -- re-run on every dev hot-reload reimport, cold start, and backend
respawn. Move the build into the page render closures (matching the
non-library doc path) so a page builds its prop tables only when it is
actually compiled.

Docs cold import 9.7s -> 1.9s; hot-reload reimport 8.3s -> 0.6s.
- disk_cache: stop re-evaluating stateful HIT pages during an incremental
  rebuild. The compiling process never serves (the daemon, the initial
  compile, and CLI compiles all exit; the serving backend re-evaluates the
  marked stateful pages itself), so re-running their render pipeline was
  pure waste. The stateful-pages marker stays complete -- hits recorded
  from the manifest, misses from the fresh compile.
- compile_daemon: poll faster (0.25s -> 0.05s) but cheaply -- stat the
  known file set each tick and rglob only every 1s for added/removed
  files, cutting detection latency without burning idle CPU.
- compile_daemon: log per-edit timing (reset / reimport / compile).
@FarhanAliRaza

Copy link
Copy Markdown
Contributor Author

@greptile please rereview

Comment thread reflex/compiler/disk_cache.py Outdated
Comment thread reflex/compiler/disk_cache.py Outdated
Profiling the hot-reload compile on the docs app (418 pages) showed the
disk-cache rebuild spent ~1.1s of its ~1.6s on manifest I/O: the manifest
stored every page's rendered output_code (and output_path/frontend_imports),
ballooning it to 46MB, yet those fields are never read back — only dep_hashes,
app_wrap_keys, is_stateful, and the merged all_imports are consumed. Storing
just the bookkeeping that is read drops the manifest to 14MB.

The in-process page cache (_PAGE_STORE / validate_page / store_page) is never
reached by the warm daemon (its fork child cleared it and the disk path returns
first), and forcing it measured ~55x slower than the disk path because it
re-runs the whole app-level pipeline (memo render, stylesheet, plugins) every
edit. Remove it, the verify mode that only guarded it
(REFLEX_COMPILE_CACHE_VERIFY), and the dead page_source_fingerprint.

Manifest schema bumped 2->3 so stale fat manifests are ignored.
Comment thread reflex/compiler/disk_cache.py Outdated
Comment thread reflex/compiler/disk_cache.py Outdated
Remove page_module_files and file_hashes, now-dead after the
compile-cache manifest rework, along with their tests.
Two correctness fixes for the incremental compile cache, plus a pass to
trim verbose comments and docstrings across the cache modules.

- global_epoch now fingerprints the app-level config files: the app
  entrypoint module plus the config-only modules it imports (theme,
  app-wraps, stylesheets), found by walking the import graph with page
  modules as barriers so per-page incrementality is preserved. An edit to
  app-wide config previously left every page a hit, so the reused on-disk
  app root / contexts / theme stayed stale.
- The incremental manifest refresh now persists the complete install
  import set (page imports merged with the memo-component imports the
  rebuild generated, and threading root through dependency discovery), so
  a later all-hit compile installs every package the reused memo files
  need instead of dropping newly-introduced memo packages.
Comment thread reflex/compiler/page_cache.py Outdated
test_reset_model_metadata_allows_table_redefinition builds an
rx.Model(table=True), which needs the SQLModel stack. The "without db
dependencies" unit-test job uninstalls it, so the first model definition
raised TypeError before the test could exercise the reset. Guard with
pytest.importorskip("sqlmodel"), matching the repo's other db-dependent
tests, so it skips there instead of failing.
Add the news fragments the changelog check requires for the packaged
source touched by this PR: a feature entry for the reflex package's
REFLEX_COMPILE_CACHE flag and a misc entry for the reflex-base recorder
hook / reproducible ref names that support it.
Comment thread reflex/compiler/page_cache.py Outdated
Patch __import__ and importlib.import_module while a page recorder is
active so modules imported during page eval are recorded as source
dependencies. Resolves names (including relative imports), skips stdlib
and out-of-project files, and caches module-file lookups. Lets the
incremental compile cache invalidate pages that depend on first-party
modules pulled in at import time.
Comment thread reflex/compiler/page_cache.py
Replace the os.path string-comparison helpers used for runtime import
tracking with pathlib.Path operations, dropping the redundant cached
root-string globals (_recorder_root_str, _recorder_raw_root_str) and the
sentinel-based module-file cache. Make the read-set target an explicit
argument instead of falling back to the recorder context.
Comment thread reflex/compiler/page_cache.py Outdated
The compile-cache read tracker patches builtins.__import__ to record a
page's runtime imports. While recording, _recordable_module_file resolves
a module file via Path(...).absolute(), which on CPython 3.12 lazily runs
`import ntpath` to compare path flavours. That import re-entered the
patched hook -> resolved a path again -> imported again, recursing until
the stack overflowed (32 unit tests failed only on ubuntu 3.12).

Suspend read/import tracking (_suspend_tracking) while the tracker's own
path/module bookkeeping runs, so imports and reads it triggers are neither
recorded nor able to re-enter. Add a version-independent regression test
that mimics the 3.12 lazy import.
Comment thread reflex/compiler/page_cache.py Outdated
@FarhanAliRaza

Copy link
Copy Markdown
Contributor Author

@codex pelase review

Config the app module pulls in dynamically (importlib calls, files read at
import time) was invisible to app_dependency_files, which only walked the
static import graph from the entrypoint. Editing such config could leave
reused app-wide output stale.

Record files read/imported while the app module loads (record_app_import,
wired into prerequisites.get_app under REFLEX_COMPILE_CACHE) and fold that
dynamic set into app_dependency_files, subtracting per-page static closures
so ordinary page edits still invalidate per-page. Extract the shared graph
walk into _walk_import_closure.
Comment thread reflex/compiler/disk_cache.py Outdated
- Forget import-time reads when read tracking switches project roots, and
  keep the reads of an import that fails.
- Expand dynamic-state pages before memo siblings, and re-evaluate them when
  their page order changes.
- Fall back when a page stops registering a module state other pages may use.
- Install the packages that component-valued state defaults import.
- Rebuild the app root when the order of its wraps changes.
- Compare functools.partial page arguments, and track the wrapped function.
- Slices: recompute the read names for computed global lookups, trust only
  qualified framework/stdlib decorators, and count entry-module class bodies.
- Record built inputs from the refreshed watch state; a newly tracked input
  is no edit.
- Treat only lock contention as a held lock; a daemon without its lock does
  not serve.
- Use directory constants, add missing docstrings, shorten the news entry.
Comment thread reflex/utils/compile_daemon.py Outdated
Immediate same-size writes can keep the previous modification timestamp on Windows. Give the edit fixtures distinct mtimes so they exercise the slice digest instead of reusing the stat cache.
@masenf

masenf commented Oct 20, 2026

Copy link
Copy Markdown
Collaborator

Pushed test-only Windows CI maintenance in 1354854. The failed Windows job rewrote module contents without always changing their size or modification time, so the stat cache could reuse the previous parse. The three affected test functions now explicitly advance the edit's timestamp; runtime cache behavior is unchanged.

Validation: preserving rewrite timestamps reproduced six failing parameter cases before the fix. Afterward, all 58 page-cache tests pass under that simulation, and all 124 page-cache/disk-cache tests pass normally. Full Ruff checks, pyright reflex tests, and applicable commit hooks pass. Fresh CI will validate Windows.

@masenf

masenf commented Oct 21, 2026

Copy link
Copy Markdown
Collaborator

Pushed 98e0259 to address the repeated Windows 3.11 failure. A backend reader can briefly prevent the daemon from replacing .compiled_inputs.json; replacement now retries twice on Windows, preserving the old record until publication succeeds. Persistent errors still propagate, and temporary files are cleaned up. The timer test also checks that the record is current instead of requiring an exact 100 ms minimum wait.

Validation: five fault-injection cases failed before the fix and pass afterward; all 215 focused daemon, lock, and cache tests, full Ruff checks, Pyright, and applicable commit hooks pass locally. Fresh CI will validate Windows.

- A process that serves the app compiles without the incremental cache, so
  every page registers its states.
- The daemon compares function-slice entries when it checks for edits made
  before its first snapshot, and keeps a newly tracked input pending unless
  the manifest records its current content.
- A computed global lookup keeps every import in the function slice.

@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 8 files (changes from recent commits).

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

View guided diff | Re-trigger cubic

Comment thread reflex/compiler/compiler.py
Comment thread reflex/compiler/compiler.py
Comment thread reflex/utils/compile_daemon.py Outdated
- A compile that bypasses the incremental cache (a serving process, a
  preloaded app, or the flag unset) discards the manifest before it writes
  frontend output, so a later cached compile cannot reuse files that no
  longer match what the manifest describes.
- The daemon's check for edits made before its first snapshot compares only
  the file-table entries the manifest still references. An incremental
  rebuild extends the table without pruning it, so a renamed page function
  left a stale slice that reported the module as unbuilt on every start.

Claude-Session: https://claude.ai/code/session_01Y27XUSyj8uGXAaLaYyzYsa

@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 5 files (changes from recent commits).

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

View guided diff | Re-trigger cubic

Comment thread reflex/compiler/disk_cache.py
…arrier

- The daemon's watch set and its startup check now cover import-time state
  files and memo source closures, which the manifest records outside any
  page's dependencies.
- The stateful-pages marker test aborted its barrier right after the last
  round; a reader released from that round but not yet scheduled saw the
  aborted barrier and failed. The barrier is now aborted only when a writer
  fails.

Claude-Session: https://claude.ai/code/session_01Y27XUSyj8uGXAaLaYyzYsa

@FarhanAliRaza FarhanAliRaza left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Full review at 3565bb6: code paths + salsa architecture

Three passes: a salsa comparison of the architecture, a correctness pass over disk_cache.py and the compiler integration, and a correctness pass over page_cache.py, compile_daemon.py and path_ops.py. The oracle for every repro: the incremental .web must match a cold compile of the same sources with the cache off. Inline comments mark each finding as CONFIRMED (reproduced) or PLAUSIBLE (from code only). A docs-app e2e run is still in progress; its findings follow in a separate review.

Architecture verdict (salsa lens)

The core model is sound and matches salsa: each page is a memoized query, keyed by content hashes of recorded reads plus a declared import closure, verified by re-hashing exactly the stored dependency set, with a full-compile fallback on global inputs. Keep it.

The weak points are where the design leaves two salsa rules:

  1. Generated identities must be keyed by their creator. Local state names, ComponentState numbers and state-default ref names come from process-global, order-dependent sequences. The PR predicts them and retries (dynamic_claims, _ComponentStateNumbersPlugin, DynamicStatePredictionError / exit code 3, compiled_in_predicting_child, the reflex.istate.dynamic attribute wipe).
  2. Fan-in outputs must replay every contributor's stored contribution, or fall back. Shared memo files, the page registry and the app root depend on cross-page state that is not recorded.

Most High findings below sit in exactly these two areas. Proposals, each removes code instead of adding it:

  • P1. Creator-keyed names. Derive local-state and ComponentState names from (owner route, class, ordinal in page), and seed state-default names per state full name. This deletes about 380 lines of prediction and retry machinery, fixes the open ref-name collision, and makes reloads faster (no retry child). It changes local-state names once, outside the flag, so it needs a framework decision.
  • P2. Content-addressed auto memos (one file per memo, named by its hash), or store each page's used memo keys and replay them. This deletes _with_module_siblings, has_memos, and the sibling half of _with_memo_contributing_pages, and fixes the shared-memo bug by construction.
  • P3. App root: fall back instead of reconstructing. If a re-evaluated page's wrap digests or bundle requests differ from its record, do a full compile. This deletes _refresh_app_root and the pickled wrap table (about 250 lines). The cost is one full compile when a page first gains a provider, which is rare.
  • P4. Make the page registry a tracked input. For example, a route-set or registration change re-evaluates every page.
  • P5. Function slices are the largest declared-dependency mechanism (about 350 lines). They are mostly conservative, but see the holes inline. Measure their benefit on bench100 before keeping or deleting them.
  • P6. A salsa-style untracked-read escape (mark a page volatile) for env, time, directory listings and subprocess reads, as an internal or env-based switch first.

Not inline (no changed line to anchor)

  • PLAUSIBLE: backend-only evaluation (app._compile_page) is not seeded per route, so it draws names in a different sequence than the compiling process. This only matters for names the backend uses, e.g. a ClientStateVar created inside a page function.
  • PLAUSIBLE, macOS: the fork child's record_compile telemetry goes through urllib → _scproxy/CoreFoundation after fork, which is the known fork-unsafe abort. On every OS, os._exit drops the child's queued telemetry. Disable telemetry sends in compile children, or flush before _exit.

Comment thread reflex/compiler/disk_cache.py Outdated
Comment thread reflex/compiler/disk_cache.py
Comment thread reflex/compiler/page_cache.py
Comment thread reflex/compiler/page_cache.py
Comment thread reflex/compiler/compiler.py Outdated
Comment thread packages/reflex-base/src/reflex_base/plugins/compiler.py Outdated
Comment thread packages/reflex-base/src/reflex_base/plugins/compiler.py
Comment thread reflex/compiler/compiler.py
Comment thread reflex/utils/telemetry.py Outdated
Comment thread reflex/compiler/page_cache.py Outdated

@FarhanAliRaza FarhanAliRaza left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Full review at 3565bb6: code paths + salsa architecture

Three passes: a salsa comparison of the architecture, a correctness pass over disk_cache.py and the compiler integration, and a correctness pass over page_cache.py, compile_daemon.py and path_ops.py. The oracle for every repro: the incremental .web must match a cold compile of the same sources with the cache off. Inline comments mark each finding as CONFIRMED (reproduced) or PLAUSIBLE (from code only). A docs-app e2e run is still in progress; its findings follow in a separate review.

Architecture verdict (salsa lens)

The core model is sound and matches salsa: each page is a memoized query, keyed by content hashes of recorded reads plus a declared import closure, verified by re-hashing exactly the stored dependency set, with a full-compile fallback on global inputs. Keep it.

The weak points are where the design leaves two salsa rules:

  1. Generated identities must be keyed by their creator. Local state names, ComponentState numbers and state-default ref names come from process-global, order-dependent sequences. The PR predicts them and retries (dynamic_claims, _ComponentStateNumbersPlugin, DynamicStatePredictionError / exit code 3, compiled_in_predicting_child, the reflex.istate.dynamic attribute wipe).
  2. Fan-in outputs must replay every contributor's stored contribution, or fall back. Shared memo files, the page registry and the app root depend on cross-page state that is not recorded.

Most High findings below sit in exactly these two areas. Proposals, each removes code instead of adding it:

  • P1. Creator-keyed names. Derive local-state and ComponentState names from (owner route, class, ordinal in page), and seed state-default names per state full name. This deletes about 380 lines of prediction and retry machinery, fixes the open ref-name collision, and makes reloads faster (no retry child). It changes local-state names once, outside the flag, so it needs a framework decision.
  • P2. Content-addressed auto memos (one file per memo, named by its hash), or store each page's used memo keys and replay them. This deletes _with_module_siblings, has_memos, and the sibling half of _with_memo_contributing_pages, and fixes the shared-memo bug by construction.
  • P3. App root: fall back instead of reconstructing. If a re-evaluated page's wrap digests or bundle requests differ from its record, do a full compile. This deletes _refresh_app_root and the pickled wrap table (about 250 lines). The cost is one full compile when a page first gains a provider, which is rare.
  • P4. Make the page registry a tracked input. For example, a route-set or registration change re-evaluates every page.
  • P5. Function slices are the largest declared-dependency mechanism (about 350 lines). They are mostly conservative, but see the holes inline. Measure their benefit on bench100 before keeping or deleting them.
  • P6. A salsa-style untracked-read escape (mark a page volatile) for env, time, directory listings and subprocess reads, as an internal or env-based switch first.

Not inline (no changed line to anchor)

  • PLAUSIBLE: backend-only evaluation (app._compile_page) is not seeded per route, so it draws names in a different sequence than the compiling process. This only matters for names the backend uses, e.g. a ClientStateVar created inside a page function.
  • PLAUSIBLE, macOS: the fork child's record_compile telemetry goes through urllib → _scproxy/CoreFoundation after fork, which is the known fork-unsafe abort. On every OS, os._exit drops the child's queued telemetry. Disable telemetry sends in compile children, or flush before _exit.

Comment thread reflex/compiler/disk_cache.py Outdated
Comment thread reflex/compiler/disk_cache.py
Comment thread reflex/compiler/page_cache.py
Comment thread reflex/compiler/page_cache.py
Comment thread reflex/compiler/compiler.py Outdated
Comment thread packages/reflex-base/src/reflex_base/plugins/compiler.py Outdated
Comment thread packages/reflex-base/src/reflex_base/plugins/compiler.py
Comment thread reflex/compiler/compiler.py
Comment thread reflex/utils/telemetry.py Outdated
Comment thread reflex/compiler/page_cache.py Outdated

@FarhanAliRaza FarhanAliRaza left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Full review at 3565bb6: code paths + salsa architecture

Three passes: a salsa comparison of the architecture, a correctness pass over disk_cache.py and the compiler integration, and a correctness pass over page_cache.py, compile_daemon.py and path_ops.py. The oracle for every repro: the incremental .web must match a cold compile of the same sources with the cache off. Inline comments mark each finding as CONFIRMED (reproduced) or PLAUSIBLE (from code only). A docs-app e2e run is still in progress; its findings follow in a separate review.

Architecture verdict (salsa lens)

The core model is sound and matches salsa: each page is a memoized query, keyed by content hashes of recorded reads plus a declared import closure, verified by re-hashing exactly the stored dependency set, with a full-compile fallback on global inputs. Keep it.

The weak points are where the design leaves two salsa rules:

  1. Generated identities must be keyed by their creator. Local state names, ComponentState numbers and state-default ref names come from process-global, order-dependent sequences. The PR predicts them and retries (dynamic_claims, _ComponentStateNumbersPlugin, DynamicStatePredictionError / exit code 3, compiled_in_predicting_child, the reflex.istate.dynamic attribute wipe).
  2. Fan-in outputs must replay every contributor's stored contribution, or fall back. Shared memo files, the page registry and the app root depend on cross-page state that is not recorded.

Most High findings below sit in exactly these two areas. Proposals, each removes code instead of adding it:

  • P1. Creator-keyed names. Derive local-state and ComponentState names from (owner route, class, ordinal in page), and seed state-default names per state full name. This deletes about 380 lines of prediction and retry machinery, fixes the open ref-name collision, and makes reloads faster (no retry child). It changes local-state names once, outside the flag, so it needs a framework decision.
  • P2. Content-addressed auto memos (one file per memo, named by its hash), or store each page's used memo keys and replay them. This deletes _with_module_siblings, has_memos, and the sibling half of _with_memo_contributing_pages, and fixes the shared-memo bug by construction.
  • P3. App root: fall back instead of reconstructing. If a re-evaluated page's wrap digests or bundle requests differ from its record, do a full compile. This deletes _refresh_app_root and the pickled wrap table (about 250 lines). The cost is one full compile when a page first gains a provider, which is rare.
  • P4. Make the page registry a tracked input. For example, a route-set or registration change re-evaluates every page.
  • P5. Function slices are the largest declared-dependency mechanism (about 350 lines). They are mostly conservative, but see the holes inline. Measure their benefit on bench100 before keeping or deleting them.
  • P6. A salsa-style untracked-read escape (mark a page volatile) for env, time, directory listings and subprocess reads, as an internal or env-based switch first.

Not inline (no changed line to anchor)

  • PLAUSIBLE: backend-only evaluation (app._compile_page) is not seeded per route, so it draws names in a different sequence than the compiling process. This only matters for names the backend uses, e.g. a ClientStateVar created inside a page function.
  • PLAUSIBLE, macOS: the fork child's record_compile telemetry goes through urllib → _scproxy/CoreFoundation after fork, which is the known fork-unsafe abort. On every OS, os._exit drops the child's queued telemetry. Disable telemetry sends in compile children, or flush before _exit.

Comment thread reflex/compiler/disk_cache.py Outdated
Comment thread reflex/compiler/disk_cache.py
Comment thread reflex/compiler/page_cache.py
Comment thread reflex/compiler/page_cache.py
Comment thread reflex/compiler/compiler.py Outdated
Comment thread packages/reflex-base/src/reflex_base/plugins/compiler.py Outdated
Comment thread reflex/compiler/disk_cache.py
Comment thread reflex/utils/compile_daemon.py
Comment thread reflex/utils/compile_daemon.py
Comment thread reflex/utils/compile_daemon.py

@FarhanAliRaza FarhanAliRaza left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Full review at 3565bb6, part 2 of 3 (inline comments continue from the first part).

Comment thread reflex/utils/compile_daemon.py Outdated
Comment thread reflex/compiler/disk_cache.py
Comment thread reflex/compiler/page_cache.py Outdated
Comment thread reflex/utils/compile_daemon.py Outdated
Comment thread reflex/compiler/page_cache.py
Comment thread reflex/compiler/page_cache.py
Comment thread reflex/compiler/page_cache.py
Comment thread reflex/compiler/disk_cache.py Outdated
Comment thread packages/reflex-base/src/reflex_base/plugins/compiler.py Outdated
Comment thread reflex/compiler/disk_cache.py Outdated

@FarhanAliRaza FarhanAliRaza left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Full review at 3565bb6, part 3 of 3 (inline comments continue from the first part).

Comment thread reflex/utils/compile_daemon.py Outdated
Comment thread reflex/utils/compile_daemon.py Outdated
Comment thread reflex/compiler/page_cache.py
Comment thread reflex/utils/path_ops.py
Comment thread packages/reflex-base/src/reflex_base/utils/serializers.py
Comment thread packages/reflex-base/src/reflex_base/plugins/compiler.py Outdated
Comment thread packages/reflex-base/src/reflex_base/plugins/compiler.py
Comment thread reflex/compiler/compiler.py
Comment thread reflex/utils/telemetry.py Outdated
Comment thread reflex/compiler/page_cache.py Outdated
…e read entry points

Reads made while a page evaluates are now also attributed to the
first-party module whose code performed them, and every page whose import
closure holds that module depends on them. A read behind a shared cache
(functools.cache, a module-level loader) happens once per process, for the
first page only; before this the other pages never saw the file change.

A dependency modified at or after the compile started imported the app is
recorded as racy and always misses the next compile: the compile may have
read earlier content than the manifest hashed.

Also: exclusions apply to the parts below the project root, so a project in
a directory named venv and reads under assets/ are tracked; io.open,
tokenize.open (linecache, inspect.getsource) and io.open_code (pkgutil) are
recorded; every import counts in a slice, and a slice reaching its module
by a computed name (globals, sys, inspect) depends on the whole file; a
dependency that was absent is recorded as such instead of missing on every
compile; mark_volatile() lets a page opt out of reuse; dead code and the
per-read root resolution are gone.

Claude-Session: https://claude.ai/code/session_01RgKnnN8CCBqaR58qm23zQp
…et fall back instead of reconstructing

Auto memos no longer mirror their page's module into one shared file. Each
is content-addressed by its tag and compiles to its own file under
utils/components, so identical subtrees anywhere in the app share one
definition and a recompiled page's memo files never hold another page's
memos. A page records the memo keys it uses (not the ones it registered
first), the memo manifest is updated by incremental compiles, and auto memo
files no page uses any more are dropped. The module-sibling expansion and
the has_memos flag are gone with the bug they worked around: a shared memo
owned only by the first page, whose edit dropped an export a reused page
still imported.

The app root is compared against its recorded inputs (the merged app wraps
in page order, the bundled libraries) and a change falls back to a full
compile that adopts the pages already evaluated. The pickled wrap table,
the bundle request recording and the library-order reconstruction are
deleted.

The page registry is an input of every page (a navigation built from the
registered pages is common): a page added, removed, reordered or registered
differently now falls back to a full compile, which deletes the route-set,
removed-page and page-order machinery. Pages added to a rebuild for any
reason are evaluated in page order.

Also: a reused page's Radix use is recorded and enables the implicit plugin
so an all-hit rebuild keeps the Radix stylesheet and theme; a recompiled
page no longer claims a state a reused page recorded; the daemon drops the
memos of purged modules before re-importing them; the refreshed manifest is
staged as the last output and the old one removed before any output is
written; absorbed contexts keep page order for the stateful routes.

Claude-Session: https://claude.ai/code/session_01RgKnnN8CCBqaR58qm23zQp
…ates and processes

The post-page phase no longer reseeds the import-time sequence: it draws
from its own sequence with the names drawn before the process's first
compile reserved, so a component created after a compile cannot repeat a
module-level component's ref name. A state's default values draw names
from a sequence seeded by the state's full name, so the compile cache can
merge values recorded by different processes without duplicate ids. The
backend's state-registering page evaluation seeds per route like the
compiling process. The compile daemon's child resets the generator before
re-importing first-party code, so it draws the names a cold process draws.
The per-page generator snapshot is dropped once the page rendered.

Claude-Session: https://claude.ai/code/session_01RgKnnN8CCBqaR58qm23zQp
…ecode cache, worker-only deferral, restart by exit code, locks outside .web, child telemetry via the daemon

The child's registry reset now keeps a reflex.istate.dynamic state that a
surviving (non-first-party) module created at import, and drops the ones
the manifest records as defined by a page, which re-evaluates. The daemon
and its children write bytecode to a private cache and drop the entries of
the changed files before re-importing them, so a second same-size edit
within a second no longer imports stale bytecode. Only backend reload
workers defer to a running daemon: reflex compile and reflex export compile
themselves. The watcher tests skip directories below the project root, so
a project under a directory named venv is watched. A daemon stopped
mid-compile kills its child on the way out, and a child dies with its
parent on Linux. A global config change exits with a restart code that
reflex run answers by starting a new daemon, on every OS, instead of an
in-place execv. The lock files live in the per-user data directory, so
rm -rf .web cannot unlink a held lock. A compile child spools its telemetry
events for the daemon to send once it exited, which keeps the forked child
free of the telemetry thread and macOS's post-fork proxy lookup; the
duplicated executor shutdown is gone.

Also: a cold compile no longer forks a predicting child when there is no
manifest, and takes its component counts from the manifest when the child
compiled; sets are sorted only in compiled literals, not on the state
delta path.

Claude-Session: https://claude.ai/code/session_01RgKnnN8CCBqaR58qm23zQp
@masenf masenf removed the on deck PRs lined up to review / merge next label Oct 21, 2026
Comment thread tests/units/utils/test_compile_daemon.py
Comment thread reflex/compiler/page_cache.py
Comment thread reflex/compiler/disk_cache.py Outdated
Comment thread reflex/compiler/disk_cache.py Outdated

@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 30 files (changes from recent commits).

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

View guided diff | Re-trigger cubic

Comment thread reflex/compiler/page_cache.py
Comment thread reflex/compiler/disk_cache.py Outdated
Comment thread reflex/compiler/disk_cache.py Outdated
Comment thread tests/units/compiler/test_disk_cache.py
Comment thread tests/units/utils/test_serializers.py Outdated
Comment thread tests/units/compiler/test_compiler.py Outdated
Comment thread reflex/compiler/disk_cache.py
Comment thread tests/units/compiler/test_compiler_utils.py Outdated
Comment thread reflex/utils/compile_daemon.py
Comment thread reflex/utils/telemetry.py Outdated
- Skip every fork-based compile daemon test without os.fork, so the
  Windows unit suite does not raise AttributeError.
- A volatile page misses even when the watcher's changed set misses its
  dependencies.
- Record a page's Radix use from its auto memos too: memoization replaces
  a stateful Radix component in the page tree with its wrapper.
- Record every memo the app root imports, including those a page shares,
  so a page dropping a shared memo no longer deletes the root's import.

Claude-Session: https://claude.ai/code/session_01PXcFKAgnzhgdnSvhuCmxWE
- SIGTERM during the daemon's initial compile stops the daemon instead of
  being logged as a failed compile (a dedicated `_Terminated` exit passes
  the failure handler).
- A manifest refresh that raises no longer fails an incremental rebuild:
  it is logged and the manifest is removed, so none vouches for the new
  outputs, like `write_manifest`.
- Removing the telemetry spool is best-effort, so it cannot fail a compile.
- Tests: restore the unique-name state and the reserved names, delete the
  temporary `get_substates` instead of rebinding it, cover a set iterating
  out of sorted order, assert the manifest is the last write and fail a
  write after the page, patch `os.fork` on Windows too, and keep the plugin
  output test in process so it observes the outputs.

Claude-Session: https://claude.ai/code/session_01PXcFKAgnzhgdnSvhuCmxWE
Content-addressed auto memos moved the page's compilable memos out of the
user memo module, leaving only DerivedCount, which React Compiler never
compiled: its id renders a ref, written into the global refs during
render. A class name keeps the lookup without the ref.

Claude-Session: https://claude.ai/code/session_01PXcFKAgnzhgdnSvhuCmxWE
Comment thread reflex/utils/telemetry.py
Comment on lines +673 to +674
with suppress(OSError):
spool.unlink(missing_ok=True)

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.

P2 Compile events get counted twice

If deleting the spool fails, send_spooled() now sends its events but leaves them in the file. Each later compile in the same daemon appends to that same filename, so the next compile sends the old events again. A temporary Windows file lock can therefore inflate usage counts.

Keep cleanup errors harmless, but clear the sent records or use a separate spool for each compile.

@masenf masenf added the roadmap https://github.com/reflex-dev/reflex/issues/2727 label Oct 23, 2026

This branch has not been deployed

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

Labels

perf Performance-improving changes roadmap https://github.com/reflex-dev/reflex/issues/2727

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants