Repository navigation
feat(bazi): add isolate-local Shen Sha catalog bindings - #35
Conversation
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8895f0fbca
ℹ️ 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".
| import 'package:test/test.dart'; | ||
|
|
||
| int evaluateInIsolate() { | ||
| final astro = Ephemeris.open().createContext(); |
There was a problem hiding this comment.
When this test starts four Isolate.run workers concurrently, each worker executes Ephemeris.open(), which reinitializes the process-wide native runtime. One worker can therefore reset shared runtime state while another is creating or using its context, making this callback-isolation test race or fail for reasons unrelated to Shen Sha. Initialize the runtime once in the main isolate and use Ephemeris.attach() in each worker, as the existing BaZi concurrency test does.
Useful? React with 👍 / 👎.
| ); | ||
| return BaziShenShaCatalog._( | ||
| _host, | ||
| _ShenHandle(_handle.module, out.value.cast(), false, _handle.callbacks), |
There was a problem hiding this comment.
After removeModule(label) creates a catalog without that module's rules, this line still retains every callback from the source catalog. Consequently, a long-lived catalog produced through repeated add/remove cycles permanently pins removed predicates and anything their closures capture until the final descendant closes. It also makes the busy checks treat contexts as sharing callbacks that are no longer present in one context, causing otherwise independent nested evaluations to be rejected. Track callbacks by module and omit the removed module's callbacks from the new handle.
Useful? React with 👍 / 👎.
| /// and callbacks must not be shared across isolates. | ||
| final class BaziShenShaCatalog implements Finalizable { | ||
| factory BaziShenShaCatalog({String? libraryPath}) { | ||
| final host = TaiyinExtensionHost.open(); |
There was a problem hiding this comment.
When an application loads the core through Ephemeris.open(libraryPath: ...) because no compatible core is discoverable at the default location, this constructor ignores that path and independently calls TaiyinExtensionHost.open() with no override. Catalog creation can therefore fail before the explicitly selected BaZi module is opened, even though the application already has valid custom core and extension libraries. Accept a separate coreLibraryPath and pass it here, matching the standalone ZiweiDataCatalog API.
Useful? React with 👍 / 👎.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fbaaa6e4fd
ℹ️ 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".
| validateTaiyinRequiredSymbols( | ||
| providesSymbol: module.library.providesSymbol, | ||
| requiredSymbols: taiyinBaziShenShaSymbols, | ||
| ); |
There was a problem hiding this comment.
When callers use the package-bundled BaZi library—the documented default—this validation always rejects it because the checked-in Linux module exports none of these beta12 symbols (verified with nm), so BaziShenShaCatalog() cannot be constructed without an external override. The native-integration workflow also still checks out and stages v1.0.0-beta.11 in .github/workflows/native-integration.yml, meaning its BaZi tests will exercise the same incompatible ABI. Update/stage the native dependency and bundled artifacts alongside this facade.
Useful? React with 👍 / 👎.
1 participant
Summary
Validation and release gate