Skip to content

TaskCompleted Codex-review hook re-reviews the whole accumulated diff per task (repeats findings + diff-local false positives) #2

Description

@LukeLIN-web

TaskCompleted Codex-review hook re-reviews the whole accumulated diff per task, repeating findings and emitting diff-local false positives

What happens

When executing a /goal plan with the task list (TaskCreate / TaskUpdate), every time a task is marked completed a Codex review runs as a TaskCompleted hook and can block completion with CHANGES_REQUESTED. Observed across one plan execution (10 tasks, single shared working tree on main):

  • task1 → 3 findings.
  • task2 → 2 different findings — genuinely useful, caught a real design flaw (per-item backward vs. the intended allreduce-first DDP sync). This is the hook working well.
  • task4 → 2 findings that were verbatim repeats of task1's findings about the same lines (already discussed/resolved on task1).

Root cause (apparent)

The hook reviews the entire accumulated working diff on each task completion, not the incremental delta introduced by that task. Consequences:

  1. Repeated findings. Because tasks in a plan often touch the same files sequentially and share one working tree, a finding about code written in task1 is re-surfaced on task2, task4, … every subsequent completion. The reviewer does not appear to track which findings were already raised/addressed/refuted.
  2. Diff-local false positives. The reviewer judges the hunk in isolation without surrounding file context. Example: it flagged open(os.path.join(args.out, "csv_probe_ids.json"), "w") as "writes before the output dir is created" — but os.makedirs(args.out, exist_ok=True) runs unconditionally ~20 lines above at the top of the same function. The reviewer can't see it, so the same false positive recurs.

Why it's a problem

  • Each repeat costs a full Codex round-trip (latency + tokens) to re-litigate settled points.
  • It pressures the agent to make redundant defensive edits (e.g. a second idempotent makedirs) just to silence a diff-local false positive, adding noise to the code.
  • Genuinely new findings (like task2's) get diluted by the repeats, reducing signal.

Suggested fixes

  • Scope the review to the task's incremental diff (e.g. diff vs. the tree state when the task was created/claimed), not the cumulative working tree.
  • Persist a per-plan ledger of already-raised findings (by file+line+claim hash) and suppress / mark-as-prior on repeat, or pass prior findings + their resolutions into the reviewer's context.
  • Give the reviewer enclosing-function (or full-file) context, not just the diff hunk, to cut diff-local false positives.
  • Optionally let the agent record a refutation for a finding (with evidence) that carries forward, so a verified non-issue isn't re-raised.

Environment

  • Workflow: /goal <plan.md> execution with TaskCreate/TaskUpdate + Codex TaskCompleted review hook.
  • Single working tree, all tasks on main (no per-task worktree isolation).

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions