Skip to content

fix(pyi): incremental .pyi regen now follows the import graph#6422

Open
FarhanAliRaza wants to merge 7 commits intoreflex-dev:mainfrom
FarhanAliRaza:fix-pyi-generation
Open

fix(pyi): incremental .pyi regen now follows the import graph#6422
FarhanAliRaza wants to merge 7 commits intoreflex-dev:mainfrom
FarhanAliRaza:fix-pyi-generation

Conversation

@FarhanAliRaza
Copy link
Copy Markdown
Contributor

Fixed

  1. Fixed partial generation, before it dropped the unchanged in files.
  2. fixed the inherited changes that were not happening before.

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

Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)

New Feature Submission:

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

…l files

Replace the .pyi_generator_last_run/.pyi_generator_diff sentinel files with
the last commit that touched pyi_hashes.json as the regen baseline, and
expand the change set along the import graph so editing a parent class also
regenerates subclass stubs. Set pass_filenames: false on the pre-commit hook
and rewrite scan_all's pyi_hashes.json update as a per-file evidence-based
merge so partial-target runs no longer truncate the file.
@FarhanAliRaza FarhanAliRaza requested a review from a team as a code owner April 29, 2026 18:22
@codspeed-hq
Copy link
Copy Markdown

codspeed-hq Bot commented Apr 29, 2026

Merging this PR will degrade performance by 4.29%

❌ 1 regressed benchmark
✅ 16 untouched benchmarks
⏩ 2 skipped benchmarks1

⚠️ Please fix the performance issues or acknowledge them on CodSpeed.

Performance Changes

Benchmark BASE HEAD Efficiency
test_get_all_imports_single_pass[_complicated_page] 1.6 ms 1.6 ms -4.29%

Comparing FarhanAliRaza:fix-pyi-generation (4062d22) with main (76ff38c)

Open in CodSpeed

Footnotes

  1. 2 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.

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented Apr 29, 2026

Greptile Summary

This PR overhauls the incremental .pyi stub regeneration pipeline: make_pyi.py is rewritten to use git log on pyi_hashes.json as the baseline commit, then expands the dirty set along the AST import graph so parent-class changes cascade to subclasses. pyi_generator.py fixes a mutable class-level written_files list and rewrites the hash-merge logic to correctly preserve unscanned entries while pruning stale ones.

Confidence Score: 4/5

Safe to merge; all findings are P2 style/hardening suggestions with no correctness-breaking defects on the happy path.

The logic is sound and well-tested. Three P2 notes: missing ast.TryStar handling (minor Python 3.11+ gap), the uncommitted-hashes loop (usability only), and the single-anchor heuristic replacing LCA (low risk with one root-level hashes file). No P0/P1 issues found.

scripts/make_pyi.py and packages/reflex-base/src/reflex_base/utils/pyi_generator.py contain all substantive logic changes.

Important Files Changed

Filename Overview
scripts/make_pyi.py Major rewrite: replaces SHA-file-based change detection with git-log baseline + AST import-graph expansion; ast.TryStar gap and uncommitted-hashes loop concern noted.
packages/reflex-base/src/reflex_base/utils/pyi_generator.py Fixes mutable class-level written_files (moved to init), resolves file targets, and rewrites hash-merge logic; anchor-from-first-file regression noted.
tests/units/reflex_base/utils/pyi_generator/test_hash_merge.py New test file covering key hash-merge scenarios: partial runs, dropped entries, missing hash file, and incremental merges. All cases appear logically correct.
.pre-commit-config.yaml Adds pass_filenames: false so the hook determines its own change set via git — consistent with the new make_pyi.py logic.
.gitignore Removes now-deleted .pyi_generator_last_run and .pyi_generator_diff transient files — correct cleanup.
AGENTS.md Updates .pyi stub instructions to reflect the new incremental workflow; removes stale references to .pyi_generator_last_run.

Reviews (1): Last reviewed commit: "fix(pyi): regenerate stubs from pyi_hash..." | Re-trigger Greptile

Comment thread scripts/make_pyi.py
Comment thread scripts/make_pyi.py
Comment thread packages/reflex-base/src/reflex_base/utils/pyi_generator.py Outdated
FarhanAliRaza and others added 4 commits April 29, 2026 23:55
… races

Add --no-update-hashes flag to pyi_generator and use it from both the
top-level and per-package hatch build hooks. The shared hash file is a
dev-tooling artifact; reading/writing it during parallel workspace
package builds caused races and isn't needed at wheel-build time.
Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
The hash file is workspace-scoped, so walking up from the first entry in
written_files made the resolved path depend on iteration order and could
latch onto a package-local file before reaching the repo root. Also warn
when pyi_hashes.json exists locally but isn't tracked in git, since that
forces a full regen on every run until it's committed.
Comment on lines +1607 to +1613
modules: list = []
root: str = ""
current_module: Any = {}
written_files: list[tuple[str, str]] = []

def __init__(self) -> None:
"""Initialize per-instance scan state."""
self.written_files: list[tuple[str, str]] = []
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

shouldn't the same be done to modules and current module?

Copy link
Copy Markdown
Contributor Author

@FarhanAliRaza FarhanAliRaza Apr 30, 2026

Choose a reason for hiding this comment

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

They need to be removed. They are not used.

Comment on lines +1718 to +1719
if use_json and (file_paths or file_targets):
file_paths = list(map(Path, file_paths))
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

this might be a good time to take PYI hash generation into its own function

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.

fixed

Move the pyi_hashes.json merge logic out of PyiGenerator.scan_all into a module-level _update_pyi_hashes_file() so scan_all reads as scan + ruff fixup + (optional) hash update. Remove unused class attributes (modules, root, current_module) that were never read or written, and collapse a duplicate _relative_to_pwd() call in the file walk.

Also fix test_tailwind flake: the stylesheet is now written to Path.cwd() / assets so it survives AppHarness's importlib.reload() across pytest reruns (the cached __file__ pinned writes to the prior tmp_path while the compiler read from the current cwd).
Copy link
Copy Markdown
Collaborator

@masenf masenf left a comment

Choose a reason for hiding this comment

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

i might be reading it wrong, but are we regenerating modules that are imported by modules that changed? the other fixes seem legit.

path.write_text(json.dumps(mapping, indent=2, sort_keys=True) + "\n")


def _make_workspace(root: Path) -> Path:
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

this should be a fixture that returns the workspace path based on the tmp_path fixture

@FarhanAliRaza
Copy link
Copy Markdown
Contributor Author

FarhanAliRaza commented May 1, 2026

For example if react_player is changed, in the main branch now it only generates react_player.pyi because it only sees this in git diff. But it should have also changed audio.pyi and video.pyi because they import react player amd inherit from it . Before partial generation missed it. Only full regneration catches that.

So we are regenerating that imports the changed modules.

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