refactor(checkers): let declarations own provider runtimes - #1299
Conversation
Refresh #1299 while preserving declaration-owned provider-runtime compatibility.
Coverage ReportTotal coverage: 77% (threshold: 50%) |
Refresh #1299 after the pytest-owned test-architecture cut without changing its declaration-owned runtime seam.
6a97690 to
2b4f61d
Compare
Synchronize #1299 before fixing the remaining stacked probability review feedback.
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2d44778228
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b33a9ff8e0
ℹ️ 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".
Compose declaration-owned clean-process runtimes with the latest batched checker-identity path. Existing checker families retain the legacy registry; new declarations may carry one unassigned provider runtime, and the composition root batches identity material across the full declaration set before authorization.
Store passive provider-runtime factories on exact replay declarations and realize each runtime once, at installation. Importing domain declarations no longer identifies or hashes checker source, while the existing installer and authorization boundary keep the same resolved runtime contract.
Retain the existing direct provider_runtime constructor as a temporary compatibility seam while making provider_runtime_factory lazy and cached. Reject dual ownership and preserve pre-authorization invariants for both forms.
b33a9ff to
3d9e943
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3d9e943739
ℹ️ 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".
Declare the compatibility accessor's dynamic return as Any so mypy accepts the dataclass field interception while the public provider_runtime field retains its precise declared type.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1922bc3be3
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1db4eb16ed
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d9b7d6ef58
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
jacobian/src/jacobian/checker_operations.py
Lines 174 to 177 in c55e4e7
When runtime construction occurs more than once in the same process, the first access permanently writes the probe onto the declaration; built-in declarations are module-level tuple members reused by every bundle build. If an optional dependency was initially unavailable and is later restored, subsequent authorizations therefore keep omitting it, while a changed available dependency is compared against the stale probe and rejected until the process restarts. Keep this cache installation-local so each authorization measures the current provider identity.
AGENTS.md reference: AGENTS.md:L137-L140
When a declaration-owned optional runtime is anything other than Python-FLINT, this diagnostic tells operators to install or repair Python-FLINT even though provider_runtime.provider identifies a different missing backend. This new generic optional-runtime path therefore provides an incorrect recovery direction; derive the hint from the declaration/runtime diagnostic or use a backend-neutral instruction.
AGENTS.md reference: AGENTS.md:L211-L214
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 18dc43b88b
ℹ️ 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".
| factory = _declaration_factory(declaration) | ||
| probe = probes.setdefault(factory, factory()) | ||
| factory = object.__getattribute__(declaration, "provider_runtime_factory") | ||
| probe = factory() if factory is not None else declaration.provider_runtime |
There was a problem hiding this comment.
Validate factory runtimes through the declaration accessor
When a custom factory returns a CapabilityProviderRuntime with nonempty checker_ids, this direct factory call bypasses the explicit rejection in ExactReplayCheckerDeclaration.__getattribute__. Installation then registers checker manifests containing those pre-bound IDs, while _authorized_provider_runtimes() overwrites only the returned summary with the IDs actually authorized, leaving inconsistent runtime identities and violating the declaration's no-preauthorization invariant. Resolve the runtime through declaration.provider_runtime or repeat the empty-checker_ids validation here.
AGENTS.md reference: AGENTS.md:L155-L159
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 567236fc93
ℹ️ 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".
| provider_runtime.provider in _OPTIONAL_EXACT_REPLAY_PROVIDERS | ||
| and source_available | ||
| ) | ||
| can_omit = optional and source_available |
There was a problem hiding this comment.
Report the actual optional provider recovery
When a declaration-owned optional runtime is not Python-FLINT, this generic omission branch still emits Install or repair the optional python-flint backend; for example, the newly supported direct-runtime path can identify any provider, and the component coverage itself uses jacobian.test-optional-declaration-checker. Operators therefore receive an irrelevant recovery direction. Derive the hint from the unavailable runtime or use provider-neutral remediation.
AGENTS.md reference: AGENTS.md:L211-L214
Useful? React with 👍 / 👎.
Summary
Remove the central checker-runtime registry as a requirement for new exact replay operations while preserving it for existing families.
ExactReplayCheckerDeclarationcarry an optional completeCapabilityProviderRuntime;Why
Recent semantic ports otherwise had to add a central string mapping and factory for every domain-owned checker module. That duplicates provider ownership in
exact_domain_checkers.pyand makes unrelated mathematical PRs conflict on one shared hub. The declaration already owns the entrypoint, replay method, reason, and verifier contract; owning its clean-process runtime completes that boundary.Architecture boundary
This is a compatibility-preserving migration seam, not another framework. Existing checker families continue to use the legacy registry. New declarations may provide one immutable runtime directly. The installer remains the only authority that grants checker IDs.
Checker identity is still measured in a batch across the complete declaration set: the composition root opens the batch and the exact-domain installer safely nests within it. Declaration-owned runtimes do not bypass manifest measurement or authorization.
Refresh and validation
The branch is one focused commit directly on current
mainat1c926ffa7bdb69ced3ae78112ffd2d21a03a8458. Unit coverage accepts an unassigned declaration runtime and rejects pre-authorized runtime identities. Hosted CI is authoritative for the complete checker installation, batched identity, and verification matrix.