Skip to content

fix(detectors): stop the screening-model picker from silently pinning an auto-picked model - #1602

Open
ka1kqi wants to merge 7 commits into
mainfrom
fix/detector-unpinned-screening-model
Open

fix(detectors): stop the screening-model picker from silently pinning an auto-picked model#1602
ka1kqi wants to merge 7 commits into
mainfrom
fix/detector-unpinned-screening-model

Conversation

@ka1kqi

@ka1kqi ka1kqi commented Jul 23, 2026

Copy link
Copy Markdown
Collaborator

Summary

The detector create form and edit panel reuse the AI-assistant ModelSelector, whose auto-pick effect wrote the first-priority model (claude-opus-4-8) into form state whenever the incoming selection was empty. Submitting the create form, or saving any field in the edit panel of an unpinned detector, then persisted that as a pinned detectionModel — silently moving detection screening (one LLM eval per sampled trace at ingestion, the highest-volume LLM workload) off the cheap default (claude-haiku-4-5) onto the most expensive model, which is also marked up on paid plans.

This adds an opt-out placeholder prop to ModelSelector: when set, an empty selection is a valid persistent state — the picker shows the default as a display-only label instead of writing a selection into form state, and only an explicit user pick pins a model. Both detector screening surfaces pass it.

It also closes the other half of the same inconsistency in the worker: a detector whose detectionSource was never set (null) was excluded from the screening default and fell through to the provider-priority model, even though every UI surface labelled it as tracking the default.

Changes

  • ModelSelector gains an optional placeholder prop. When set, the reconcile effect no longer auto-picks a default for an empty selection, and the trigger shows the placeholder label. The adapter/legacy backfill branches and every consumer that doesn't pass the prop (assistant chat, the Agent Model pickers) are unchanged.
  • The create form and edit panel pass DETECTOR_SYSTEM_DEFAULT_MODEL_ID as the placeholder, so an unpinned detector shows the bare model id — the same label an explicit pick of that model renders.
  • The detectors list model column drops its (default) marker and shows the model name alone, matching the picker. That was the only consumer of the helper's isDefault flag, so it returns a plain label string now.
  • An untouched create form now persists detectionModel: null; the edit panel no longer converts an unpinned detector to a pin on an unrelated save.
  • The worker resolves a null detectionSource as "system", so legacy and API-created rows screen on the same default as everything else. That makes the list page's separate "Auto-selected" branch unreachable, so it is removed.

Behavior

  • Created or edited without touching the picker → detector tracks the system default, shown as claude-haiku-4-5 in both the picker and the list.
  • Pick a model explicitly → that model is pinned and shown.
  • An unpinned detector and one explicitly pinned to the same model now read identically everywhere. The distinction survives in the data — detectionModel: null follows the default if it ever changes, a pin does not — but is deliberately not surfaced.
  • Legacy detectionSource: null rows now screen on the system default instead of the provider-priority model, which is what the UI already claimed they did.
  • No data migration: rows already silently pinned are indistinguishable from deliberate picks, so this only stops new pins. Existing pinned rows keep their model.

Deployment invariant this establishes

Unpinned detectors now resolve to a fixed default model rather than an availability-aware provider pick. The previous auto-pick was availability-aware by construction, because the system-model catalog endpoint filters SYSTEM_MODELS by env-var presence — so a deployment with only OPENAI_API_KEY used to pin a working OpenAI model.

After this change, a deployment that holds no key for the default model's provider (neither in env nor as a workspace BYOK row) will fail those evals with No API key configured rather than quietly screening on whatever else is configured. This is accepted deliberately: it is the same tradeoff every explicitly-"system" detector already made, and it keeps the highest-volume LLM workload on a known cheap model instead of silently drifting to whatever a deployment happens to have. Deployments are expected to configure a key for the provider of DETECTOR_SYSTEM_DEFAULT_MODEL_ID.

Tests

  • ModelSelector: new coverage that a placeholder suppresses the auto-pick while preserving the legacy model-only backfill; existing auto-pick tests unchanged, which proves the other surfaces are unaffected.
  • Create form and edit panel: assert the screening-model picker receives the system-default placeholder.
  • Worker: null-source detectors resolve to the screening default, and attribute the run as system — billing is unaffected, since the only consumer asks whether the source is byok.
  • List page: null-source rows render the default label rather than the removed "Auto-selected" string, and no row renders a (default) marker.

Known gaps, deliberately not addressed here

  • A detectionSource: "byok" row with no detectionModel still resolves to the adapter's first curated model while the edit panel labels it the system default. That state is unreachable from the UI — picking a BYOK entry always sets a concrete model id — so it only arises from direct API calls or legacy data. The fix is write-time validation, which belongs with the upcoming public write-API work.
  • Un-pinning is not reachable from the UI: the popover lists only concrete models, and the patch builder omits an empty model, so a user who pins a model cannot return the detector to default-tracking. Pre-existing, but this change makes "empty" a first-class state and so raises the question.
  • Detector write failures are still invisible to the user; tracked separately.

Screenshots

The earlier screenshots showed the interim System default (claude-haiku-4-5) wording and have been removed — the picker now renders the bare model id.

ka1kqi and others added 3 commits July 23, 2026 12:54
…holder state

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…reening model

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…default

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@ka1kqi
ka1kqi requested a review from a team as a code owner July 23, 2026 19:55

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found across 7 files

Confidence score: 5/5

  • Automated review surfaced no issues in the provided summaries.
  • No files require special attention.

You’re at about 98% of the monthly reviewed-line limit. You may want to disable incremental reviews to conserve quota. Reviews will continue until that limit is exceeded. If you need help avoiding interruptions, please contact contact@cubic.dev.

Architecture diagram
sequenceDiagram
    participant User as User (Browser)
    participant CreateForm as Detector Create Form
    participant EditPanel as Detector Edit Panel
    participant ModelSelector as ModelSelector Component
    participant Templates as templates.ts
    participant Core as @traceroot/core/llm-providers
    participant FormState as Form State (React)
    participant FormSubmit as Form Submit Handler
    participant API as API Route
    participant Worker as Detection Worker

    Note over User,Worker: Detector Screening Model Selection Flow

    User->>CreateForm: Opens new detector page
    CreateForm->>Templates: Imports DETECTOR_MODEL_PLACEHOLDER
    Templates->>Core: Reads DETECTOR_SYSTEM_DEFAULT_MODEL_ID
    Core-->>Templates: Returns "claude-haiku-4-5"
    Templates-->>CreateForm: Placeholder = "System default (claude-haiku-4-5)"

    CreateForm->>ModelSelector: Renders with placeholder prop
    ModelSelector->>ModelSelector: Reconcile effect runs
    alt Empty selection + placeholder set
        ModelSelector->>ModelSelector: Skips auto-pick default
        Note over ModelSelector: Displays placeholder label in trigger button
    else Existing non-empty selection
        ModelSelector->>ModelSelector: Behaves normally (backfill, auto-pick)
    end
    ModelSelector-->>CreateForm: Shows "System default (claude-haiku-4-5)"

    User->>ModelSelector: Skips model picker (no interaction)
    User->>FormSubmit: Clicks create
    FormSubmit->>FormState: Reads detectionModel = null
    FormSubmit->>API: POST /detectors { detectionModel: null }
    API-->>FormSubmit: 201 Created
    FormSubmit-->>User: Redirect to detector

    Note over Worker: At evaluation time
    Worker->>Worker: Resolves null to system default (claude-haiku-4-5)

    Note over User,EditPanel: Edit Flow

    User->>EditPanel: Opens detector edit panel
    EditPanel->>Templates: Imports DETECTOR_MODEL_PLACEHOLDER
    EditPanel->>ModelSelector: Renders with placeholder prop
    ModelSelector->>ModelSelector: Reconcile effect runs
    alt Detector has null detectionModel + placeholder set
        ModelSelector->>ModelSelector: Skips auto-pick default
    else Detector has pinned model
        ModelSelector->>ModelSelector: Shows pinned model
    end
    ModelSelector-->>EditPanel: Shows placeholder or pinned model

    User->>EditPanel: Saves unrelated field (e.g., sample rate)
    EditPanel->>FormState: Reads detectionModel = null (unchanged)
    EditPanel->>API: PATCH /detectors { detectionModel: null }
    API-->>EditPanel: 200 OK

    Note over User,Worker: User explicitly picks a model

    User->>ModelSelector: Opens dropdown and selects model
    ModelSelector->>FormState: Writes concrete ModelSelection
    User->>FormSubmit: Saves
    FormSubmit->>API: POST/PATCH with pinned detectionModel
    API-->>FormSubmit: Success
    Worker->>Worker: Uses pinned model for evaluations
Loading

Re-trigger cubic

@XinweiHe

Copy link
Copy Markdown
Collaborator

maybe remove the system default ..., just say haiku-4.5 as usual should be fine.

ka1kqi and others added 4 commits July 27, 2026 16:56
The screening-model picker labelled an unpinned detector "System default
(claude-haiku-4-5)". Show the bare model id instead, matching how every
other model renders in the picker and how the detectors list already
labels a default-tracking row. Drops the now-redundant wrapper constant
and passes DETECTOR_SYSTEM_DEFAULT_MODEL_ID directly.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The worker gated the cheap screening default on detectionSource === "system"
specifically, so a null source fell through to the provider-priority model
instead. Nothing else in the app agrees with that: every UI write sends
"system" explicitly, and the edit panel reads null back as "system". A
legacy or API-created detector therefore screened on the priority model
while the list labelled it "Auto-selected" and the panel labelled it the
screening default.

Resolve null as system at the point of use, which lets the list page's
separate legacy branch go.

Two consequences worth naming. Combined with the pickers persisting an empty
selection, every unpinned detector — new ones included — now screens on the
fixed default rather than an availability-aware provider pick, so a
deployment holding no key for that provider fails those evals instead of
falling back to whatever else is configured. And attribution records these
runs as system, matching the model that actually ran; billing is unchanged,
since the processor only asks whether the source is byok.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The column now reads the same for an unpinned detector as for one pinned to
that same model, matching the picker, which already shows the bare model id
either way. With no consumer left for the flag, formatDetectorModel returns
a plain label string.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Removes state the current behavior no longer produces:

- inferenceSource can no longer be null now that the worker resolves a
  never-set source as system, so the type says so and the compiler enforces
  the invariant instead of a comment.
- Five negative assertions guarded prefixed label formats ("System default:",
  "BYOK provider:") from an earlier design; none of those strings exist in
  the app. Two of the three that remain guard labels this branch removed;
  the third guards branch ordering in formatDetectorModel, where a pinned
  model must win over the BYOK provider name.
- The edit-panel placeholder test set detectionModel/Provider to null as if
  the prop were conditional. It never was, so the fixture implied a branch
  that does not exist; the test now covers pinned and unpinned alike and
  says the prop is unconditional.
- ModelSelector is generic, but its placeholder tests used a detector model
  id, reading as though the prop were detector-specific.
- formatDetectorModel's result is no longer only a model — it can be a
  provider name — so the call site calls it modelLabel.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

@XinweiHe XinweiHe left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's do these before the merge

  1. need new screenshots
  2. fix the merge conflict

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants