Repository navigation
feat(thermo): add BijectiveResiduationExecutor Stage 1 prototype for … - #12
Igor Holt (igor-holt) wants to merge 1 commit into
Conversation
…total bijective execution traces. Wired example in integration_example.py. Non-critical path for crystal-scoring benchmark hook.
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code review is billed via overage credits. To resume reviews, an organization admin can raise the monthly limit at claude.ai/admin-settings/claude-code.
Once credits are available, push a new commit or reopen this pull request to trigger a review.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Igor Holt (igor-holt)
left a comment
Igor Holt (igor-holt)
left a comment
There was a problem hiding this comment.
Claude (@claude) @Genesis-Conductor-Engine/braintrust
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a prototype for total bijective state transitions with the BijectiveResiduationExecutor class and an integration example. The code review highlights several critical issues: the initial state is not stored in the state space, which causes the first inversion to fail; invert_last() does not remove the inverted transition from history, preventing backward traversal; the hash chain is non-deterministic due to the inclusion of timestamps; and the example script uses an incorrect import path.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| new_state = operation(state) | ||
| new_hash = self._compute_hash(new_state, self.current_hash) | ||
|
|
||
| # Record as invertible transition (residuation pair stored implicitly via hash chain) | ||
| self.history.append((self.current_hash, op_name, time.time())) | ||
| self.state_space[new_hash] = new_state | ||
| self.current_hash = new_hash | ||
| return new_state, new_hash |
There was a problem hiding this comment.
The initial state (associated with "genesis_root") is never stored in self.state_space. As a result, calling invert_last() on the very first transition will raise a ValueError: State not found in closed space because self.state_space.get("genesis_root") returns None. To fix this, store the initial state in self.state_space under self.current_hash if it is not already present before applying the operation.
| new_state = operation(state) | |
| new_hash = self._compute_hash(new_state, self.current_hash) | |
| # Record as invertible transition (residuation pair stored implicitly via hash chain) | |
| self.history.append((self.current_hash, op_name, time.time())) | |
| self.state_space[new_hash] = new_state | |
| self.current_hash = new_hash | |
| return new_state, new_hash | |
| if self.current_hash not in self.state_space: | |
| self.state_space[self.current_hash] = state | |
| new_state = operation(state) | |
| new_hash = self._compute_hash(new_state, self.current_hash) | |
| # Record as invertible transition (residuation pair stored implicitly via hash chain) | |
| self.history.append((self.current_hash, op_name, time.time())) | |
| self.state_space[new_hash] = new_state | |
| self.current_hash = new_hash | |
| return new_state, new_hash |
| """Invert last transition — demonstrates total bijective property.""" | ||
| if not self.history: | ||
| raise ValueError("No history to invert") | ||
| prev_hash, op_name, _ = self.history[-1] |
There was a problem hiding this comment.
invert_last() currently retrieves the last transition using self.history[-1] but does not remove it from self.history. This means that subsequent calls to invert_last() will repeatedly attempt to invert the same transition instead of traversing further back in history, and the history list will become inconsistent with self.current_hash. Use self.history.pop() to correctly remove the inverted transition from the history.
| prev_hash, op_name, _ = self.history[-1] | |
| prev_hash, op_name, _ = self.history.pop() |
| def _compute_hash(self, state: Any, prev_hash: str) -> str: | ||
| payload = f"{prev_hash}:{str(state)}:{time.time_ns()}" | ||
| return hashlib.sha256(payload.encode()).hexdigest() |
There was a problem hiding this comment.
The hash computation includes time.time_ns(), which makes the hash chain non-deterministic. For a hash-linked state space intended for attestation and observability, the hashes should be deterministic and reproducible based solely on the state transitions. Consider removing the timestamp from the hash payload.
| def _compute_hash(self, state: Any, prev_hash: str) -> str: | |
| payload = f"{prev_hash}:{str(state)}:{time.time_ns()}" | |
| return hashlib.sha256(payload.encode()).hexdigest() | |
| def _compute_hash(self, state: Any, prev_hash: str) -> str: | |
| payload = f"{prev_hash}:{str(state)}" | |
| return hashlib.sha256(payload.encode()).hexdigest() |
| # Example wiring of BijectiveResiduationExecutor into a non-critical path | ||
| # (e.g., monitoring hook or crystal-scoring benchmark extension) | ||
|
|
||
| from thermo_bijection_layer import BijectiveResiduationExecutor |
There was a problem hiding this comment.
The import statement assumes thermo_bijection_layer is in the root search path. Since the file is located in the core/ directory, running this example directly will result in a ModuleNotFoundError. Update the import to use the correct package path.
| from thermo_bijection_layer import BijectiveResiduationExecutor | |
| from core.thermo_bijection_layer import BijectiveResiduationExecutor |
![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: 913ae1521d
ℹ️ 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 (@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 (@codex) address that feedback".
| # Example wiring of BijectiveResiduationExecutor into a non-critical path | ||
| # (e.g., monitoring hook or crystal-scoring benchmark extension) | ||
|
|
||
| from thermo_bijection_layer import BijectiveResiduationExecutor |
There was a problem hiding this comment.
Running this example as checked in (python examples/thermo_integration_example.py from the repo root) fails before exercising the prototype because thermo_bijection_layer.py lives under core/, and that directory is not on Python's module search path for scripts in examples/. Use an import/path layout that matches the repository structure so the example remains runnable.
Useful? React with 👍 / 👎.
| self.history.append((self.current_hash, op_name, time.time())) | ||
| self.state_space[new_hash] = new_state |
There was a problem hiding this comment.
On the first apply(), self.current_hash is genesis_root, but only the post-operation state is stored under new_hash; the incoming state is never stored under genesis_root. As a result, any immediate invert_last() after a single transition looks up the root hash and raises ValueError, so the advertised one-step recovery path cannot work.
Useful? React with 👍 / 👎.
| prev_hash, op_name, _ = self.history[-1] | ||
| prev_state = self.state_space.get(prev_hash) | ||
| if prev_state is None: | ||
| raise ValueError("State not found in closed space") | ||
| self.current_hash = prev_hash |
There was a problem hiding this comment.
When rolling back a multi-step trace, this always reads self.history[-1] but never removes that entry, so after reverting h2 -> h1, a second invert_last() reads the same last transition again and leaves the executor stuck at h1 instead of continuing to the previous state. This also leaves the execution trace describing transitions that have already been undone.
Useful? React with 👍 / 👎.
| new_state = operation(state) | ||
| new_hash = self._compute_hash(new_state, self.current_hash) | ||
|
|
||
| # Record as invertible transition (residuation pair stored implicitly via hash chain) | ||
| self.history.append((self.current_hash, op_name, time.time())) | ||
| self.state_space[new_hash] = new_state |
There was a problem hiding this comment.
If an operation mutates the supplied state in place, the previous snapshot stored under its hash is mutated as well because the executor keeps the same object reference in state_space. In a trace such as h1 = apply(...) followed by an in-place second operation on h1's returned dict, invert_last() returns the already-mutated dict instead of the pre-transition state, so rollback is not actually bijective for mutable states.
Useful? React with 👍 / 👎.
|
|
||
| def apply(self, operation: Callable[[Any], Any], state: Any, op_name: str = "unnamed") -> Tuple[Any, str]: | ||
| """Apply operation as total bijection. Returns (new_state, new_hash).""" | ||
| new_state = operation(state) |
There was a problem hiding this comment.
When operation(state) raises, apply() exits before recording any transition or failure state, so the promised invertible failure path is lost. This matters for wrapped hooks that throw after partially changing mutable or external state: there is no history entry to inspect or invert, and the executor cannot distinguish the failed transition from one that never ran.
Useful? React with 👍 / 👎.
…total bijective execution traces. Wired example in integration_example.py. Non-critical path for crystal-scoring benchmark hook.