Skip to content

reregister memo in app harness - #6799

Draft
benedikt-bartscher wants to merge 8 commits into
reflex-dev:mainfrom
benedikt-bartscher:fix-memo-harness
Draft

benedikt-bartscher wants to merge 8 commits into
reflex-dev:mainfrom
benedikt-bartscher:fix-memo-harness

Conversation

@benedikt-bartscher

Copy link
Copy Markdown
Contributor

No description provided.

@greptile-apps

greptile-apps Bot commented Jul 18, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerView in GreptileConfidence Score: 4/5

[Medium impact] The changes affect app loading in the test harness and memo compilation.

The PR still needs the existing-app registration-context issue fixed before merging.

Findings

  1. P1 Real Apps Share Registration Context ▶

Summary

The PR preserves existing apps in AppHarness and restores shared @rx.memo definitions after the registry is cleared.

  • Existing apps keep their submodule pages and registrations in the harness.
  • Memo definitions return to the compiler after a harness clears their registry.

The unnumbered identifier thread was manually resolved without explanation.

Reviews (8) · Last reviewed commit: "Restore nested memos from cached bodies ..." · Reviewed by Greptile

@codspeed

codspeed Bot commented Jul 18, 2026 •

Copy link
Copy Markdown

Merging this PR will not alter performance

⚠️ 14 benchmarks spent significant time in system calls

System calls cannot be consistently instrumented, so they are not included in the measure, which understates the real cost. Please switch to the Walltime instrument to accurately measure system calls.

Measurement and system calls

✅ 148 untouched benchmarks


Comparing benedikt-bartscher:fix-memo-harness (377a848) with main (d3f4ab1)

Open in CodSpeed

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

@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: 40fed7de7d

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread packages/reflex-base/src/reflex_base/components/memo.py
@benedikt-bartscher
benedikt-bartscher marked this pull request as draft July 18, 2026 11:40
Resolve conflicts in tests/units/test_testing.py (keep the reload=False /
MEMOS-preserved assertions alongside main's new frontend-env test),
tests/units/components/test_memo.py (merge both sides' imports and appended
tests), and pyi_hashes.json (regenerated).

Adapt test_memo_reregisters_on_use_after_registry_clear to main's new memo
module output, which wraps the memo in a PURE IIFE that sets displayName.
Comment thread reflex/testing.py
Comment thread tests/units/components/test_memo.py
Comment thread reflex/testing.py
Comment on lines +282 to +286
self.app_instance, self.app_module = (
reflex.utils.prerequisites.get_and_validate_app(reload=False)
)
self.app_asgi = self.app_instance()
return

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.

P1 Real Apps Share Registration Context

If a test process starts a real-app harness after another app, this early return skips the registration-context fork. Importing the next app then tries to use a context already associated with the previous app, raising ReflexRuntimeError and preventing the test from starting. The real app’s registrations need to be preserved without reusing that context.

Comment thread packages/reflex-base/news/+memo-harness-reregister.bugfix.md Outdated
Comment thread packages/reflex-base/src/reflex_base/components/memo.py
Comment thread tests/units/components/test_memo.py Outdated
Re-run a memo's cached body when re-registering it after a registry
clear so nested memos come back, make the pre-pass test fail without
the fix, reload a real app's config before initializing its frontend,
and reword the fragments for users.

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

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant