phillip-sync -> self-updating review rubric

You keep the /phillip rubric current by mining THIS repo's recent, resolved PR-review comments and folding the recurring lessons back into the sibling phillip/RUBRIC.md and phillip/RUBRIC.private.md. You run as a pre-step inside /phillip.

Which file a row goes to

Git tracks RUBRIC.md and the repository is public. Git never tracks RUBRIC.private.md. Route every row before you write it:

Candidates always go private. Their ADDED column carries a source PR by contract, which is private provenance.

Publication test

A row passes only when its Repo, Category, Trigger, Rule, and Added text contain none of:

• the repo slug, the owning organization, or the repository name
• a file path, a directory name, or a filename specific to this repo
• a package, module, service, collection, queue, or environment name from this repo
• a product, feature, or screen name
• a PR number, a ticket id, or an internal URL
• a customer name, an account name, or a person's name

When the row fails on an example only, rewrite the example generically and re-test it. Keep the rule, lose the internal illustration. When the internal fact IS the rule, retag REPO to the repo slug and send it private.

Unsure means private. A wrong private call costs one row in the public core. A wrong public call publishes private detail permanently.

Be terse. Use ->, not em dashes. Never print tokens or keys.

0. Hard rules (read first)

• NON-BLOCKING. If anything is missing or fails -> print ONE warning line and STOP with success so /phillip proceeds on the existing rubric. Never error out, never block.
• COOLDOWN. If this repo synced < 24h ago -> print one "rubric fresh" line and STOP.
• IDEMPOTENT. Re-running adds nothing already present. Anchored blocks are the only thing you ever rewrite, and you dedupe before writing.
• SECURITY. gh owns its auth token. Never echo it, never print env that holds it.
• ONE REPO AT A TIME. A slug-consistency guard in steps 4 and 7 detects a concurrent run in a different repo and SKIPS, so the shared /tmp/phillip_sync_* files are never corrupted.

Locate the directories containing the loaded phillip and phillip-sync skills. Call them PHILLIP_DIR and PHILLIP_SYNC_DIR. Paths used throughout:

• Public rubric: $PHILLIP_DIR/RUBRIC.md (tracked, world-readable)
• Private rubric: $PHILLIP_DIR/RUBRIC.private.md (gitignored)
• State file: $PHILLIP_DIR/.sync-state.json
• Scripts: $PHILLIP_SYNC_DIR/scripts/plan.py and $PHILLIP_SYNC_DIR/scripts/cursor.py

HARD RULE, applies to every code block below: each Bash call is a fresh shell, so NO shell variable survives between blocks. All cross-block state goes through two files, /tmp/phillip_sync_slug.txt (the repo slug, written in step 2) and /tmp/phillip_sync_plan.json (slug + cooldown + since window, written in step 3). Re-read them in every later block. Never rely on $SLUG or $SINCE carrying over.

1. Guard: is sync even possible? (non-blocking)

Run this. Any failure -> print the warning it emits and STOP (success).

# gh present?
command -v gh >/dev/null 2>&1 || { echo "phillip-sync: gh not installed -> skipping rubric sync (using existing rubric)."; exit 0; }
# gh authenticated? (does not print the token)
gh auth status >/dev/null 2>&1 || { echo "phillip-sync: gh not authenticated -> skipping rubric sync (run 'gh auth login')."; exit 0; }
# inside a git repo with a remote?
git rev-parse --is-inside-work-tree >/dev/null 2>&1 || { echo "phillip-sync: not a git repo -> skipping rubric sync."; exit 0; }
echo "phillip-sync: guards passed"

If the rubric file is missing, warn and stop:

test -f "$PHILLIP_DIR/RUBRIC.md" || { echo "phillip-sync: RUBRIC.md not found -> skipping."; exit 0; }
test -f "$PHILLIP_DIR/RUBRIC.private.md" || echo "phillip-sync: RUBRIC.private.md not found. It is untracked, so a fresh host lacks it. Public rows still sync; private rows are reported, not written."

2. Detect the current repo (any project, not just Atllas)

