Skip to content

feat(validation): reject provably-doomed inference requests at ingress - #1839

Open
pjb157 wants to merge 12 commits into
mainfrom
feat/ingress-validation
Open

pjb157 wants to merge 12 commits into
mainfrom
feat/ingress-validation

Conversation

@pjb157

@pjb157 pjb157 commented Sep 23, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Adds an ingress validation stage that rejects inference requests which are certain to fail, before they are forwarded to a provider or enqueued. Rejections are fast, consistent, and use the error shape of the surface the client called. Clients no longer wait in a queue for a provider error, and doomed requests no longer consume provider or queue capacity.

The same stage runs in two places:

  • Inference endpoints (/v1/chat/completions, /v1/completions, /v1/responses, /v1/messages, /v1/embeddings): in the inference middleware, after the body is parsed (and after previous_response_id hydration) and before the realtime/flex split, so it covers realtime and queued requests alike. It only validates callers whose key may use the requested model (bearer token or x-api-key), checked against the same routing table and request-class pool onwards authorises against. Unknown models are left to the normal routing 404. Everyone else gets the normal authentication or access error, so a rejection never describes a model the caller cannot use.
  • Batch input files: every line of an uploaded purpose=batch file. A doomed line fails the upload and the error names the line.

Rules

Rule Fires when
model_type_mismatch The model's type does not match the endpoint (e.g. an embeddings model on chat completions)
invalid_service_tier service_tier is not a recognised value
invalid_max_tokens The max-tokens field is not a positive integer
max_tokens_exceeds_limit The max-tokens field exceeds the model's max_output_tokens or context_window
unsupported_modality Image input is sent to a model whose capabilities don't include vision (audio and file inputs are tracked but not gated yet)
context_length_exceeded The prompt is larger than the model's context_window

Guardrails:

  • Precision over recall. A rule fires only when the request provably cannot succeed. Every rule passes when the model metadata it needs is missing.
  • Shadow first. The feature is off by default. Once enabled, every rule starts in shadow mode: it records dwctl_request_validation_violations_total{rule,surface,source,model,mode} and forwards the request. Each rule can be switched to enforce on its own.
  • Lossless. Unknown request fields are never a reason to reject.

Context-length check

The check has two stages, so the common case costs nothing:

  1. Byte count. Byte-level BPE never produces more tokens than bytes, so a prompt that fits the window in bytes passes without a count. For completions/embeddings input lists, each element is its own sequence and the largest one is measured.
  2. Exact count. Larger prompts get an exact chat-templated count from the tokenizer service, under a deadline. If the tokenizer is unavailable or too slow, the request passes. A circuit breaker skips the tokenizer for a cooldown after a timeout or outage.

Context rejections always rest on an exact count, so with exact counting disabled the context rule never rejects.

Model metadata

Rules read a per-alias metadata cache (sync/model_metadata.rs). It mirrors zdr_keys: it loads at startup and refreshes on auth_config_changed, with a periodic fallback. Catalog metadata gains max_output_tokens. context_window and max_output_tokens are now range-checked on the admin API and in model provisioning.

Other changes

  • Malformed JSON on the inference endpoints now returns a 400 error envelope (code: invalid_json) instead of an empty body, and a failed body read returns a body_read_failed envelope. Both apply whether or not validation is enabled.
  • The batch upload model-type check now uses the same endpoint-to-model-type mapping as the validation rules.

Configuration

request_validation:
  enabled: false            # DWCTL_REQUEST_VALIDATION__ENABLED
  default_mode: shadow      # off | shadow | enforce
  rules: {}                 # e.g. DWCTL_REQUEST_VALIDATION__RULES__MODEL_TYPE_MISMATCH=enforce
  exact_count_enabled: false
  exact_count_deadline_ms: 2000
  exact_count_max_per_batch_file: 1000

Suggested rollout: enable in shadow mode, check the violations metric for false positives, enable exact counting, then enforce rule by rule.

Test plan

  • Unit tests for request extraction on every surface, each rule and its boundaries, both error envelopes, exact counting (mocked tokenizer: counted, fallback, unmapped memoization, unavailable, timeout), and the metadata cache (including a real-DB NOTIFY refresh test)
  • End-to-end tests through the full app: enforce/shadow behaviour, callers without model access are left to normal auth errors, the Anthropic envelope on /v1/messages, the malformed-JSON envelope, flex rejection leaving no queued request, modality, context length, model-type mismatch, fail-open without metadata, and batch upload (a doomed line is rejected with its line number; shadow mode accepts the file)
  • cargo test -p dwctl: 2668 passed
  • cargo sqlx prepare --check, cargo fmt

🤖 Generated with Claude Code

Engines return a steady stream of 4xx for requests that were never going
to succeed (unknown models, text-only models sent images, prompts over the
context window), and queued requests for them occupy queue and retry
capacity before failing. Add one validation stage and run it wherever
requests enter: the inference middleware (after the body is parsed, before
the realtime/flex split) and every line of an uploaded batch input file.
A doomed request is rejected before it is forwarded or enqueued.

- inference/validation: surface-aware request view, rules (model not
  found, model type vs endpoint, service_tier, max tokens, modality,
  context length), OpenAI/Anthropic rejection envelopes, and a two-stage
  context check: prompts that fit the window in bytes pass for free; larger
  ones get an exact chat-templated count from the tokenizer service under
  a deadline.
- sync/model_metadata: per-alias metadata cache refreshed on
  auth_config_changed, mirroring zdr_keys. Missing metadata fails open.
- Batch upload reuses the same ValidationStage (shared via AppState) and
  the same endpoint -> model type mapping.
- Rules are off unless request_validation.enabled, default to shadow
  mode (dwctl_request_validation_violations_total, labelled by source),
  and are enforced per rule.
- Malformed JSON now returns a 400 error envelope instead of an empty body.
- Catalog metadata gains max_output_tokens; context_window and
  max_output_tokens are range-checked on the admin API and in provisioning.
Copilot AI lite review requested due to automatic review settings September 23, 2026 16:40
@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Sep 23, 2026 •

Copy link
Copy Markdown

Deploying control-layer with  Cloudflare Pages  Cloudflare Pages

Latest commit: f6c1e29
Status: ✅  Deploy successful!
Preview URL: https://5f6d005f.control-layer.pages.dev
Branch Preview URL: https://feat-ingress-validation.control-layer.pages.dev

View logs

Copilot AI 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.

Copilot review overview

🟡 Changes recommended

Unresolved authorization, exact-counting, context-bound, and validation-correctness issues remain.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 3 High severity

Open (3)
What changed in this PR

Adds configurable ingress validation for inference and batch requests, backed by cached model metadata and optional exact token counting.

Changes:

  • Adds validation rules, error envelopes, metrics, and shadow/enforce modes.
  • Integrates validation into inference middleware and batch uploads.
  • Adds metadata synchronization and provisioning constraints.
  • Updates configuration, tests, dashboard types, and SQLx metadata.
