Record the installed positronic revision in an episode, not the working tree's - #740
Record the installed positronic revision in an episode, not the working tree's#740v-positronic wants to merge 1 commit into
Conversation
…ng tree's An episode's `writer.git` and a run's `git.positronic` read the git state around the process: the working directory, and the repository that contains `site-packages`. On a rig that runs an installed wheel from a venv inside a checkout, both name that checkout. Every episode then carries the same stale sha, and a recording cannot say which code wrote it. `get_package_git_state` reads the installed distribution's `direct_url.json` (PEP 610). A wheel built from a VCS URL names the commit it was built from. An editable install names the checkout it imports from. Any other install records no revision. `git.current` still records the working directory. Ticket: Positronic-Robotics/internal#1295 #refs
![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: c82efaf1a9
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| def git_repo(path: Path) -> str: | ||
| path.mkdir(parents=True, exist_ok=True) | ||
| env = {'GIT_AUTHOR_NAME': 't', 'GIT_AUTHOR_EMAIL': 't@t', 'GIT_COMMITTER_NAME': 't', 'GIT_COMMITTER_EMAIL': 't@t'} | ||
| run = lambda *args: subprocess.run(['git', '-C', str(path), *args], check=True, capture_output=True, env=env) # noqa: E731 |
There was a problem hiding this comment.
Rule grandfathered-violation violated:
The newly added git_repo helper silences E731 with # noqa even though new files must land without suppressions; replace the assigned lambda with a small local def run(...) so Ruff can check the code normally.
AGENTS.md reference: AGENTS.md:L7-L8
Useful? React with 👍 / 👎.
| return get_git_state(workdir=checkout) if checkout is not None else None | ||
|
|
||
|
|
||
| def _direct_url(distribution: str) -> dict | None: |
There was a problem hiding this comment.
_direct_url above its first caller
Rule stranded-definition violated:
_direct_url is defined after both functions that use it, forcing readers to search forward to understand get_package_checkout; place this private helper directly above its first caller.
AGENTS.md reference: AGENTS.md:L7-L8
Useful? React with 👍 / 👎.
1 participant
Fixes Positronic-Robotics/internal#1295.
The defect
An episode's
writer.gitreads the git state of the working directory. A run'sgit.positronicreads the git repository that containssite-packages. On a rig that runs an installed wheel from a venv inside a checkout, both name that checkout. Every episode then carries the same stale sha (e1455be2, dirty), and a recording cannot say which code wrote it.The change
positronic.utils.git.get_package_git_statereads the installed distribution'sdirect_url.json(PEP 610):dirty: False, the URL, and the requested revision. This is what the rig runs.DiskEpisodeWriterwrites this aswriter.git.run_metadatawrites it asgit.positronic, and writesgit.positronic.diffonly for an editable checkout.git.currentstill records the working directory.writer.giton a wheel carries nobranchkey. The one reader in the tree (test_episode.py) requiredcommitanddirty; it now requires only those.Tests
test_episode_written_by_an_installed_wheel_records_that_wheel_revisionfails onmainand passes here.positronic/utils/tests/test_git.pycovers the wheel, the editable install, an install with no origin, andrun_metadataon a wheel.Left out