Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Merging this PR will not alter performance
Comparing 
Footnotes
|
 The PR appears safe to merge; no actionable new issue or outstanding previous finding remains.
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
All reported issues were addressed across 21 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
There was a problem hiding this comment.
All reported issues were addressed across 11 files (changes from recent commits).
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
4da5154 to
b923072
Compare
| constants.Reflex.VERSION, | ||
| [str(arg) for arg in command], | ||
| runtimes, | ||
| sorted(os.environ.items()), |
There was a problem hiding this comment.
Hash only the environment that reaches Vite, not all of os.environ.
With the full environment in the key, ordinary variation between two runs of the same command defeats the cache. Reproduced on an unchanged app, each case rebuilt and replaced the snapshot, so the next plain run missed again:
reflex export --loglevel debug(setsREFLEX_LOGLEVEL)OLDPWD=/tmp reflex export- the same command in the foreground and in a background subshell (
SHLVLdiffers)
In CI, a per-run variable such as a job id makes the cache never hit. Hash an allowlist (for example PATH, NODE_*, NPM_CONFIG_*, VITE_*, REFLEX_* minus REFLEX_LOGLEVEL), or the explicit env dict that _build_frontend passes to the subprocess.
| raise ValueError(msg) | ||
| digest = hashlib.sha256() | ||
|
|
||
| def visit(directory: Path) -> None: |
There was a problem hiding this comment.
Split visit by entry type.
_tree_digest measures cyclomatic complexity 15 and visit 13 (ruff --select C901 at max 8). One helper per entry kind (symlink, node_modules metadata, reflex.json, regular file) keeps each path readable and testable on its own.
| return Path(package_manager).stem.lower() == "bun" | ||
|
|
||
|
|
||
| def _npm_installed_package_sections( |
There was a problem hiding this comment.
Extract the per-package verification into a helper.
This function measures cyclomatic complexity 13. The loop body is a chain of guard clauses that reads better as _verified_installed_version(name, declaration, entry) -> str | None, with this function reduced to the loop and the assignment.
| progress.advance(task) | ||
| zipf.write(file, Path(file).relative_to(root_directory)) | ||
| # Sidecars are already compressed for serving the frontend. | ||
| compress_type = ( |
There was a problem hiding this comment.
Move the compress-type choice into a small helper.
_zip now measures cyclomatic complexity 9. It was under 8 on main. A helper such as _zip_compress_type(component_name, file) keeps the write loop flat.
| include_db_file=upload_db_file, | ||
| backend_excluded_dirs=backend_excluded_dirs, | ||
| ) | ||
| with build_cache.frontend_build_lock(prerequisites.get_web_dir()): |
There was a problem hiding this comment.
Take the lock only when frontend is true.
A backend-only export in a project with no .web now creates a .web directory that contains only .reflex-build.lock. Reproduced with reflex export --backend-only after removing .web.
| { | ||
| _version_check_timestamp_key(package_name): datetime.now( | ||
| timezone.utc | ||
| ).isoformat() |
There was a problem hiding this comment.
Use isoformat() for the attempt timestamp as well.
The attempt key at line 193 still writes str(now), so reflex.json mixes 2026-09-10 18:19:33+00:00 and 2026-09-10T18:19:34+00:00. Both parse, but one format is simpler to reason about.
FarhanAliRaza
left a comment
FarhanAliRaza
left a comment
There was a problem hiding this comment.
Tested with a two-page counter app that has one static asset, built against this branch.
Ran reflex export with and without zip, with the cache enabled and disabled, with bun and with npm. Repeat exports with the cache enabled skipped Vite and restored the snapshot. Setting the variable to false rebuilt and removed the cache directory. Two concurrent exports serialized on the lock, and the second one reused the first one's snapshot when the environment was identical. The frontend zip stored the .gz sidecars and deflated the other entries.
Ran reflex run --env prod with the cache enabled. The restart restored the cached build. In the browser the counter updated over the websocket, the asset loaded, page navigation worked, and gzip content negotiation returned compressed assets. No console errors.
The unit tests for the touched files pass.
The inline comments hold the requested changes. The first one, about the cache key, is the one that matters most: the same command with a different log level, a different OLDPWD, or a different SHLVL rebuilt every time.
2 participants
Repeated deploys redo frontend dependency and archive work, and unchanged production frontends always rerun Vite. This PR reuses matching installed framework pins, avoids unnecessary manifest rewrites and sidecar recompression, and adds opt-in reuse of deterministic production frontend builds.
Changes
--save-dev, and include development dependencies underNODE_ENV=production.REFLEX_FRONTEND_BUILD_CACHE=true, disabled by default. Cache pristine Vite output and replay post-build plugins, fallback handling, compression, and frontend-path processing. Validate inputs/output, invalidate changed inputs, fall back on unsupported/corrupt snapshots, and publish only after a successful complete build.Current rebase validation (
87ea648d)Rebased onto
mainat77cfe61bon September 8, 2026. The rebase preserves main's lazy CLI startup, dependency-only install fingerprint, per-package version checks, and failure cooldown.87ea648dfollow-up only consolidates documentation and adds duration comments. Its Python AST is identical to the fully testedb9230726revision.Historical local deploy measurements (
93c8683, before rebase)These measurements predate this rebase. Main now includes independent startup and dependency-cache improvements, so these numbers are not a performance comparison against current main.
Same nine-page dashboard, two warmups and three fresh-process samples per variant, rotating run order. Times include imports, preparation, compilation/dependency handling, build or restore, postprocessing/compression, and both ZIPs. They exclude cloud authentication/provider APIs, upload, remote builds/rollout, optional version requests, telemetry delivery, interpreter teardown, and artifact verification.
All unchanged candidate runs skipped dependency-manager commands; enabled-cache runs also skipped Vite. Python hash seeds vary between interpreters while the child build environment remains constant.
Cache misses add about 0.8 seconds over the new default. The cache is for repeat deterministic builds on local macOS/Linux filesystems with reliable file timestamps; Windows bypasses it. External/clock/remote build inputs and excluded generated caches require a fresh build. Set
REFLEX_FRONTEND_BUILD_CACHE=falseto discard the snapshot and rebuild. Source/snapshot contents are hashed; installed dependencies use identity, size, mtime, and ctime. Documentation covers ignored private telemetry timestamps and symlink handling.Measured on macOS arm64, Python 3.14.5, Node 26.0.0, npm 11.12.1, Bun 1.4.0 against base
c49a85df9c41ddffbe9fc2ac37d495d363f4e313. No competing builds/tests ran during timing. These are warm local results, not total cloud deploy latency. Small differences can be noise. The original npm changed-URL samples were 8.489, 5.787, and 5.776 seconds; all are retained in the median. Avoiding sidecar recompression increased the earlier dashboard ZIP by 0.063%; upload impact was not measured.Earlier validation before rebase
dev/devOptionalflags while requiring other manifest/lock metadata and resolved versions to agree..webrestoration and npmNODE_ENV=production.pyright reflex tests, whitespace checks, and patch application against the base pass. All applicable pre-commit hooks pass, including codespell, stub generation, Pyright, and ty. Independent review identified and resolved serializer and symlink edge cases. Regression tests, documentation, and news fragments for each touched package are included.Full cloud deployment and arbitrary concurrent workspace writers (initialization/dev hot reload/external tools) were not exercised. The new concurrency regressions use separate processes with controlled compiler/build/ZIP test doubles; production behavior is also checked by the Bun/npm integration tests.
Earlier review follow-up (
4da5154, before rebase)Candidate-only follow-up on the same warmed unchanged-frontend fixture (two warmups, three fresh-process runs per variant, alternating order):
All measured repeats skipped dependency-manager commands; enabled-cache repeats skipped Vite and reran compression. The original baseline and changed-URL cases were not rerun in this follow-up; the earlier table remains the initial revision's comparison. Timing still covers local preparation through ZIP creation, excluding cloud and interpreter teardown.