Independent review found 1 BLOCKER + 5 CONCERNs + 4 NITs. Resolutions:
B1 BLOCKER: summary callback signature was inconsistent across §3 and §6.
Canonical now: summarize_fn(prior_summary, evicted_turns) -> string|nil
dispatching on the two args:
(nil, [turns]) — first-time summarize
(str, [turns]) — additive (extend prior summary with new evictions)
(str, nil) — compress (re-summarize the prior summary itself)
C1: re-summarize trigger now uses the (str, nil) compress signal
rather than degenerate (str, {}).
C2: routing decision is taken once on entry to ask_ai. The chosen
active_cfg is used for every tool-sub-loop iteration. Original
active_cfg restored after ask_ai returns.
C3: AUTO-routing does NOT fire inside the Norris loop. Model fixed
at :norris launch time; planner stays on it for every iteration.
Q39 resolved. Per-iteration fallback still gated by
cfg.routing.fallback — retries the failing call against cloud
without permanently switching the planner.
C4: Summary block suppressed in Norris (mirrors Phase 4 R-C1 for
the [background] block). Both are "earlier context" the planner
generally doesn't need.
C5: Fallback pattern coverage expanded — added HTTP 408 (Q41
resolved) and "Operation timed out" (libcurl version variant).
Dropped "HTTP response code said error" from A2 — FAILONERROR
was removed in Phase 4 f26cbd9.
NITs folded:
N1 :route check <text> always runs heuristic; suffix
"(routing currently disabled)" when cfg.routing.auto = false
N2 reasoning → nil by default (not → "cloud"); user explicitly
opts in to map reasoning to a paid model. Same cost-safety
rationale as confirm_cmd default true.
N3 "Retry only when no deltas have arrived" promoted to §5
normative rule (was in §11 risk row).
N4 cfg.routing.cloud_fallback renamed cfg.routing.fallback to
align with the :fallback meta verb.
Reviewer verdict: commit #1 (router.classify_model) is implement-
ready; B1/C1 resolution required before commit #2.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
marfrit
2e389c1475