SLUG=$(gh repo view --json owner,name -q '.owner.login + "/" + .name' 2>/dev/null)
if [ -z "$SLUG" ]; then
  # Fallback: parse the origin remote URL (handles git@ and https forms, strips .git)
  URL=$(git remote get-url origin 2>/dev/null)
  SLUG=$(printf '%s\n' "$URL" | sed -E 's#^git@[^:]+:##; s#^https?://[^/]+/##; s#\.git$##')
fi
[ -z "$SLUG" ] && { echo "phillip-sync: could not resolve owner/repo -> skipping."; exit 0; }
printf '%s' "$SLUG" > /tmp/phillip_sync_slug.txt   # persist: later Bash calls are fresh shells
echo "repo: $SLUG"

3. Cooldown + SINCE from the per-repo cursor (30-day cap)

The state file is shaped:

{ "<owner/repo>": { "lastSync": "<ISO8601>" } }

plan.py computes both in one pass. SINCE = max(cursor.lastSync, now-30d). Cold start (no cursor) -> now-30d. It writes the slug into the plan file too, so step 7 can read it back.

python3 "$PHILLIP_SYNC_DIR/scripts/plan.py" "$(cat /tmp/phillip_sync_slug.txt 2>/dev/null)" \
  || { echo "phillip-sync: state read failed -> skipping."; exit 0; }

If cooldown -> print and STOP:

COOLDOWN=$(python3 -c "import json;print(json.load(open('/tmp/phillip_sync_plan.json')).get('cooldown'))" 2>/dev/null)
LASTH=$(python3 -c "import json;print(json.load(open('/tmp/phillip_sync_plan.json')).get('lastHuman'))" 2>/dev/null)
if [ "$COOLDOWN" = "True" ]; then echo "phillip-sync: rubric fresh (synced $LASTH ago) -> skipping."; exit 0; fi

4. Fetch recent PR reviews (one capped GraphQL page)

One page: the <= 40 most-recently-updated MERGED PRs in the window (first:40, sort:updated-desc). Merged-only signals acted-on.

The filter is updated:>=, not merged:>=. A PR merged months ago but updated yesterday re-enters the window carrying ALL its old threads, so do NOT treat every returned thread as new since the cursor. The step 5 text dedupe absorbs the repeats.

SLUG=$(cat /tmp/phillip_sync_slug.txt 2>/dev/null)
SINCE=$(python3 -c "import json;print(json.load(open('/tmp/phillip_sync_plan.json')).get('since',''))" 2>/dev/null)
{ [ -n "$SLUG" ] && [ -n "$SINCE" ]; } || { echo "phillip-sync: missing repo/window -> skipping."; exit 0; }
# Concurrency guard: if a sync in ANOTHER repo clobbered the temp slug, it won't match this
# repo -> skip rather than mine the wrong one (see "ONE REPO AT A TIME" in the hard rules).
CUR=$(gh repo view --json owner,name -q '.owner.login + "/" + .name' 2>/dev/null)
[ -n "$CUR" ] && [ "$CUR" != "$SLUG" ] && { echo "phillip-sync: temp state is for $SLUG but this repo is $CUR (concurrent run?) -> skipping."; exit 0; }
SINCE_DATE=${SINCE%%T*}   # date granularity (YYYY-MM-DD); may re-scan the last-sync day, but the text dedupe drops repeats
echo "phillip-sync: syncing $SLUG since $SINCE_DATE"

# Single page (first:40), newest-updated first, merged PRs only -> one JSON object.
gh api graphql \
  -f query='query($q:String!){ search(query:$q,type:ISSUE,first:40){ nodes{ ... on PullRequest { number title author{login} updatedAt mergedAt reviewThreads(first:50){nodes{ isResolved comments(first:20){nodes{ author{login} body path line createdAt }}}} reviews(first:50){nodes{ author{login} body state submittedAt }} }}}}' \
  -F q="repo:${SLUG} is:pr is:merged sort:updated-desc updated:>=${SINCE_DATE}" \
  > /tmp/phillip_sync_prs.json 2>/tmp/phillip_sync_err.txt \
  || { echo "phillip-sync: GitHub fetch failed ($(head -1 /tmp/phillip_sync_err.txt 2>/dev/null)) -> using existing rubric."; exit 0; }