File Summary
model-provisioning.schema.json Adds token metadata constraints.
dwctl/​src/​test/​utils.rs Updates validation test configuration.
dwctl/​src/​test/​request_validation.rs Adds end-to-end validation tests.
dwctl/​src/​test/​mod.rs Registers validation tests.
dwctl/​src/​sync/​model_metadata.rs Implements metadata caching and refresh.
dwctl/​src/​sync/​mod.rs Registers metadata synchronization.
dwctl/​src/​model_provisioning.rs Validates provisioning metadata.
dwctl/​src/​metrics/​errors.rs Adds metadata-sync metrics.
dwctl/​src/​lib.rs Wires validation and synchronization.
dwctl/​src/​inference/​validation/​view.rs Extracts validation inputs.
dwctl/​src/​inference/​validation/​stage.rs Coordinates validation behavior.
dwctl/​src/​inference/​validation/​rules.rs Implements validation rules.
dwctl/​src/​inference/​validation/​mod.rs Defines validation types and configuration.
dwctl/​src/​inference/​validation/​exact.rs Adds tokenizer-backed counting.
dwctl/​src/​inference/​validation/​envelope.rs Renders client-facing errors.
dwctl/​src/​inference/​mod.rs Exposes validation modules.
dwctl/​src/​inference/​middleware.rs Integrates ingress validation.
dwctl/​src/​db/​models/​deployments.rs Adds model token metadata.
dwctl/​src/​db/​handlers/​api_keys.rs Updates test configuration.
dwctl/​src/​config.rs Adds validation configuration.
dwctl/​src/​auth/​middleware.rs Updates test state.
dwctl/​src/​api/​handlers/​files.rs Validates batch input lines.
dwctl/​src/​api/​handlers/​deployments.rs Validates catalog metadata.
dashboard/​src/​api/​control-layer/​types.ts Updates dashboard metadata types.
config.yaml Documents validation settings.
.sqlx/​query-45070a43e7311f3f5b692b4c5d792425e72cc38ef0b2e1940ff750d492f7fdc2.json Adds SQLx query metadata.
Files not reviewed (1)
  • .sqlx/query-45070a43e7311f3f5b692b4c5d792425e72cc38ef0b2e1940ff750d492f7fdc2.json: Generated file

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread dwctl/src/inference/middleware.rs Outdated
Comment on lines +194 to +196
if let (Some(validation), Some(surface)) = (&state.validation, surface)
&& let Some(rejection) = validation.check(surface, &request_value).await
{

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed in efeed8c. Validation now runs only for callers whose key may use the requested model, checked against the same in-memory routing table onwards authorises against (ModelAccess in validation/stage.rs). Unauthenticated or unauthorised traffic never reaches the tokenizer and gets the normal auth error downstream. The exact counter also has a circuit breaker now (61c21c6): after a timeout or outage it stops calling the tokenizer for a cooldown, which bounds the cost of repeated near-limit bodies.

Comment thread dwctl/src/inference/middleware.rs Outdated
Comment on lines +192 to +197
// Validate on the bare alias (serving-class suffix already stripped) and
// before anything is persisted, forwarded or enqueued.
if let (Some(validation), Some(surface)) = (&state.validation, surface)
&& let Some(rejection) = validation.check(surface, &request_value).await
{
return rejection;

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed in efeed8c. Rules run only when the caller's key may use the model (same check as onwards' own routing and /models visibility). Anyone else gets the normal 401/403/404, so a rejection never describes a model the caller can't use. Covered by callers_without_access_are_not_validated (unit and end-to-end).

Comment thread dwctl/src/inference/validation/rules.rs Outdated
Comment on lines +280 to +287
// With exact counting on, every rejection is backed by a real count: the
// size heuristic below assumes no token spans more than
// `reject_min_bytes_per_token` bytes, which is likely but not provable.
let estimated = bytes / config.reject_min_bytes_per_token;
if !config.exact_count_enabled && estimated > limit {
evaluation
.violations
.push(context_length_violation(view.surface, context_window, estimated.floor() as u64));

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Agreed. Removed in efeed8c along with reject_min_bytes_per_token. Stage 1 now only passes prompts (bytes <= context_window is a proof, since byte-level BPE gives tokens <= bytes). Anything larger is rejected only on an exact tokenizer count, so with exact counting off the context rule never rejects.

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

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

6 issues found across 26 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="dwctl/src/lib.rs">

<violation number="1" location="dwctl/src/lib.rs:3524">
P1: This background task can take the whole server down when its initial listener connection fails, contradicting the fail-open behavior promised for unavailable model metadata. Keep the sync task alive with retry/backoff around listener setup (as well as reload failures) so validation continues using the last or empty cache.</violation>
</file>

<file name="dwctl/src/config.rs">

<violation number="1" location="dwctl/src/config.rs:241">
P2: `request_validation` accepts `reject_min_bytes_per_token: 0` without startup validation, so enabling the feature can reject valid near-limit prompts because the context rule divides by zero. Validate this value as finite and strictly positive before enabling request validation.</violation>
</file>

<file name="dwctl/src/api/handlers/files.rs">

<violation number="1" location="dwctl/src/api/handlers/files.rs:960">
P2: The new validation error reports the accepted-request ordinal instead of the JSONL source line. Track the physical line number separately so blank lines do not misidentify which batch entry failed.</violation>
</file>

<file name="dwctl/src/inference/validation/stage.rs">

<violation number="1" location="dwctl/src/inference/validation/stage.rs:64">
P1: Responses continuations are validated before their prior response is inlined, so context checks and exact counts omit prior-turn tokens and can forward or enqueue a request that exceeds the model window after hydration. Run validation after hydration or validate the hydrated body.</violation>
</file>

<file name="dwctl/src/inference/validation/view.rs">

<violation number="1" location="dwctl/src/inference/validation/view.rs:53">
P1: `service_tier: "standard_only"` is valid on the Anthropic Messages surface, but the global tier rule rejects it after this extraction. Keep the allowlist surface-aware or skip this rule for Messages.</violation>
</file>

<file name="dwctl/src/test/request_validation.rs">

<violation number="1" location="dwctl/src/test/request_validation.rs:651">
P3: The shadow-mode tests never verify that the would-be rejection was actually recorded, so a wiring bug that silently skipped validation in shadow mode (or never recorded the violation) would still pass. `ValidationStage.enforced_violation` records every violation before forwarding (rules::record + tracing), so assert on that observability (a violation counter or log record) in addition to the forwarded response.</violation>
</file>

Tip: instead of fixing issues one by one fix them all with cubic

Re-trigger cubic

Comment thread dwctl/src/lib.rs
let metadata_cache = cache.clone();
let metadata_shutdown = shutdown_token.clone();
let metadata_fallback = config.background_services.onwards_sync.fallback_interval_milliseconds;
background_tasks.spawn("model-metadata-sync", async move {

@cubic-dev-ai cubic-dev-ai Bot Sep 23, 2026 •

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1: This background task can take the whole server down when its initial listener connection fails, contradicting the fail-open behavior promised for unavailable model metadata. Keep the sync task alive with retry/backoff around listener setup (as well as reload failures) so validation continues using the last or empty cache.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At dwctl/src/lib.rs, line 3524:

<comment>This background task can take the whole server down when its initial listener connection fails, contradicting the fail-open behavior promised for unavailable model metadata. Keep the sync task alive with retry/backoff around listener setup (as well as reload failures) so validation continues using the last or empty cache.</comment>

<file context>
@@ -3481,6 +3499,44 @@ async fn setup_background_services(input: BackgroundServicesInput) -> anyhow::Re
+        let metadata_cache = cache.clone();
+        let metadata_shutdown = shutdown_token.clone();
+        let metadata_fallback = config.background_services.onwards_sync.fallback_interval_milliseconds;
+        background_tasks.spawn("model-metadata-sync", async move {
+            crate::sync::model_metadata::run(
+                metadata_pool,
</file context>
Fix with cubic

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed in d9a8549. run() no longer returns on DB errors: listener setup is retried with capped exponential backoff (1s→30s, honouring shutdown) and only returns on shutdown. Tested with a pool whose first connect fails.

Comment thread dwctl/src/inference/validation/exact.rs Outdated
/// Run every rule on a parsed body, record all violations, and return the
/// first enforced one. Callers render it in their own error shape.
pub async fn enforced_violation(&self, surface: Surface, body: &Value, source: Source) -> Option<Violation> {
let view = view::extract(surface, body);

@cubic-dev-ai cubic-dev-ai Bot Sep 23, 2026 •

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1: Responses continuations are validated before their prior response is inlined, so context checks and exact counts omit prior-turn tokens and can forward or enqueue a request that exceeds the model window after hydration. Run validation after hydration or validate the hydrated body.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At dwctl/src/inference/validation/stage.rs, line 64:

<comment>Responses continuations are validated before their prior response is inlined, so context checks and exact counts omit prior-turn tokens and can forward or enqueue a request that exceeds the model window after hydration. Run validation after hydration or validate the hydrated body.</comment>

<file context>
@@ -0,0 +1,196 @@
+    /// Run every rule on a parsed body, record all violations, and return the
+    /// first enforced one. Callers render it in their own error shape.
+    pub async fn enforced_violation(&self, surface: Surface, body: &Value, source: Source) -> Option<Violation> {
+        let view = view::extract(surface, body);
+        let lookup = match view.model.as_deref() {
+            Some(alias) => self.models.lookup(alias),
</file context>
Fix with cubic

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed in efeed8c. The validation call moved after previous_response_id hydration (still before the realtime/flex split), so prior turns count toward the context window.

Comment thread dwctl/src/inference/validation/view.rs
// `model` is the bare alias string; non-string values are ignored.
model: body.get("model").and_then(Value::as_str).map(str::to_owned),
// Raw, so a rule can report a non-string tier rather than silently drop it.
service_tier: body.get("service_tier").cloned(),

@cubic-dev-ai cubic-dev-ai Bot Sep 23, 2026 •

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1: service_tier: "standard_only" is valid on the Anthropic Messages surface, but the global tier rule rejects it after this extraction. Keep the allowlist surface-aware or skip this rule for Messages.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At dwctl/src/inference/validation/view.rs, line 53:

<comment>`service_tier: "standard_only"` is valid on the Anthropic Messages surface, but the global tier rule rejects it after this extraction. Keep the allowlist surface-aware or skip this rule for Messages.</comment>

<file context>
@@ -0,0 +1,709 @@
+        // `model` is the bare alias string; non-string values are ignored.
+        model: body.get("model").and_then(Value::as_str).map(str::to_owned),
+        // Raw, so a rule can report a non-string tier rather than silently drop it.
+        service_tier: body.get("service_tier").cloned(),
+        ..RequestView::default()
+    };
</file context>
Fix with cubic

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed in efeed8c. The accepted service_tier values are surface-aware; /v1/messages also accepts standard_only.

Comment thread dwctl/src/sync/model_metadata.rs Outdated
Comment thread dwctl/src/test/request_validation.rs Outdated
let too_long = "a".repeat(5000);
let response = fixture.upload_batch(&batch_jsonl(&alias, &[&too_long])).await;

assert_eq!(response.status_code(), StatusCode::CREATED, "{}", response.text());

@cubic-dev-ai cubic-dev-ai Bot Sep 23, 2026 •

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P3: The shadow-mode tests never verify that the would-be rejection was actually recorded, so a wiring bug that silently skipped validation in shadow mode (or never recorded the violation) would still pass. ValidationStage.enforced_violation records every violation before forwarding (rules::record + tracing), so assert on that observability (a violation counter or log record) in addition to the forwarded response.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At dwctl/src/test/request_validation.rs, line 651:

<comment>The shadow-mode tests never verify that the would-be rejection was actually recorded, so a wiring bug that silently skipped validation in shadow mode (or never recorded the violation) would still pass. `ValidationStage.enforced_violation` records every violation before forwarding (rules::record + tracing), so assert on that observability (a violation counter or log record) in addition to the forwarded response.</comment>

<file context>
@@ -0,0 +1,652 @@
+    let too_long = "a".repeat(5000);
+    let response = fixture.upload_batch(&batch_jsonl(&alias, &[&too_long])).await;
+
+    assert_eq!(response.status_code(), StatusCode::CREATED, "{}", response.text());
+}
</file context>
Fix with cubic

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Partly addressed: the shadow end-to-end test now uses a known model that breaks a rule (so a rule definitely fires) and asserts the request is forwarded and unmarked. Asserting the counter would need a process-global metrics recorder in the test harness, which other tests share, so I've left that to the unit level (rules::record / describe_metrics).

Comment thread config.yaml Outdated
Comment thread dwctl/src/inference/validation/rules.rs Outdated
pg_stat_activity shows the LISTEN statement from the moment it starts; a
NOTIFY committed before the LISTEN commits is never delivered, so the
barrier must also require the connection to be idle.

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

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

All reported issues were addressed across 1 file (changes from recent commits).

Reply with feedback, questions, or to request a fix.

Fix all with cubic | Re-trigger cubic

Comment thread dwctl/src/sync/model_metadata.rs Outdated
pjb157 and others added 9 commits September 24, 2026 13:48
- Validate only callers whose key may use the requested model (the same
  routing table onwards authorises against). Everyone else gets the normal
  authentication/access error, so rejections never describe a model the
  caller cannot use and unauthenticated traffic never reaches the tokenizer.
- Validate after previous_response_id hydration so prior turns count
  toward the context window.
- Drop the size-only context rejection: no byte ratio proves a prompt is
  over the window, so context rejections always rest on an exact count
  (removes reject_min_bytes_per_token).
- Completions/embeddings inputs are independent sequences: bound the
  largest element, not the sum.
- Accept Anthropic's `standard_only` service tier on /v1/messages.
- max tokens: strict u32 integers on Messages/Responses, integral floats
  bounded by u32::MAX elsewhere; a null max_completion_tokens falls back to
  max_tokens.
- Responses: only function tools and URL images count toward the view,
  matching what the translator forwards.
- Tests: poll with a short sleep; document the `source` metric label.
Retry listener setup with capped backoff instead of ending the task, reload
once after every (re)subscribe so changes committed before LISTEN are not
missed, and use saturating_sub for the debounce deadline.
…cuit breaker

Completions prompt and embeddings input lists are independent sequences, so
the exact count is the largest element, not the sum. After a timeout or
outage the exact counter stops calling tokenizer-svc for a cooldown, so an
outage cannot cost one deadline per request or per batch line.
…into feat/ingress-validation

# Conflicts:
#	dwctl/src/inference/middleware.rs
#	dwctl/src/lib.rs
#	dwctl/src/sync/mod.rs
#	dwctl/src/test/mod.rs
- Drop the model_not_found rule: validation only runs once routing has
  accepted the alias, so an unknown alias in the metadata cache can only mean
  that cache is stale, and routing already answers genuinely unknown models.
- Authorise validation against the pool onwards resolves for the request
  class, and accept Anthropic's x-api-key header for the gate.
- Stop gating audio input: the catalog has no audio capability vocabulary,
  so its absence proves nothing.
- Only open the exact-count circuit breaker on real outages (transport,
  5xx, 408, 429); other tokenizer 4xx responses are about the request.
- Cap exact counts per batch file upload
  (request_validation.exact_count_max_per_batch_file, default 1000).
- Tests for x-api-key callers and for disabled validation forwarding a
  violating request; doc and config.yaml placement fixes.

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

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

1 issue found across 10 files (changes from recent commits).

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="dwctl/src/inference/validation/stage.rs">

<violation number="1" location="dwctl/src/inference/validation/stage.rs:152">
P3: The per-upload budget is consumed in shadow mode too (exact counts still run to record violations there), so a file with >1000 near-limit lines stops producing shadow violation metrics past the budget — silently under-reporting exactly the signal the shadow rollout phase uses to judge enforcement impact. If tokenizer load capping is the goal, consider not debiting the budget when the context rule is only in shadow; if the cap must apply regardless, document the shadow-mode measurement bias.</violation>
</file>

Tip: Review your code locally with the cubic CLI to iterate faster.

Re-trigger cubic

Comment thread dwctl/src/inference/middleware.rs Outdated
let decided = rules::first_enforced(&evaluation.violations, &self.config).is_some();
if let (Some(needed), Some(exact), Some(alias), false) = (&evaluation.exact_count, &self.exact, view.model.as_deref(), decided)
&& self.config.mode(RuleId::ContextLengthExceeded) != RuleMode::Off
&& budget.is_none_or(ExactCountBudget::try_take)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P3: The per-upload budget is consumed in shadow mode too (exact counts still run to record violations there), so a file with >1000 near-limit lines stops producing shadow violation metrics past the budget — silently under-reporting exactly the signal the shadow rollout phase uses to judge enforcement impact. If tokenizer load capping is the goal, consider not debiting the budget when the context rule is only in shadow; if the cap must apply regardless, document the shadow-mode measurement bias.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At dwctl/src/inference/validation/stage.rs, line 152:

<comment>The per-upload budget is consumed in shadow mode too (exact counts still run to record violations there), so a file with >1000 near-limit lines stops producing shadow violation metrics past the budget — silently under-reporting exactly the signal the shadow rollout phase uses to judge enforcement impact. If tokenizer load capping is the goal, consider not debiting the budget when the context rule is only in shadow; if the cap must apply regardless, document the shadow-mode measurement bias.</comment>

<file context>
@@ -114,6 +149,7 @@ impl ValidationStage {
         let decided = rules::first_enforced(&evaluation.violations, &self.config).is_some();
         if let (Some(needed), Some(exact), Some(alias), false) = (&evaluation.exact_count, &self.exact, view.model.as_deref(), decided)
             && self.config.mode(RuleId::ContextLengthExceeded) != RuleMode::Off
+            && budget.is_none_or(ExactCountBudget::try_take)
             && let ExactOutcome::Counted(tokens) = exact.prompt_tokens(alias, surface, body).await
             && let Some(violation) = rules::exact_count_violation(surface, alias, tokens, needed)
</file context>

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Keeping the budget in shadow mode on purpose: its job is to cap tokenizer load per upload, and shadow mode does the same tokenizer work. Documented the measurement bias in f6c1e29 (ValidationConfig::exact_count_max_per_batch_file and config.yaml): past the budget, shadow metrics under-count context violations for that file.

This branch has not been deployed

No deployments
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