Skip to content

fix: remove useless AST second time parsing#14

Merged
PhysShell merged 1 commit into
mainfrom
claude/p016-b0a-direct-module
Jun 16, 2026
Merged

fix: remove useless AST second time parsing#14
PhysShell merged 1 commit into
mainfrom
claude/p016-b0a-direct-module

Conversation

@PhysShell

@PhysShell PhysShell commented Jun 16, 2026

Copy link
Copy Markdown
Owner

Summary by CodeRabbit

  • Refactor
    • Optimized internal resource declaration processing by directly constructing abstract syntax trees from JSON facts instead of generating source code and re-parsing.
    • Enhanced resource acquisition and release workflow handling to more accurately track resource subscriptions.
    • Streamlined resource subscription processing by eliminating unnecessary intermediate parsing steps.

@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, you can upgrade your account or add credits to your account and enable them for code reviews in your settings.

@coderabbitai

coderabbitai Bot commented Jun 16, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 8b342ed6-eea7-46e4-82fe-d90a7ceb963b

📥 Commits

Reviewing files that changed from the base of the PR and between 33bfe5b and 152f36e.

📒 Files selected for processing (1)
  • ownlang/ownir.py

📝 Walkthrough

Walkthrough

ownlang/ownir.py replaces the .own source-text generation and re-parse round-trip with direct Module AST construction. It adds _prelude_resources() and a new public to_module() that lowers OwnIR facts into Let/Acquire/Release AST nodes, and updates check_facts() to call check_module() on the resulting AST.

Changes

OwnIR direct AST lowering

Layer / File(s) Summary
AST node imports and to_module() implementation
ownlang/ownir.py
Imports Acquire, FnDecl, Let, Module, Release, ResourceDecl, ResourceMember, Stmt from .ast_nodes; adds _prelude_resources() to build ResourceDecl lists and to_module() to lower each owned-subscription fact into Let(handle, Acquire(Resource)) with optional Release, skipping unresolved-subscription entries; returns the Module plus a handle→fact map.
check_facts() wired to direct AST path
ownlang/ownir.py
Imports check_module, calls to_module(facts) to obtain the Module AST, and passes it to check_module(); removes the prior to_own() + _collect() source-generation and parse-failure code path.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐇 No more strings to parse and fret,
The AST is built direct!
Acquire, Release, Let in line,
Each resource node is yours and mine.
The rabbit hops the parse-free way—
OwnIR lowered here to stay! 🌿

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: eliminating redundant AST parsing by constructing the Module AST directly from JSON facts instead of generating source text and re-parsing it.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch claude/p016-b0a-direct-module

Comment @coderabbitai help to get the list of available commands and usage tips.

@PhysShell PhysShell merged commit 8323c77 into main Jun 16, 2026
17 checks passed
This was referenced Jun 16, 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.

1 participant