# Extract PR nodes; degrade to empty on any error.
python3 - <<'PY' || { echo "phillip-sync: parse failed -> using existing rubric."; exit 0; }
import json
try: d=json.load(open("/tmp/phillip_sync_prs.json"))
except Exception: d={}
nodes=((d.get("data") or {}).get("search") or {}).get("nodes") or []
json.dump(nodes, open("/tmp/phillip_sync_capped.json","w"), indent=2)
print(f"phillip-sync: {len(nodes)} merged PR(s) in window")
PY

Now READ /tmp/phillip_sync_capped.json with the Read tool. If the file is empty / [], go to step 7 (update the cursor so the cooldown still applies), then print the step 8 summary line, then return success.

5. Distill (your own reasoning over the JSON)

Read the JSON. For EACH review thread across ALL PRs, judge it the way Phillip does:

RESOLUTION = the quality signal. Keep a comment's lesson ONLY if the thread was accepted and acted on:

• KEEP when isResolved: true AND the evidence says the author acted on it -> a later comment in the thread agrees / says "fixed" / "good catch", or a review with state: CHANGES_REQUESTED was later followed by state: APPROVED, or the PR shipped after the comment with the change applied.
• EXCLUDE threads that were dismissed, declined, "won't fix", "by design", "out of scope", or explicitly accepted as a tradeoff. An unresolved thread is weak evidence -> exclude unless the same lesson recurs elsewhere from resolved threads.

Learn from all reviewers present in the fetched threads, not just one pair.

The per-PR fetch is capped at 50 threads / 20 comments / 50 reviews, so a very busy PR is partially sampled.

Weight by RECURRENCE first: one senior comment is a Candidate, the same issue raised twice is rubric. You can add weight for the repo owner or a clearly senior reviewer.

From the kept comments, keep only patterns that are ALL of:

• (a) RECURRING -> the same class of issue appears in >= 2 distinct threads/PRs.
• (b) GENERALIZABLE -> a class of bug (e.g. "fetch not checking response.ok"), not a one-file detail ("rename foo in bar.ts:14").
• (c) NOVEL -> not already covered by ANY row in $PHILLIP_DIR/RUBRIC.md or $PHILLIP_DIR/RUBRIC.private.md. Read both files and compare meaning, not exact words. A row in RUBRIC.md counts as covered even though this skill never writes there. In RUBRIC.private.md, compare against ALL THREE anchored blocks: auto, candidates, AND auto-donotflag. A pattern already sitting in candidates is NOT novel, so it can never be re-mined into auto a month later. A pattern matching a donotflag row is the inverse of a finding -> drop it.
• (d) ACTED-ON -> per the resolution rule above.

Phrase each survivor as ONE table row in the rubric's column order:

| <repo> | <category> | <trigger -> failure> | <rule> | <YYYY-MM-DD> |

• REPO: the slug from /tmp/phillip_sync_slug.txt when the pattern names identifiers, products, or services specific to this repo. any when the lesson holds in any codebase. Never leave it blank, because readers skip rows tagged to a repo other than their own. This column drives the routing table at the top of this skill, so set it before you route.
• CATEGORY: one value from the closed set at the top of RUBRIC.md (Security, Races, Silent failures, Correctness, Performance, Data loss, Comments, Encoding, Docs, Tests, UI, Permissions, Firestore).
• TRIGGER -> FAILURE: the pattern plus its concrete user-visible consequence. One idea.
• RULE: what to do instead. One idea. Split a survivor carrying more than one pattern into one row per pattern rather than packing them into a single cell.
• Escape every literal | inside a cell as \|, including inside backticks, or the row breaks the table.
• Candidate rows put PR #<n>, <YYYY-MM-DD> in the Added column instead of a bare date.

Bucket your survivors:

