Skip to content

Sidecar preflight: base-probe success never validates the real serving token budget #1454

Open

Description

@seonghobae

Background

Devin Review, on PR #1452 (ADR-0005's implementation), verified against current code:
scripts/ci/contextual_orchestrator_review_launcher.py's _preflight_review_agents admits a
candidate into the serving pool the moment its cheap REVIEW_PREFLIGHT_BASE_TOKENS = 16 base probe
returns any non-empty content. Escalation to REVIEW_PREFLIGHT_ESCALATED_TOKENS (REVIEW_MAX_OUTPUT_TOKENS,
4096 — the real budget main()'s serving ModelClient uses for actual review traffic) only fires on
evidence of failure (empty content matching the budget-too-small signature). A candidate that
succeeds at 16 tokens is never probed at 4096 before being admitted.

Why this is a real gap, not just theoretical

ADR-0005's own Research section (docs/adr/0005-sidecar-preflight-token-budget.md, axis 2, around line
81) already documents that provider completion-token ceilings are a real, evidenced, per-model quantity
separate from reasoning-overhead (axis 1): "Some providers reject a request outright if max_tokens
exceeds what that specific model supports."
Axis 1 (reasoning overhead) is what the escalation path
corrects for. Axis 2 (a hard completion ceiling strictly between 16 and 4096) is not corrected for on
the success path: if a candidate's real ceiling is, say, 500 tokens, it happily returns real content at a
16-token probe, gets admitted, and then a real review request at 4096 tokens can be rejected by the
provider outright.

Layer 2 (the shell script's separate virtual-pool smoke request) does exercise the real 4096-token budget
— but only against the ONE candidate the virtual pool router happens to select for that single smoke
request, not against every individually-admitted Layer 1 candidate. With up to 12 admitted candidates,
this leaves most of the pool's success-path candidates unvalidated at the real serving budget.

Known mitigation (partial, not a fix)

contextual_orchestrator.orchestrator.TaskOrchestrator/ModelClient already implements per-request
failover to the next capability-matched agent plus a per-agent circuit breaker in production. A candidate
that is actually incompatible with the real budget should fail over at request time rather than silently
serve garbage — but this still costs latency/retries on real review traffic and could trip a circuit
breaker on an otherwise-healthy candidate that was simply preflighted at the wrong budget.

Options considered (not evaluated to a decision — needs its own design pass)

  1. Probe the real 4096-token budget for every candidate, not just on the failure/escalation path — but
    this risks reintroducing exactly the original bug ADR-0005 exists to fix (a uniformly-large probe
    budget caused the 120s-timeout regression that motivated this whole redesign).
  2. Block on real per-model max_output_tokens/context_window discovery data (tracked separately,
    ContextualWisdomLab/contextual-orchestrator#927) so admission can be decided from known ceilings
    instead of an empirical probe at all.
  3. Something narrower — e.g. a second, cheap-but-larger confirmation probe only for candidates that will
    actually be selected into the served catalog, bounded so it doesn't reintroduce the timeout regression.

Ask

Design a fix (or an explicitly accepted, documented residual-risk decision, per this org's convergence
convention) for the success-path gap. Not blocking PR #1452's fix of the 7 verified Devin Review findings
already in that PR — flagged there as architecturally significant enough to need its own pass rather than
a guessed patch.

Cross-ref: ADR-0005 (docs/adr/0005-sidecar-preflight-token-budget.md), PR #1452, PR #1449,
ContextualWisdomLab/contextual-orchestrator#927.

Activity

  1. seonghobae commented on Aug 30, 2026

    @seonghobae
    ContributorAuthor

    Decision (2026-08-30): accepted as a known, tracked residual risk — not blocking PR #1452

    Reviewed with the coordinator against the three options above:

    None of the three is a safe, obviously-right fix to make unilaterally right now. PR #1452's design is a
    genuine, verified improvement over the status quo it replaces (which had no diagnostic retry at all,
    and reproduced the 120s-timeout bug repeatedly) — it does not need to close every residual failure mode to
    be worth merging. The risk this issue describes is real but partially mitigated today by
    contextual_orchestrator.orchestrator.TaskOrchestrator's existing per-request failover and per-agent
    circuit breaker, and is now durably tracked here instead of silently unhandled the way it was before this
    PR existed.

    This stays open, as an accepted, tracked, non-blocking residual-risk item — not resolved, not closed,
    not fixed. Follow-up design work (evaluating (a)/(b)/(c) or another option, once #927 or a narrower-probe
    design exists) happens here when someone picks it up.

    Cross-referenced from docs/adr/0005-sidecar-preflight-token-budget.md and
    scripts/ci/contextual_orchestrator_review_launcher.py (both already point here); the corresponding PR
    #1452 review thread is resolved with a pointer to this comment.


    Generated by Claude Code

  2. added
    area: ci-cdCI, GitHub Actions, checks, release, or supply chain
    bugSomething isn't working
    status: triagedOpen issue has an organization taxonomy assignment
    type: bugDefect or incorrect behavior
    on Sep 2, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    area: ci-cdCI, GitHub Actions, checks, release, or supply chainbugSomething isn't workingpriority: highHigh-priority or P1 workstatus: triagedOpen issue has an organization taxonomy assignmenttype: bugDefect or incorrect behavior

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions