Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 20 additions & 33 deletions .agents/skills/reviewing-before-pr/SKILL.md
Original file line number Diff line number Diff line change
@@ -1,49 +1,36 @@
---
name: reviewing-before-pr
description: >
Run one local Greptile review of a branch before its PR opens, with `hogli review`.
Opt-in: it needs a seat on PostHog's Greptile workspace, which most people do not have yet,
so use it only when someone asks for a local Greptile review by name. Trigger terms:
hogli review, greptile, local review, pre-PR review.
Review a branch with the harness's own code review skill before its PR opens, and record the pass
in the PR description. Use when someone asks for a local review, a self-review, or a review before
opening or pushing a PR. Trigger terms: local review, self-review, review before PR,
review my branch, pre-PR review.
---

# Reviewing before opening a PR

`hogli review` runs the same Greptile reviewer that comments on every PostHog PR, locally, against the committed branch diff.
A finding handled here is an ordinary pre-push edit; the same finding after the PR opens is a bot comment, a fix push, a stale-thread cleanup, and a CI re-run.
Run your harness's code review over the branch diff before `gh pr create`.
A finding handled here is an ordinary pre-push edit; the same finding after the PR opens costs a bot comment, a fix push, a stale-thread cleanup, and a CI re-run.

This is the weaker pass, not a replacement for review: it reads code your own session may have written, with no independent context.
The PR review bot still reviews every PR, and human reviewers still read it.

## When to run

- Only when someone asks for it. A Greptile seat is per-person and hand-granted today, so opening a PR does not call for this review by default — the PR bot still reviews every PR.
- When asked, run it **once**, on the committed branch, right before `gh pr create`.
- Do not loop it after every push, and do not re-run it after addressing findings — each run is a paid review, and the PR bot passes over the final state anyway. `hogli review` enforces this by re-printing HEAD's existing completed review instead of starting a new one.
- Skip it for changes trivial enough for the `skip-agent-review` label.
- When someone asks for it. Opening a PR does not call for this review by default.
- Run it **once**, on the branch as it stands, right before `gh pr create`.
- Do not loop it after every push, and do not re-run it after addressing findings.

## The flow

1. Finish the work and commit everything. Greptile reviews committed changes only; uncommitted edits are invisible to it.
2. Run `hogli review`.
3. Verify each finding's premise against the code before acting on it. Findings carry severity and confidence, and they can be false positives — rejecting one, with a reason, is a valid outcome.
4. Fix what you agree with and commit the fixes.
5. Skip the duplicate bot review. Run `hogli review --check` (on a stacked branch, `-b <base>` scopes it to this layer's commits): it exits 0 when any commit on the branch has a completed review — that is, the local loop ran, even if fix commits landed after it. On exit 0, open the PR with `--label no-greptile`. On a nonzero exit (no local review ever ran on this branch, or signed out), omit the label and let the bot review.
6. Either way, record the local review in the PR description's Agent context section: the review ID and each finding's disposition (fixed, or rejected with the reason). The findings are otherwise invisible to reviewers — they only appeared in the terminal.
7. Continue the normal PR-opening flow (`hogli ci:preflight`, `gh pr create`).

## Fallback without Greptile access

When `hogli review` cannot run (exit 78 with no way to sign in, or no CLI in a sandbox), do not block the PR on it.
Run your harness's own review instead (Claude Code: `/code-review`) over the branch diff, and triage its findings through the same flow: verify each premise, fix what holds, commit.
Treat it as the weaker pass — it reviews code your own session may have written, with no independent context — so verify premises strictly.
A fallback review never earns the `no-greptile` label: `hogli review --check` stays nonzero without a Greptile review, so the gate enforces this on its own and the bot reviews the PR as usual.
Say in the PR description's receipt that the local pass was the harness fallback, so reviewers know the independent review is still the bot's.
1. Finish the work. Commit first when the harness's review takes a branch or commit range rather than the working tree.
2. Run the harness's review over the branch diff. In Claude Code that is `/code-review`; on a stacked branch, scope it to this layer's commits against its base.
3. Verify each finding's premise against the code before acting on it. Findings can be false positives, and rejecting one with a reason is a valid outcome.
4. Fix what holds and commit the fixes.
5. Record the pass in the PR description's Agent context section: which review ran, and each finding's disposition (fixed, or rejected with the reason). The findings are otherwise invisible to reviewers, because they only appeared in the terminal.
6. Continue the normal PR-opening flow (`hogli ci:preflight`, `gh pr create`).

## Notes

- **Exit 78 means not signed in.** Ask the user to run `greptile login` and sign in with Google, using their @posthog.com account — that is what grants access to the PostHog Greptile org, so pick "Continue with Google" in the browser window rather than creating an email-and-password account. Headless environments set `GREPTILE_API_KEY` in `.env.local` instead (see `.env.local.example`). Never attempt the interactive login yourself; when sign-in is not available, use the fallback above.
- **Missing CLI.** Flox activation installs the version pinned in `.flox/env/on-activate.sh` into the machine-shared store `~/.config/posthog/tools/greptile/`, so re-entering the environment usually fixes it. Outside flox: `brew install greptileai/tap/greptile` or `npm install -g greptile`.
- **Held-back files.** Greptile holds back files that look like they contain secrets. Leave them held back; pass `--include <path>` only when certain the file is safe to send.
- **Focus.** `--instructions "<text>"` steers the reviewer, the same way an `@greptile` comment does on a PR.
- **`--force`** starts a fresh paid review even when HEAD already has one — only when the user asks for it.
- **The `no-greptile` label** works because `.greptile/config.json` lists it in `disabledLabels`. Apply it only through the `--check` gate above — never to silence a bot review you have not run locally. When in doubt, leave the label off; the default is that the bot reviews.
- Review behavior is configured in `.greptile/` (shared with the PR bot): `config.json` for settings, `files.json` for context files, with nested per-directory configs (`products/desktop/.greptile/` exists).
- The diff goes to Greptile's API for review. This repo is public, so that is fine for repo content — but it is one more reason not to `--include` held-back files.
- **The bot pass is separate.** A local review never substitutes for the PR review bot, so never label a PR to skip the bot on the strength of this pass.
- **Focus it when the diff has a risky part.** Most harness reviews accept instructions or a path target; pointing the review at the part you are least sure about beats a broad sweep.
5 changes: 0 additions & 5 deletions .env.local.example
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,6 @@
# AZURE_INFERENCE_ENDPOINT=op://General/yw6efvcz5ooxx5fgll3kyang5a/username
# AZURE_INFERENCE_CREDENTIAL=op://General/yw6efvcz5ooxx5fgll3kyang5a/credential

# ── Local code review (PostHog employees) ─────────────────────────────
# hogli review normally authenticates via `greptile login` (Google SSO).
# Set the key only for headless environments.
# GREPTILE_API_KEY=

# ── Workflows / messaging (PostHog employees) ─────────────────────────
# Server-side rendering of email template designs (Unlayer export API).
# Without it, design-only template saves over the API return a 400.
Expand Down
57 changes: 0 additions & 57 deletions .flox/env/on-activate.sh
Original file line number Diff line number Diff line change
Expand Up @@ -365,52 +365,6 @@ if [[ "$_PHROCS_SKIP" -eq 0 ]]; then
_BG_PHROCS_START=$(date +%s)
fi

# Greptile CLI: machine-global, version-addressed store. Greptile is not in
# the flox catalog (proprietary npm package), and `hogli review` needs it.
# Not a root devDependency on purpose: that would pull a review-only CLI into
# every pnpm install, including CI and sandboxes that never review. One npm
# install per machine per pinned version serves every checkout and survives
# .flox/cache wipes; each activation only ensures the version and symlinks
# the venv bin (Step 2b), so worktrees on different branches resolve their
# own pin. A failed install must not break activation; the CLI is only needed
# at PR-open time and `hogli review` prints install guidance when absent.
_GREPTILE_VERSION="3.4.1"
_GREPTILE_STORE="$HOME/.config/posthog/tools/greptile/$_GREPTILE_VERSION"
_GREPTILE_BIN="$_GREPTILE_STORE/node_modules/.bin/greptile"
_GREPTILE_STAMP="$_GREPTILE_STORE/.complete"

_install_greptile() {
# Explicit `|| return`/`|| exit`: callers suppress errexit, so a failed
# install would otherwise fall through and stamp the broken state.
mkdir -p "$_GREPTILE_STORE" || return 1
(
# The store is shared across checkouts, so serialize concurrent
# activations (fresh worktrees) installing the same version.
flock 9 || exit 1
if [[ ! -x "$_GREPTILE_BIN" || ! -f "$_GREPTILE_STAMP" ]]; then
if [[ "$_DEV_SANDBOX_INSTALLS" -eq 1 ]]; then
# printf %q: dev-sandbox re-parses its command string, so the path
# must survive a $HOME with spaces or quotes.
"$FLOX_ENV_PROJECT/bin/dev-sandbox" "npm install --prefix $(printf '%q' "$_GREPTILE_STORE") --no-fund --no-audit greptile@$_GREPTILE_VERSION" || exit 1
else
npm install --prefix "$_GREPTILE_STORE" --no-fund --no-audit "greptile@$_GREPTILE_VERSION" || exit 1
fi
[[ -x "$_GREPTILE_BIN" ]] || exit 1
touch "$_GREPTILE_STAMP" || exit 1
fi
) 9>"$_GREPTILE_STORE/.install.lock"
}

_GREPTILE_SKIP=0
[[ -x "$_GREPTILE_BIN" && -f "$_GREPTILE_STAMP" ]] && _GREPTILE_SKIP=1
if [[ "$_GREPTILE_SKIP" -eq 0 ]]; then
_BG_GREPTILE_LOG=$(mktemp)
_ACTIVATION_TMPFILES+=("$_BG_GREPTILE_LOG")
( _install_greptile ) >"$_BG_GREPTILE_LOG" 2>&1 &
_BG_GREPTILE_PID=$!
_BG_GREPTILE_START=$(date +%s)
fi

# ── Step 1: Python packages (must run before hogli — it needs Click) ─
if [[ "$_UV_SKIP" -eq 1 ]]; then
done_step "Python packages (cached)"
Expand Down Expand Up @@ -463,17 +417,6 @@ else
wait_bg_step "Node packages" "$_BG_PNPM_PID" "$_BG_PNPM_START" "$_BG_PNPM_LOG"
fi

# ── Step 2b: Greptile CLI (reap; launched above with the other jobs) ──
if [[ "$_GREPTILE_SKIP" -eq 1 ]]; then
done_step "Greptile CLI (cached)"
else
wait_bg_step "Greptile CLI" "$_BG_GREPTILE_PID" "$_BG_GREPTILE_START" "$_BG_GREPTILE_LOG" \
|| warn_step "Greptile CLI install failed ${C_DIM}(hogli review prints manual install steps)${C_RESET}"
fi
if [[ -x "$_GREPTILE_BIN" && -d "$UV_PROJECT_ENVIRONMENT/bin" ]]; then
ln -sf "$_GREPTILE_BIN" "$UV_PROJECT_ENVIRONMENT/bin/greptile"
fi

# ── Step 3: /etc/hosts ──────────────────────────────────────────────
POSTHOG_HOSTS="127.0.0.1 db redis7 kafka clickhouse clickhouse-coordinator objectstorage seaweedfs temporal # posthog"
if grep -qF "$POSTHOG_HOSTS" /etc/hosts; then
Expand Down
4 changes: 0 additions & 4 deletions hogli.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -1042,10 +1042,6 @@ tools:
ci:preflight:
click: hogli_commands.ci_preflight:ci_preflight
description: Catch deterministic CI failures reachable from your diff before you push (advisory; --fix, --strict)
review:
click: hogli_commands.review:review
description: One local Greptile review of the branch before opening the PR (skips when HEAD is already reviewed; --force, --instructions, --check)
needs_secrets: true
desktop:
desktop:dev:
cmd: pnpm --dir products/desktop dev
Expand Down
158 changes: 0 additions & 158 deletions tools/hogli-commands/hogli_commands/review.py

This file was deleted.

Loading
Loading