• All four of (a) to (d) -> the auto block.
• Any one of the four uncertain -> the candidates block.
• A DECLINED comment class (the thread resolved with the reviewer being told the comment was wrong, unreachable, or unwanted) -> the auto-donotflag block, using its own column shape: | <repo> | <category> | <pattern> | <why it is not a finding> | <YYYY-MM-DD> |.

Build, in memory, the lists of fully-formatted rows. Dedupe each new row against the current contents of all three anchored blocks before writing.

If after dedupe ALL lists are empty -> print "phillip-sync: no new patterns" and go to step 7 (still update the cursor).

6. Write into the anchored blocks (idempotent, provenance-tagged)

Both files carry the same anchor names. Each file owns the blocks below:

Write each row into the block that the routing table at the top of this skill selected. Re-check the publication test on every row bound for RUBRIC.md immediately before the Edit. Never write a candidate row to RUBRIC.md, which has no candidates block.

If a needed anchor pair is missing (older rubric), do NOT guess a spot and do NOT insert the block yourself. ALWAYS skip the write for that file. Print: "phillip-sync: anchors missing in <file> -> skipping write. Fix: add the missing <!-- phillip-sync:... START/END --> marker lines to $PHILLIP_DIR/<file>." Then go to step 7.

When RUBRIC.private.md is absent, print the private rows under "phillip-sync: private rows not written" and continue. Write the public rows normally. Never redirect a private row to RUBRIC.md because the private file is missing.

For each NEW row, APPEND it just before its block's END marker, using the Edit tool anchored on that END marker so insertion is deterministic. Get today's date for the Added column with date +%F and tag every new row with it.

To add one auto row, Edit the END marker like:

old_string:

<!-- phillip-sync:auto END -->

new_string:

| any | Races | <trigger -> failure> | <rule> | <today> |
<!-- phillip-sync:auto END -->

Do not leave a blank line between the last row and the END marker. The rows must stay contiguous with the table header or the table breaks. Repeat per row, or batch several rows above the marker in one Edit.

6b. Retirement (the blocks are capped, not monotonic)

Run this AFTER inserting this run's rows.

• RECONFIRM instead of duplicating. When a survivor matches an existing row, do not insert. Edit that row's Added column to today's date. That is what "re-observed" means below.
• AUTO block cap: 40 rows. If inserting exceeds 40, move the rows with the oldest Added date back into the candidates block, oldest first, until 40 remain. Moving preserves the row verbatim. Only its block changes.
• CANDIDATES block cap: 30 rows. A candidate row whose Added date is more than 90 days old AND was not re-observed this run is DELETED. If the block still exceeds 30 rows after that, delete the oldest rows until 30 remain.
• DO-NOT-FLAG block: no cap and no retirement. It is small and each row prevents a recurring false positive.
• Report every retirement in the step 8 summary (e.g. "-2 auto -> candidates, -1 candidate expired") so a human can see what left.

7. Update the per-repo cursor

Persist lastSync = now for this repo. This arms the 24h cooldown and shrinks the next window. Do it even when nothing was written.

CUR=$(gh repo view --json owner,name -q '.owner.login + "/" + .name' 2>/dev/null)
python3 "$PHILLIP_SYNC_DIR/scripts/cursor.py" "$CUR" \
  || { echo "phillip-sync: cursor update skipped (non-fatal)."; exit 0; }

8. One-line summary

Print exactly one closing line. Count the public and the private rows separately, so the reader sees what landed in the tracked file: phillip-sync: +2 public, +1 private, +1 candidate from 7 PRs (Atllas-Inc/codebase). Cursor armed (24h cooldown). or, on any guard/cooldown/empty path, the single line that branch already printed. Then return success so /phillip continues into its review loop.

Cleanup

Removing these temp files is optional:

rm -f /tmp/phillip_sync_prs.json /tmp/phillip_sync_capped.json /tmp/phillip_sync_plan.json /tmp/phillip_sync_slug.txt /tmp/phillip_sync_err.txt 2>/dev/null; true