Skip to content

bug(unic-archon-dlc): reconcile marks a finding fixed when the next review omits it, and post resolves the thread #482

Closed

Description

@orioltf

Observed

Run 3, unic-dlc-pr-review iteration 2 on PR 5881, byte-identical commit 08e2523 to iteration 1. The second review did not mention six of the first iteration's thirteen findings (confidence 78, 74, 72, 72, 64, 61, all Standards). reconcile returned:

{"iteration":2,"fixed":6,"still_present":7,"regressed":0,"new":4,"prior_count":13,"verdicts":13,"unverified_fixed":0,"unmatched_priors":0}

post then set threads 65901, 65904, 65905, 65906, 65909 and 65910 to Azure DevOps status fixed. Nobody changed the code and nobody resolved anything by hand.

unverified_fixed stayed 0 because it reads the PR's diff against its base, and the six files were in that diff. The evidence a fixed needs is the change between the two iterations, and the Box records no head commit per iteration.

Why it matters

A duplicate thread is noise a reader discards. A false fixed is a defect a reader stops looking for. On a repository with no CI the pull request is the only record, so this turns the reviewer's variance into false state on the Consumer's PR.

Evidence

Decisions

Grilled with the maintainer on 2026-10-27, under map #498.

  1. A fixed needs the finding's file to change between the two iterations. The evidence is the change between the prior iteration's recorded head and this iteration's head. The PR's diff against its base is never evidence for a fixed. A change to the file is necessary. It is not proof that the fix is right, and that limit is accepted.
  2. An omitted prior finding whose file did not change gets a verdict of its own, unverified. It is neither fixed nor still_present. A finding whose real fix landed in another file also lands here. That false open costs the author one click, while a false fixed hides a defect.
  3. post replies once to an unverified thread and does not change its status. It does not reply again in a later iteration while the finding stays unverified.
  4. /pr-review reviews again on an unchanged head. It does not stop. A rerun is how a post that failed halfway gets finished.
  5. With no prior head to compare against, every omitted prior is unverified. This covers the first iteration after this ships, whose prior iteration an older Box posted, and a prior head the clone cannot reach after a rebase or a force push. The summary tells the developer why and what is asked of them.
  6. An omitted scoped finding is always unverified. A finding whose file is pull request, work item or repository has no file to check, so a person closes it.
  7. A thread that is resolved and whose finding is reported again is reopened, whoever resolved it. This includes an unverified thread a person resolved by hand.

Acceptance criteria

Every criterion names the diff it reads. "The change between the two heads" means git diff --name-only <prior head> <head>, where both heads are recorded iteration heads. "The PR's diff" means the diff against the PR's base, which prep computes today.

Recording the head

  • Every iteration records the commit it reviewed, in the PR, in a place the next iteration's prep reads back. The value comes from git rev-parse HEAD in the reviewed tree, not from a model's account of it.
  • prep emits the prior iteration's head, or states that there is none. "None" covers both a prior iteration that recorded no head and a recorded head the clone cannot reach after a fetch.

Classifying, in reconcile

  • reconcile writes the list of files in the change between the two heads to the session directory, as the output of the git command, before it classifies anything. Every fixed verdict can be checked against that file.
  • A prior finding with no match this run is fixed only when its file is a path and that path appears in the change between the two heads. Otherwise it is unverified. The PR's diff plays no part in this rule.
  • A prior finding with no match this run is unverified when prep reports no prior head. No prior finding is fixed in that iteration.
  • prep reads unverified back as a prior verdict, alongside open and fixed, from what the prior iteration posted. A prior unverified finding is then classified like an open one: still_present on a match, fixed by the rule above, unverified otherwise.
  • reconcile emits an unverified count. unverified_fixed is removed, because a fixed that rests on absence alone no longer exists. Every reader of unverified_fixed in unic-dlc-pr-review.yaml reads the new count instead: the review-gate message and post's final line. unmatched_priors stays, and stays the number that must be zero.
  • The "since iteration N−1" delta in the summary shows unverified beside fixed, still present, regressed and new.

Writing to the PR, in post

  • On an unverified thread post posts one reply and leaves the thread's status as it was. The reply says that the review did not report the finding this iteration, that its file did not change since the prior iteration (or that the finding is scoped, or that there was no prior head), and that the author should check it by hand and resolve it if it is gone. A thread that already carries that reply from an earlier iteration gets no second one.
  • post reads each thread's status before it acts. A thread that is resolved, and whose finding is still_present or regressed this iteration, is reopened with a reply saying the finding was reported again in iteration N. Where the tracker has no reopen, post replies on the thread.
  • When prep reports no prior head, the summary carries a section that tells the developer, in plain words, why no finding could be ruled fixed (no recorded head, or the history was rewritten) and what is asked of them: check each unverified thread by hand and resolve the ones that no longer apply. It also says that the check is automatic again from the next iteration unless the history is rewritten again.
  • fixed keeps today's behaviour in post: reply "✅ resolved in iteration N" and resolve where the tracker can.

The four cases that prove it, and where each runs

Each case names the diff it reads.

  • Identical head. Two iterations on one commit. The change between the two heads is empty. Every omitted prior is unverified, fixed is 0, every unverified thread stays unresolved and carries exactly one reply. A third iteration on the same commit adds no reply to those threads. This is run 3's case.
  • Rerun after a partial post. post is killed after it has written at least one reply and before it finishes. A rerun on the same head ends with one thread per finding and no reply repeated on any thread, whatever the verdict. A reply counts as repeated when the thread already carries a reply for the same finding hash and the same verdict. To tell what is already posted, post reads the PR's existing threads and replies by their hidden markers. It never reads the killed run's session directory. So every reply post writes carries a hidden marker with the finding's hash and the verdict it answers. That hash is the hash of the thread's own finding, the one the thread was opened with, never a hash the rerun computes fresh: the reviewer rewords, so a fresh hash would never count a reworded re-report as repeated. The rerun's post compares against the prior finding it matched in reconcile. The rerun counts as a new iteration (N+1), as today, because the killed run's summary already carries its iteration marker.
  • The finding's file changed. The change between the two heads includes the finding's file, and the review does not report the finding again. The finding is fixed and its thread is resolved.
  • Only another file changed. The change between the two heads is not empty and does not include the finding's file, and the review does not report the finding again. The finding is unverified, and its thread stays unresolved with one reply.

Where each case runs, per map #498 § Run 4's shape:

  • Identical head: run 4's iteration 2, on the same head as iteration 1.
  • Rerun after a partial post: run 4's rerun of iteration 2, on the same head.
  • The finding's file changed, and only another file changed: run 4's review after the single fix commit, checked against git diff --name-only between the recorded heads of the rerun and that review.

The rerun is numbered N+1 on the pull request, so the review the map calls iteration 3 carries a higher number there.

Documents

  • apps/claude-code/unic-archon-dlc/docs/boxes/unic-dlc-pr-review.md lists unverified wherever it lists the verdicts, and says what post does with it and with a resolved thread whose finding is back.
  • If the change adds a node to unic-dlc-pr-review.yaml, apps/claude-code/unic-archon-dlc/docs/architecture/20260925-unic-dlc-pr-review-pipeline.json is updated and both PNGs are exported again. The decisions above add no node, so this is expected to need nothing.
  • apps/claude-code/unic-archon-dlc/CHANGELOG.md carries an entry under ## [Unreleased].

What a run here does not prove

  • The pull request states that the change is unproven in the Consumer until a Consumer run reads it. A run in this repository does not prove it.

Not decided here, on purpose

  • The exact wording of the unverified reply, the reopen reply and the summary section. The criteria fix what each must say, not the text.
  • Where in the PR the head is recorded. The criteria require only that the next iteration's prep reads it back.

Activity

  1. added
    bugSomething isn't working
    needs-specsEnough info from reporter; ready to write a spec
    p1High priority
    on Sep 16, 2026
  2. orioltf commented on Oct 27, 2026

    @orioltf
    MemberAuthor

    Stale — do not follow. The criteria are approved and the fix is dispatched under /afk-loop 498; the opener for the fix is the newer comment that starts with **Opener for #482**.

    Opener — How to start this session (written 2026-10-27 by unic-sdlc-orchestrator-7, at dispatch)

    What this session produces

    Approved acceptance criteria in this ticket's body, then the label ready-for-agent. Nothing else. The fix runs later under /afk-loop 498. This ticket blocks #502, run 4.

    Start: the maintainer types /wayfinder 498 #482. This ticket's parent is #498. When the grilling starts, the maintainer types /grill-with-docs. Both carry disable-model-invocation, so the session cannot invoke them itself.

    Do not close this ticket. This overrides /wayfinder step 4. This stream grills a ticket into its own criteria and implements it under the same number. Do not invoke /implement, /tdd or /to-tickets. Keep needs-specs until the maintainer approves, then replace it with ready-for-agent.

    Read in this order

    1. This ticket's body. It was filed on 2026-09-17 from run 3, and its three "Not decided" points are the starting questions.
    2. apps/claude-code/unic-archon-dlc/.archon/workflows/unic-dlc-pr-review.yaml on origin/develop: prep (how prior_findings and prior_verdict are read), reconcile step 3 (the fixed, retired and unverified_fixed rules), review-gate, and post (what a fixed verdict does to a thread).
    3. docs/boxes/unic-dlc-pr-review.md, the part on re-review.

    The trap that costs an hour

    Assuming unverified_fixed already covers run 3's case. Since the ticket was filed, reconcile defines unverified_fixed as a fixed verdict where "THIS run's diff does not touch that file at all". But "this run's diff" is the PR's diff against its base. In run 3, iteration 2 ran on a byte-identical commit, and the files of the six omitted findings were in the PR's diff, so the rule counts zero. That is what run 3 recorded. The evidence a fixed needs is the change between the two iterations, and the Box records no head commit per iteration today: nothing in prep or reconcile names one. A criterion that tightens unverified_fixed without saying which diff it reads passes review and repeats run 3.

    What this ticket does not decide yet

    Each of these is a question for the maintainer. None is yours to settle with a plausible default.

    • What evidence makes a fixed. Either the prior finding's file changed between the prior iteration's head and this one, which needs that head recorded, or the prior finding is re-checked against the current tree. Or both.
    • What post does with a fixed it cannot verify. Reply without resolving, or resolve with an unverified marker. Today post replies "✅ resolved" and sets a resolved status wherever the tracker has one.
    • The default for an omitted prior. Whether a prior finding the new review does not mention becomes still_present, unverified_fixed or a verdict of its own.
    • A re-run on an unchanged head. Whether /pr-review on the same head commit as the prior iteration should re-review at all, or carry the prior verdicts forward.
    • The first iteration after this ships. A PR whose prior iteration was posted by an older Box has no recorded head. Decide what the first new iteration does then.

    Other traps

    The bar that proves it

    Run the four reads from docs/agents/dispatching-and-learning.md § Before you post one on the criteria as a set. Then check:

    • A second /pr-review on a byte-identical head, whose review omits a prior finding, leaves that finding's thread unresolved and does not call it fixed.
    • A prior finding whose file changed between the two iterations, and which the new review does not report, can still be fixed.
    • Every criterion names which diff it reads: the PR's diff against its base, or the change between two iterations.

    A false pass is a stricter unverified_fixed that still reads the PR's diff: it is counted, it stays zero on run 3's case, and post still resolves the thread.

    On landing

    1. Rewrite this ticket's body with the approved criteria, and post a comment saying what changed.
    2. Before you apply any label, report to unic-sdlc-orchestrator-7 with the link. The orchestrator reads the criteria as one set and replies.
    3. Then replace needs-specs with ready-for-agent, keep p1 unless the maintainer moves it, and leave it unassigned for /afk-loop 498.
    4. Change no file in the repository.

    Report back

    BEFORE YOU FINISH, AND WHENEVER YOU LEARN SOMETHING THAT WOULD HAVE CHANGED THIS PROMPT,
    report to unic-sdlc-orchestrator-7 with four things and nothing else:
      1. what you expected
      2. what happened
      3. the command or file that proved it
      4. whether it changes only this ticket, or every session in this repo
    Send it when you learn it, not only at the end — a session that dies unreported
    takes its learnings with it.
    Talk only to unic-sdlc-orchestrator-7. Do not message another orchestrator or its
    sessions; tell unic-sdlc-orchestrator-7, who relays it.
    

    Review and handover

    There is no pull request. The maintainer approves the criteria in the session. The orchestrator reads them as one set before the label goes on. Implementation runs under /afk-loop 498, which this seat records as unproven in the Consumer until a Consumer run reads it.

  3. self-assigned this
    on Oct 27, 2026
  4. orioltf commented on Oct 27, 2026

    @orioltf
    MemberAuthor

    Body rewritten after grilling with the maintainer, 2026-10-27.

    What changed:

    • The three "Not decided" points are replaced by seven decisions. The core one: a fixed needs the finding's file to change between the prior iteration's recorded head and this one. An omitted prior whose file did not change gets a new verdict, unverified. post replies once and does not resolve its thread.
    • Acceptance criteria added. Every criterion names the diff it reads, and the proof is three cases: identical head, finding's file changed, only another file changed. The map's bench may be cited for the identical-head case only.
    • unverified_fixed is to be removed. It read the PR's diff against its base, which is why it stayed 0 on run 3.
    • Two open questions belong to map #498: where the two non-identical cases are proven, and whether the bench stays as it is.
    • "the client's PR" now reads "the Consumer's PR".

    Labels stay as they are until unic-sdlc-orchestrator-7 has read the criteria as one set.

  5. orioltf commented on Oct 27, 2026

    @orioltf
    MemberAuthor

    Body amended after the orchestrator's set read, 2026-10-27. The cause is the map change on #498: run 4 no longer runs a byte-identical bench.

    • A fourth case, rerun after a partial post. A rerun on the same head ends with one thread per finding and no repeated reply. post tells what is already posted from the hidden markers on the PR's threads and replies, so every reply carries a marker with the finding's hash and its verdict. The maintainer decided that the rerun counts as a new iteration (N+1), as today.
    • The heading now reads "The four cases that prove it, and where each runs". Each case is mapped to its step in wayfinder: run 4 — are the chain's claims about its own work derived from the work? #498 § Run 4's shape. The two hard cases are checked against git diff --name-only between the recorded heads of the rerun and the review after the fix commit.
    • The bench mention is gone, and so is the "Open questions" section. The map decided both questions.
  6. removed their assignment
    on Oct 27, 2026
  7. added
    ready-for-agentFully specified, ready for an AFK agent
    and removed
    needs-specsEnough info from reporter; ready to write a spec
    on Oct 27, 2026
  8. orioltf commented on Oct 27, 2026

    @orioltf
    MemberAuthor

    Body amended, 2026-10-27. The partial-post case now says whose hash a reply's marker carries: the hash of the thread's own finding, the one the thread was opened with, never a hash the rerun computes fresh. Then ready-for-agent, p1, no assignee, and #556 is now blocked by this ticket.

  9. self-assigned this
    on Oct 27, 2026
  10. orioltf commented on Oct 27, 2026

    @orioltf
    MemberAuthor

    Opener for #482

    Written 2026-10-28 by the /afk-loop 498 orchestrator, at dispatch. It supersedes the grilling opener above, whose job ended when the criteria were approved.

    What this session produces

    One pull request to develop that changes apps/claude-code/unic-archon-dlc/.archon/workflows/unic-dlc-pr-review.yaml and its documents so that an omitted prior finding is fixed only when its file changed between two recorded iteration heads, and unverified otherwise. The criteria in this ticket's body are the spec. Do not invoke /tdd or /to-tickets: this repository's product here is prose, and the bar is a read, not a test (AGENTS.md § Acceptance criteria are prose).

    Read in this order

    1. This ticket's body, whole. The three comments after the grilling opener explain why the body changed. The body already holds their content.
    2. unic-dlc-pr-review.yaml on origin/develop: prep § 4 (how prior_findings and prior_verdict are read), prep's diff step (where HEAD and SESSION/changed-files.txt come from), reconcile steps 2 to 6 (the verdicts, retired, unverified_fixed, the summary frame, the fixed list and its markers, and the thread plan), the review-gate message, and post (the summary update, the per-line kinds, and the closing line).
    3. apps/claude-code/unic-archon-dlc/docs/boxes/unic-dlc-pr-review.md, the part on re-review.

    The trap that costs an hour

    A reader with no writer. prep can only read a prior head, a prior unverified verdict, or an earlier reply back from text that an earlier iteration posted. So each new thing prep reads needs a named place where reconcile or post writes it, in the same pull request:

    • The prior head: reconcile step 5 writes the summary, and post updates that summary in place. A head that prep looks for and nothing writes reads as "no prior head" on every iteration, so fixed is 0 forever and the identical-head case passes for the wrong reason.
    • A prior unverified verdict: today the summary lists fixed findings with their markers, and prep reads prior_verdict from that list. An unverified finding needs the same, or prep reads it back as open. That reads the right verdict by accident, but a scoped unverified finding has no thread, so the summary is the only place its hash survives.
    • The "already replied" check: post must find an earlier reply by its hidden marker, which carries the thread's own finding hash and the verdict. Read the body's partial-post case for whose hash that is.

    For each one, trace it once from writer to reader on paper before you edit.

    What this ticket does not decide

    The body leaves these open on purpose. Pick, write the choice and the reason in the PR body, and keep it consistent everywhere:

    • Where the head is recorded. A hidden marker in the summary, beside the iteration marker, is the cheapest place, because post already rewrites the summary in place on every run. The value must come from git rev-parse HEAD in the reviewed tree, written to a file in the session directory by the command, never retyped by a model.
    • The shape of the reply marker. It must carry the finding hash and the verdict. Decide whether it reuses finding=<hash> or is a marker of its own. If it reuses it, prep must not read a reply as a second prior finding, and post must still find a thread by its first comment.
    • The exact wording of the unverified reply, the reopen reply and the no-prior-head section. The body fixes what each says, not the text.

    Other traps

    • retired is unchanged, and must stay that way. A prior fixed with no match is still retired. A prior unverified with no match is fixed or unverified by the new rule, never retired. Write the rule for each prior_verdict value explicitly.
    • unverified_fixed has readers outside reconcile: its output_format, the review-gate message, and post's closing line. Grep the file for every one before you call it removed.
    • Do not add a script, module or test to make the four cases runnable here. AGENTS.md names that as a defect. The four cases run in the Consumer, in run 4.
    • post must read a thread's status before it acts, and a resolved thread whose finding is back gets reopened. Ask the tracker whether reply and status change are one call or two, as the file already says for fixed.
    • This repository is public. Name no client and no Consumer repository. Use synthetic examples only, and write "the Consumer".
    • Use absolute paths in every shell call, and never cd. The NDA hook refuses a gh command that changes directory.

    The bar that proves it

    Read the four cases from the body against the file at your head, and for each one follow it from prep through reconcile and threads.json to post, naming the line that produces each verdict and each reply. The PR body states the walk for each case in a few lines.

    A false pass: reconcile emits unverified, but post dedupes replies against a hash this run computed, or against the session directory. Then the identical-head case looks right on iteration 2 and adds a second reply on iteration 3.

    On landing

    1. Add the entry to apps/claude-code/unic-archon-dlc/CHANGELOG.md under ## [Unreleased], and commit it with the change.
    2. Bump with pnpm --filter unic-archon-dlc bump minor, because the plugin is 0.x and the Box's behaviour changes. Commit the result on its own as chore(unic-archon-dlc): release <version>.
    3. The PR body says the change is unproven in the Consumer until a Consumer run reads it, and holds Closes #482.
    4. #556 waits on this ticket and edits the same file. Do not touch it.

    Report back

    Report in your final answer, as your worker prompt's step 7 says, and stop after the handover. The orchestrator may continue you with fix rounds.

    Review and handover

    The orchestrator runs the review rounds, sends you the findings to fix, requests the Copilot review, and merges when the gate holds. You do not merge, and you do not answer review threads.

  11. added
    resolvedImplemented; ready for a PR
    and removed
    ready-for-agentFully specified, ready for an AFK agent
    on Oct 27, 2026
  12. added
    closedPR has been merged
    and removed
    resolvedImplemented; ready for a PR
    on Oct 27, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions