docs: log the ollama cold-start park and the judgment-per-PR gap

[itsHabib]

Session log for the #259 read-window fix, appended per the per-session convention already in friction-log.md. Docs-only.

Two entries, both recurrences rather than new discoveries — which is why they're worth recording.

1. Ollama cold-start timeout parks a run even with the daemon supervised

This is the existing #214 entry one failure mode further in. That session's remedy was to supervise the daemon (brew services). The daemon was up this time, with qwen2.5:7b present — and review-consolidation still escalated, this time with context deadline exceeded (Client.Timeout exceeded while awaiting headers) rather than connection refused.

A cold load of a 4.7 GB model outruns gate's HTTP client timeout, so the first run after the model is evicted from memory parks regardless of daemon health. Supervision doesn't close this. The deeper #214 remedy — have review-consolidation report local_model_unavailable instead of dressing an infra failure as a judgment question — remains the right fix and is still unimplemented. A failed extraction is not a finding against the code, and the operator's remedy differs completely.

2. Every PR burns an irreversible judgment to reach merge

The panel signals by PR comment and never posts a formal GitHub review decision, so readiness (no review decision reported by GitHub) and review-panel-completeness (completed=0 expected=1 missing=[claude]) escalate structurally on every PR — not because of anything PR-specific. The same pair parked #248, whose judgment says so explicitly.

The consequence: the only terminating path for any PR here runs through gate judge, a one-shot irreversible artifact, even when every substantive rung passes. gate next currently lists 16 runs awaiting judgment, which makes genuine escalations hard to pick out of load-bearing bookkeeping.

Suggested smallest fix is in the entry: teach review-panel-v1 to count a completed bot comment from a panel member declared in .ship.json as a completed review, so a clean PR can pass on evidence instead of consuming a judgment.

Checked

Not stale against main — #253 (a run that decides nothing records why, and still burns no cycle) landed just ahead of this and concerns cycle counting and aborted-run records, not panel completeness or the local-model rung. gofmt/go vet clean; no code touched.

🤖 Generated with Claude Code

[itsHabib]

@claude please review. Docs-only — a session entry appended to friction-log.md per the per-session convention already in that file.

The thing worth checking is whether either entry is stale or overstated, since both are recurrences and I'd rather delete a claim than log a wrong one:

1. The ollama entry asserts this is distinct from the existing docs: name friction-log.md in the Map and split it from FOLLOWUPS #214 entry — that one was connection refused with the daemon down, this one is context deadline exceeded with the daemon supervised and qwen2.5:7b present, which I attribute to cold-model-load latency exceeding gate's client timeout. If that attribution is wrong, or if the timeout is configurable somewhere I missed, the entry should say so instead.

2. The judgment-per-PR entry claims readiness and review-panel-completeness escalate structurally on every PR because the panel comments rather than posting a formal GitHub review decision. I grounded that in feat(contracts/org): record spine, contract law, and the ownership fold #248's judgment text and the current 16-deep gate next queue. If some PRs do pass those rungs cleanly — meaning the cause is narrower than I claim — that entry needs rewriting.

I checked this is not stale against #253, which landed just ahead of it: that change concerns cycle counting and aborted-run records, not panel completeness or the local-model rung.

[claude]

Claude finished @itsHabib's task in 4m 42s —— View job

---

Review: friction-log.md — #259 session entries

• Read the new friction-log entries
• Check gate's HTTP client timeout configuration (is it hard-coded or configurable?)
• Check readiness and review-panel-completeness verifier logic for structural escalation claim
• Verify cross-references and root-cause attributions
• Post review findings

---

Entry 1 — Ollama cold-start timeout

Verdict: accurate. No changes needed.

The attribution holds. local/local.go:28 has a hard-coded var httpClient = &http.Client{Timeout: 3 * time.Minute} — a package-level var with no CLI flag, env var, or caller-injectable path. A cold load of qwen2.5:7b (4.7 GB) returning the first header byte in under 3 minutes isn't guaranteed, which is exactly what context deadline exceeded (Client.Timeout exceeded while awaiting headers) describes.

One minor precision nit: the entry says "gate's HTTP client timeout" — this is the local package's client, not something gate independently sets. Both wordings are accurate enough for a friction log; not worth rewriting.

The #214 distinction holds: connection refused means the daemon is not running; context deadline exceeded while awaiting headers with the daemon up is a cold-model-load timeout. The #214 remedy (surface local_model_unavailable instead of dressing an infra failure as a judgment question) is confirmed unimplemented in the current code.

---

Entry 2 — Every PR burns an irreversible judgment

Verdict: the "structural" observation holds, but the attribution and the suggested fix are both wrong. The entry should be rewritten before merging.

What the code actually shows:

PR #235 (da0b170) shipped panelStandIn — a complete exact-head review panel now satisfies readiness directly without escalating. readiness.go:294-295:

decisionAbsent := pv.State != "MERGED" && pv.ReviewDecision == ""
if decisionAbsent && !stand.satisfied {
    escalations = append(escalations, standInEscalation(stand))
}

If stand.satisfied is true (panel complete, nobody asking for changes), no escalation fires. So the entry's framing — "the panel signals by PR comment and never posts a formal GitHub review decision" as the root cause — describes the pre-#235 state. The readiness mechanic is no longer the bottleneck.

The actual bottleneck is in claude.yml:

The workflow has an "Attest the reviewed head for gate's review panel" step (lines 121–171) that posts the `` sentinel that workflowAttestation in `evidence/panel.go` looks for. But it only fires when the trigger comment matches a narrow regex (line 156):

'^[[:space:]]*@claude[[:space:]]+(please[[:space:]]+)?review([[:space:]]+(this([[:space:]]+pr)?|the[[:space:]]+pr))?[[:space:]]*[.!]?[[:space:]]*$'

This requires the ENTIRE comment body to be @claude [please] review [this [pr]] with nothing else. A trigger like @claude please review. Context... — which is every real-world review request in this repo — doesn't match. The workflow itself acknowledges this: "a missed attestation leaves the panel incomplete and gate parks for judgment, which is merely today's behaviour."

So the chain is:

1. Real review requests always include extra text → attestation regex doesn't match → attestation step skips
2. Claude posts an issue comment, not a formal GitHub review → latestExactHeadReview can't see it
3. Panel stays completed=0 expected=1 missing=[claude] → PanelCompleteness escalates
4. Incomplete panel → panelStandIn returns satisfied: false → readiness escalates
5. Both escalations land on every PR → gate parks → judgment is the only exit

The "structural" claim is accurate for how the repo actually uses review requests — but the cause is the attestation trigger precision, not the readiness/review-decision mechanics.

The suggested fix is in the wrong direction:

teach review-panel-v1 to count a completed bot comment from a panel member declared in .ship.json as a completed review

evidence/panel.go:184 explicitly refuses this: "A comment saying 'Approved', 'LGTM', or 'ready to merge' without that line completes nothing — prose is not authority here, and a verdict with no commit anchor cannot state which tree it applies to." The attestation sentinel exists precisely BECAUSE counting bare prose was refused. Pointing at review-panel-v1 as the fix asks the code to do the thing it was designed not to do.

The real smallest fix is one of two paths (already noted in claude.yml's own comments):

• Path A: Widen the attestation trigger regex to match real-world review requests — e.g., anything where @claude [please] review is the first thing in the comment, even with trailing context.
• Path B: Have Claude post a formal GitHub review via the PR Reviews API (with the gh pr review system-prompt instruction already in claude.yml) rather than an issue comment — then latestExactHeadReview sees it directly.

Suggested rewrite for the entry's "smallest fix" and attribution lines:

Replace:

"because the panel signals by PR comment and never posts a formal GitHub review decision"

With something like:

"because the attestation trigger in claude.yml requires the trigger comment to be ONLY @claude [please] review with nothing after it. Real review requests include context, so the attestation step always skips, missing=[claude] is structural for typical usage, and the panel stand-in (#235) that would otherwise close this path never fires."

And replace the suggested fix:

"teach review-panel-v1 to count a completed bot comment from a panel member declared in .ship.json as a completed review"

With:

"widen the attestation regex in claude.yml to match @claude please review as a prefix rather than requiring the whole comment body to match, so the attestation fires on real review requests."

---

Cross-check: #253 staleness claim

Confirmed clean. PR #253 touches cycle counting and aborted-run records (run_aborted, cycle reducer, explain). It has no intersection with panel evidence gathering, the attestation step, the panelStandIn logic in readiness.go, or the local package timeout. The not-stale claim holds.

---

Summary

Entry	Accurate?	Action
Ollama cold-start timeout	Yes	None needed
Judgment-per-PR gap	Structural observation correct; attribution and suggested fix wrong	Rewrite attribution + fix pointer before merging

The ollama entry can merge as-is. The judgment entry should be corrected — it will otherwise log the wrong root cause and point future work at the wrong layer.

[itsHabib]

Round 2 — finding addressed, with one correction to the correction.

I verified all four load-bearing claims in the review against the code before rewriting, rather than taking them on trust. All four hold:

• local/local.go:28 — var httpClient = &http.Client{Timeout: 3 * time.Minute}, package-level, no injection point ✓
• readiness.go — escalates only when decisionAbsent && !stand.satisfied, so gate: a complete exact-head panel satisfies readiness #235's panelStandIn does answer the absent-decision case ✓
• claude.yml — the attestation step validates the whole body against ^@claude [please] review [this [pr]]$ and exits not a review request otherwise ✓
• evidence/panel.go — explicitly refuses prose as authority ✓

So entry 2's attribution was wrong and is rewritten: the bottleneck is the attestation step never firing, not the comment-vs-approval mechanic. My original suggested fix is likewise refused by design, and the entry now says so.

Where I did not follow the review: it proposed widening the regex to match @claude review as a prefix. The step's own comment rules that out — @claude review permissions and @claude review the failing CI logs start identically and must not attest, and it states the precision-over-recall choice is deliberate. A prefix match would credit those. The entry now points at the review's Path B instead (post a formal GitHub review), which recovers recall somewhere that cannot credit a review that never happened, and explicitly records the regex-widening as the tempting-but-wrong fix.

Entry 1 took the precision nit: the timeout is pinned to local/local.go:28 as the local package's client, not one gate sets.

This PR now also tests its own claim. The entry says a request carrying context never attests, and names the workaround: bare trigger, context in a separate comment. This comment is the context; the next one is the bare trigger. If the entry is right, the attestation fires this time and the panel completes — which would be the first run in this session that does not need a judgment to reach merge.

[itsHabib]

@claude please review

[claude]

Claude finished @itsHabib's task in 1m 42s —— View job

---

Review: PR #264 — friction-log.md session entries (round 3)

• Read the current friction-log.md diff
• Verify entry 1 (ollama cold-start) — unchanged, check still holds
• Verify entry 2 (judgment-per-PR) — rewritten; check revised attribution and fix pointer against code
• Post review findings

---

Entry 1 — Ollama cold-start timeout

Accurate. No changes needed.

All claims hold against the current code:

• local/local.go:28: var httpClient = &http.Client{Timeout: 3 * time.Minute} — package-level var, no flag, env var, or injection point. Confirmed.
• The connection refused vs. context deadline exceeded while awaiting headers distinction from docs: name friction-log.md in the Map and split it from FOLLOWUPS #214 is correct and meaningful: one is daemon-down, the other is cold-model-load racing the timeout with the daemon running.
• The smallest-fix pointer (surface local_model_unavailable vs. dressing an infra failure as a judgment question) is still unimplemented.

---

Entry 2 — A review request carrying context does not attest

Accurate. The rewrite is correct.

All four load-bearing claims verify against the code:

• Regex in claude.yml:156: '^[[:space:]]*@claude[[:space:]]+(please[[:space:]]+)?review([[:space:]]+(this([[:space:]]+pr)?|the[[:space:]]+pr))?[[:space:]]*[.!]?[[:space:]]*$' — the entire comment body is validated (bash =~, not grep), not just a prefix. Confirmed.
• readiness.go:293-295: decisionAbsent := pv.State != "MERGED" && pv.ReviewDecision == "" / if decisionAbsent && !stand.satisfied — escalation fires only when both the GitHub decision is absent AND the stand-in is unsatisfied. panelStandIn at verify/readiness.go:445 is the stand-in path; it requires attestation evidence. Confirmed.
• panel.go:182: "prose is not authority here, and a verdict with no commit anchor cannot state which tree it applies to" — the design refusal of counting bare prose is in the code and unchanged. Confirmed.
• Path B fix (formal GitHub review): the entry correctly identifies this as the fix that satisfies both latestExactHeadReview and panelStandIn without heuristic loosening. The system prompt already instructs Claude to post a formal PR review (gh pr review); the gap is that it's not always followed through. The entry could note this but doesn't need to for a friction log.

One observation worth noting (not a blocker): The entry's "workaround available today" — post bare @claude please review as its own comment — is being tested live by this PR's trigger. The trigger comment is @claude please review with nothing else, which matches the regex exactly. Whether the attestation fires will be visible in the CI run for this review. If it does, the entry has confirmed itself in practice; if not, something else is blocking attestation (e.g., a head-SHA mismatch at review time). Either outcome is informative and could warrant a follow-up note, but is not a reason to hold this PR.

---

Cross-check: structural completeness

The entry documents:

• What was tried ✓
• What happened ✓
• The cause (attestation trigger precision, not comment-vs-approval mechanic) ✓
• Why the tempting fixes are wrong (regex-widening, counting prose) ✓
• The correct smallest fix (Path B) ✓
• A working workaround available today ✓

This is well-structured and complete for a friction log entry.

---

Summary

Both entries are accurate in their final form. No changes needed. The rewrite of entry 2 correctly attributes the root cause, names the right fix, and explicitly rules out the two wrong approaches. Ready to merge.

[github-actions]

Reviewer: claude
Reviewed commit: 298058b6a2932a3de5a1551bcd2de76be0820c15