Repository navigation
Fixed task layout, PR bodies hold the details, a deliverables setting; release 0.4.0 - #14
Conversation
…e 0.4.0 No report by default: the agent asks what the task should deliver and records it. Decisions, PRs and issues go in the README; deliverables and PR bodies link to it. Earlier task directories are never edited. Figure naming is left to sci-figures. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011aQpF4MA9v7dTVK1FqMMVJ
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. |
![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: 9471359652
ℹ️ 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".
| record the path. | ||
| 4. `analysis-workflow check layout`, then push. | ||
| 5. The worktree is removed only once no batch job submitted from it is still queued or running. | ||
| 5. Open the PR and add it to the README. Its body links the task README and says what this PR adds or changes, by |
There was a problem hiding this comment.
When creating the final PR, its number or URL is only available after step 4 has already pushed the branch. Adding that PR to the tracked README therefore creates a new local change, but the checklist never commits or pushes it, so following these steps leaves the PR without the required self-record. Explicitly commit and push the README update after PR creation.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 3c06a01: step 5 now opens the PR, adds it to the README and pushes again.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011aQpF4MA9v7dTVK1FqMMVJ
A hook holds task directories and READMEs to the task template, read from the template itself. The PR body, laid out by the analysis template's PR template, holds a PR's details; the README summarizes each PR and lists the session's jobs. Analysis moves forward: earlier task directories are never edited. `deliverables` sets the default a task proposes. Figure and compute-environment rules leave the plugin, and what the analysis template owns is linked, not restated. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011aQpF4MA9v7dTVK1FqMMVJ
Marius1311
changed the title
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e5da9ec7e4
ℹ️ 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".
| **Run** | ||
| - Every output path comes from `PATHS = task_paths(__file__)` in `_common.py`; `PATHS.ensure()` in the writer. | ||
| A bare relative path written from a worktree dies with the worktree. | ||
| - `PATHS = task_paths(__file__)` lives in `scripts/_common.py`; call `PATHS.ensure()` in the writer. |
There was a problem hiding this comment.
_common from its new scripts location
When a user follows the template's Running commands from the task directory, the python -c "from _common ..." command searches the task root, but this change moves _common.py into scripts/; consequently the documented LOGS=... command raises ModuleNotFoundError. Update the command to import scripts._common or otherwise put scripts/ on the import path.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in eed76ee: the template's Running section now runs from scripts/, where _common.py lives, so both the step and the -c import resolve it.
| allowed = {p.name for p in TEMPLATE.iterdir()} | output_dirs() | ||
| if top not in allowed: | ||
| return f"`{top}` is not part of a task directory, whose top holds only {', '.join(sorted(allowed))}." |
There was a problem hiding this comment.
When a benchmark uses the task-local workspace supported by guard_worktree_pixi.py and documented under Environments, writing analysis/.../bench_v1/pixi.toml is rejected because the allowed top-level names omit the manifest. This makes ordinary Write/Edit operations unable to create or maintain the supported workspace; include the task-local manifest files in the permitted layout.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in eed76ee: pixi.toml and pixi.lock are allowed at a task's top level, for the task-local environment the skill describes; covered by a test.
| for parent in path.parents: | ||
| if parent.name == "analysis": | ||
| return None | ||
| if TASK_DIR.match(parent.name): | ||
| return parent |
There was a problem hiding this comment.
If a valid task contains a nested directory whose artifact or package name ends in _vN, such as demo_v1/outputs/model_v1/config.json, this nearest-ancestor search treats model_v1 as a new task and rejects config.json as an invalid top-level entry. Since descendants of allowed directories are otherwise unrestricted, identify the actual task boundary rather than selecting a version-looking directory inside it.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in eed76ee: the task is the outermost _vN directory under analysis/, so demo_v1/outputs/model_v1/config.json belongs to demo_v1; covered by a test.
…ts outermost _vN Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011aQpF4MA9v7dTVK1FqMMVJ
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: eed76eeaff
ℹ️ 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".
| for i, parent in enumerate(parents): | ||
| if parent.name == "analysis": | ||
| return next((p for p in reversed(parents[:i]) if TASK_DIR.match(p.name)), None) |
There was a problem hiding this comment.
When an adopted repository is cloned beneath any directory literally named analysis (for example, /work/analysis/repo), editing a non-task path such as src/package_v1/config.json finds that external ancestor, treats package_v1 as a task, and blocks the valid edit for having an unknown top-level file. Resolve the checkout first and only search beneath <checkout>/analysis so unrelated ancestor names cannot activate the guard.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in da5faa0: the guard resolves the checkout first and looks for a task only below its own analysis/; covered by a test with the repo inside an unrelated analysis directory.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011aQpF4MA9v7dTVK1FqMMVJ
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011aQpF4MA9v7dTVK1FqMMVJ
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0d224f78e7
ℹ️ 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".
| return found | ||
|
|
||
|
|
||
| def after(tool_input: dict, path: Path) -> str | None: |
There was a problem hiding this comment.
When the python3 on PATH is Python 3.9, evaluating this str | None annotation (and the later Path | None) raises TypeError while importing the hook, before main() can run; this is a supported hook runtime and causes every Write/Edit invocation to skip the new layout enforcement. Adding postponed annotations here is also insufficient by itself because output_dirs() executes the Python-3.12-targeted paths.py, which contains the same union syntax, so keep all code executed by this standalone hook Python-3.9-compatible.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 2f0dc48: postponed annotations, and the hook now reads the output directories from paths.py with ast instead of running it. Checked by running every hook, and an end-to-end task in a rendered project, under Python 3.9.
…d not run Jobs table says completed, as Slurm and euler-jobs --summary do. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011aQpF4MA9v7dTVK1FqMMVJ
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2f0dc4876a
ℹ️ 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".
| if not path.is_relative_to(analysis): | ||
| return None | ||
| below = path.relative_to(analysis).parts[:-1] | ||
| return next((analysis.joinpath(*below[: i + 1]) for i, p in enumerate(below) if TASK_DIR.match(p)), None) |
There was a problem hiding this comment.
After the outermost-directory fix, a topic whose legitimate name ends in _vN is now mistaken for the task itself. For example, writing analysis/cohort_v1/demo_v1/scripts/01_step.py selects cohort_v1, then rejects demo_v1 as an invalid top-level entry; even a human notebook directly under analysis/cohort_v1/ is blocked. The documented <topic> component has no naming restriction, so task discovery needs to distinguish the actual task directory rather than reserving every version-looking ancestor.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in a06f447: walking down from analysis/, the task is the last _vN directory before the path enters a task's own entries, so cohort_v1/demo_v1/scripts/… belongs to demo_v1 and demo_v1/outputs/model_v1/… still to demo_v1; both tested. A file directly in a versioned topic can't be told from a task's own file; topics named <name>_vN collide with the task naming convention, so that case stays as is.
| "type": "command", | ||
| "command": "python3 \"${CLAUDE_PLUGIN_ROOT}/hooks/guard_task_layout.py\"", | ||
| "timeout": 10 |
There was a problem hiding this comment.
When a task is created or modified through Bash—such as cp, mkdir, sed, or an analysis script—the new layout guard never runs because it is registered only for Edit|Write|NotebookEdit. This leaves arbitrary top-level files, missing READMEs, and invalid headings undetected; the finishing command does not provide a backstop because src/analysis_workflow/cli.py::layout still checks only stray, orphaned, and oversized outputs and can report Clean. Add a repository-wide layout validation to the finish check or otherwise cover non-file-tool changes.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in a06f447, as a Stop hook rather than in check layout: the installed package doesn't ship the plugin's task template, so it couldn't check headings. When a turn ends, the same guard checks the task directories with uncommitted changes (git status), which catches files made with cp, mkdir or a script, and earlier tasks are never scanned. It keeps the agent working once (stop_hook_active); tested.
…en a turn ends Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011aQpF4MA9v7dTVK1FqMMVJ
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a06f447a85
ℹ️ 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".
| status = subprocess.run( | ||
| ["git", "status", "--porcelain=v1", "-z", "--untracked-files=all", "--", "analysis"], |
There was a problem hiding this comment.
When a shell command creates an off-layout file or malformed README and the agent commits it before ending the turn, this git status command returns no paths, so the Stop hook exits successfully. Fresh evidence in the current code is that the shell-change backstop is scoped exclusively to uncommitted status, despite the normal Finish flow committing and pushing before the turn ends; include task changes already committed on the branch or perform a full validation before allowing Stop.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 63fbac3: the check now diffs the working tree against the branch's merge base with the default branch, plus untracked files, so files committed in the same turn are caught; earlier tasks the branch doesn't touch are still not scanned. Tested by committing an off-layout file before Stop.
| skip = entry[0] in "RC" # a rename's or copy's original path follows | ||
| if "D" not in entry[:2]: | ||
| paths.append(top / entry[3:]) |
There was a problem hiding this comment.
When README.md is removed through Bash, its status is D, and this filter discards the only path that identifies the changed task; tasks therefore remains empty and the later "no README.md" check never runs. Fresh evidence after the Stop-hook fix is the explicit exclusion of every deleted path here, so retain deleted paths for task discovery while skipping only inappropriate per-file validation.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 63fbac3: deleted paths now count for finding the changed task, so a deleted README is reported; a task directory renamed away (gone, not broken) is skipped. Both tested.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011aQpF4MA9v7dTVK1FqMMVJ
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 63fbac3883
ℹ️ 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".
| """Paths under `analysis/` this branch changed, committed or not, deleted ones included, and untracked files.""" | ||
| base = next((b for ref in ("origin/HEAD", "origin/main", "main") if (b := git(top, "merge-base", "HEAD", ref))), "") | ||
| names = git(top, "diff", "--name-only", "--no-renames", "-z", base.strip() or "HEAD", "--", "analysis").split("\0") | ||
| names += git(top, "ls-files", "--others", "--exclude-standard", "-z", "--", "analysis").split("\0") |
There was a problem hiding this comment.
When a script or Bash command creates an off-layout artifact matched by the repository's ignore rules—for example, analysis/topic/demo_v1/model.zarr, since adoption explicitly adds *.zarr—this command omits it because --exclude-standard applies the standard Git exclusions. The preceding diff also cannot see an untracked path, so the Stop hook returns success and leaves the invalid task layout undetected. Fresh evidence after the earlier shell-change fix is that its untracked-file backstop explicitly excludes ignored files; scan ignored paths as well while allowing their contents only beneath the permitted output directories.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Not fixing: outputs come from task_paths and a hook blocks bare output paths, so an ignored artifact at a task's top is already prevented where it is written; ignored files never reach the PR; and scanning ignored paths would also walk a task-local .pixi environment. The cost is more than the case is worth.
| if TASK_DIR.match(part): | ||
| task = i |
There was a problem hiding this comment.
When a legitimate topic itself ends in _vN, writing a human notebook directly beneath it—such as analysis/cohort_v1/notes.ipynb—sets task to cohort_v1, after which problem() rejects the notebook as an invalid task entry. The skill permits notebooks flat under analysis/<topic>/ and imposes no topic-name restriction. Fresh evidence in the current revision is that the revised walk only passes over cohort_v1 when a later versioned directory replaces it, so the direct-notebook scenario from the earlier review remains broken.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Resolved by the convention instead of the hook, in 1eb2fb8: only task directories end in _vN, a topic never does. A marker heuristic (a README or scripts/ in the directory) only moves the ambiguity, since topics may have READMEs too.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011aQpF4MA9v7dTVK1FqMMVJ
Every task had to write a findings report next to its README. Across the recent finished tasks of one analysis repo, the human never referred to a report: figures and their legends were what got read. Fewer than half the tasks had a report. README and report repeated each other, and when they disagreed it was a stale file name, a status or a "still holds" claim, in whichever copy nobody touched after the change.
deliverablessetting (default "a figure"), which the session start lists. A task can change it, and the answer goes in the README. No report by default.paths.py._common.pymoves intoscripts/, where the template's own run commands already needed it. Scripts areNN_<step>.pyin run order.🤖 Generated with Claude Code
https://claude.ai/code/session_011aQpF4MA9v7dTVK1FqMMVJ