Skip to main content
On this page

P4 · PR task folder restructure — progress note (2026-05-21)

Status: SHIPPED as MVP (locally). File-by-file refactor + live-data move deferred to P7.

Commits

Text
95bbe98 v4/P4: pr_review_dir + pr_review_file helpers (MVP)

Test pass count

330 passed (+5 new in test_pr_review_subfolder.py).

Exit-criteria check (plan §8 P4)

Criterion Evidence Status
Helper pr_review_dir(task_dir) exists _common.py:pr_review_dir, mkdir-on-demand. ✓
Every script that reads/writes pr.json, findings.json, etc. goes through it Helpers in place; call sites not yet refactored. DEFERRED to P7
No PR-review-specific file lives at the top of the task dir Live task folders still have files at the top (e.g. ~/.agents-devkit/pr-reviews/ecomm-ssr_pr-5211/pr.json). DEFERRED to P7

Why MVP

The user has many active task folders under ~/.agents-devkit/pr-reviews/ with PR-specific files at the top (alongside code/, code-index/, docs/). Moving those files mid-session without P7’s preservation-contract (staging + rollback + verification) carries data-loss risk.

Per the plan §8 P7:

“Every existing ~/.agents-devkit/pr-reviews/<repo>_pr-<n>/ task folder is MOVED to ~/.agents-devkit/skill-pr-review/<repo>_pr-<n>/, with its code/, code-index/, scip/, docs/ subfolders untouched (only PR-specific files relocate into pr-review/).”

So P7’s job is exactly to (a) MOVE the task folder root from pr-reviews/ to skill-pr-review/ AND (b) mkdir pr-review/ and mv the well-known files in. Both halves at once, idempotent, with rollback.

P4 ships the helpers so P7 (and any new code) can use the v4 layout. The 7 scripts that read/write the well-known files still use the legacy paths today; the helper’s read-shim ensures they keep working.

What got skipped (and why)

  • Call-site refactor across run_review.py, fetch_pr.py, comment_resolver.py, triage.py, post_comments.py, report.py, validate_findings.py — ~50 sites. Each needs task_dir / "xxx.json" → pr_review_file(task_dir, "xxx.json") for reads and task_dir / "xxx.json" → pr_review_dir(task_dir) / "xxx.json" for writes. Mechanical but lots of surface area. P7’s data migration is the natural seam to refactor these together with the data move.
  • Live-data move — P7’s preservation contract.

What PR_REVIEW_FILES enumerates

The set lists 19 well-known file names the skill writes:

  • Inputs: pr.json, pr-comments.json, diff.patch, queue-context.json
  • Intermediate: precis.md, findings.json, validated-findings.json, initial-findings.json, findings-final.json, validation-report.json, triage.json, triage-state.json, posting-plan.json
  • Output: comment-actions.json, post-result.json, findings.md, report.md
  • State: state.json, review.log

P7’s migration script walks this set and mvs each file from the task-dir top into task_dir/pr-review/.

Decisions made this run

  • Scope-narrow P4 vs heavyweight P4 — narrow. Reason: matches the P3 staging philosophy. Trade-off: P7 carries the call-site refactor + data move together.
  • Helper API shape — pr_review_dir() returns the subfolder (mkdir on demand); pr_review_file() resolves with read-shim. Same pattern as task_dir_for() in P2.
  • PR_REVIEW_FILES as a frozen set — enables P7’s migration to walk a single source of truth.

Exact commands to inspect

Shell
git show 95bbe98 --statpython3 -m pytest skills/adk-cli/scripts/tests/test_pr_review_subfolder.py -v