feat: per-request request_id/token_range pipeline (generate -> native -> host -> ClickHouse) - #19
Merged
Conversation
- Add task size support on BackendFuture and submit as budget in submit to host engine.
padded tokens. - DMX host engine shard_rank support for TP. - DMX host engine batch to request breaking. - DMX host engine support for dropping padded tokens. - NOTE: Still require changes from monitoring engine for submit API change. Need to track per request id, track effective start_token_idx and end_token_idx.
…update - dmx_host_utils: fix std::move bug in hook loop (use const ref params) - dmx_host_utils: add inner/outer length validation - dmx_host_engine: clean up submit() signature to use const refs - future_process: fix variable name error (token_end/start_idx) - future_process: add tensor.size(0) vs request_ids.size() validation - future_process: per-request select+narrow+clone, skip empty ranges - future_process: special handling for attn hook token/key dim narrowing - future_process: remove unsqueeze(0) to preserve correct tensor shape - clickhouse_client: update schema comment to 8-column with shard_rank - bindings: expose BackendFuture.size() - native_engine/hooks: fix BackendFuture task size type to pair<int64_t,int64_t>
- engine.py: replace single request_id with per-batch request_id list - engine.py: add _db_state_lock, _auto_batch_group_id, per-request start_idx and finished state - engine.py: shape-based prefill detection (input_ids.shape[1] > 1) as fallback for HF cache modes that pass non-None past_key_values - engine.py: EOS/PAD detection checks last_ids before advancing range (not after), fixing off-by-one row count for finished requests - engine.py: update submit() call to new C++ signature - hook_points.py: pass attention_mask to _register_db_step - generate.py: sync shape-based prefill detection in monitored_forward
- validate_request_id_pipeline.py: E2E test via full DB pipeline
- validates per-request token ranges are contiguous and correct
- validates prefill length matches attention_mask effective length
- validates row count matches effective generated length (EOS-aware)
- validates tensor shape rank for final_logits and attn hooks
- --with-attn-hook: exercises is_attn narrowing path in future_process
- --exercise-eos-path: forces early EOS via LogitsProcessor to validate
finished-request row count
- prompts_varlen_validation.txt: variable-length prompts for padding test
- test_monitoring_engine_request_id.py: unit tests for engine state logic
Collaborator
Author
|
(proj-dmx) nengneng@FROOT-Lab:~/AIPrometheus/HF_Prometheus$ ./tests/run_request_id_tests.sh ========================================
|
Collaborator
Author
|
Follow-up update: removed Reason: in the current runtime model, we do not have multiple workers concurrently mutating the same Python engine instance (single inference owner per engine / per-rank engine separation), so this lock is non-essential for current behavior and only adds complexity. The previously lock-protected fields were:
|
zaoxing
added a commit
that referenced
this pull request
Sep 6, 2026
DMI-vLLM-Integration PR #21 merged (squash 23717cb): attach_model accepts layers=LayerSelection(...), applies it to the local specs and the model-wide candidate-rank sets, and stays importable on DMI builds without the layer-range facade. The pin moves old-main 29f26c3 -> 23717cb, picking up both the V2-runner promotion (#19) and the layer range (#21).
zaoxing
added a commit
that referenced
this pull request
Sep 7, 2026
DMI-vLLM-Integration PR #21 merged (squash 23717cb): attach_model accepts layers=LayerSelection(...), applies it to the local specs and the model-wide candidate-rank sets, and stays importable on DMI builds without the layer-range facade. The pin moves old-main 29f26c3 -> 23717cb, picking up both the V2-runner promotion (#19) and the layer range (#21).
zaoxing
added a commit
that referenced
this pull request
Sep 7, 2026
DMI-vLLM-Integration PR #21 merged (squash 23717cb): attach_model accepts layers=LayerSelection(...), applies it to the local specs and the model-wide candidate-rank sets, and stays importable on DMI builds without the layer-range facade. The pin moves old-main 29f26c3 -> 23717cb, picking up both the V2-runner promotion (#19) and the layer range (#21).
zaoxing
added a commit
that referenced
this pull request
Sep 22, 2026
* [Docs] Add DMI-configurator design plan
Design doc for DMI-configurator: a configuration tool that turns a model
descriptor plus visual selections into validated DMI runtime YAML.
YAML does not replace the existing configuration mechanisms. It is added in
front of them via a loader/compiler pair:
config.yaml -> load_config() -> DMIConfig -> compile_config()
-> CompiledDMIConfig -> existing DMI runtime
load_config() knows nothing about the runtime; compile_config() knows nothing
about YAML.
Includes an appendix verifying the design against the current code
(CaptureSchedule, the hook catalog, HookSpec.layer_no, select_hook_specs, and
the attach_model integration point), and the open schema decisions that must be
settled before Phase 1.
* [Feat] DMI-configurator: structured configuration, YAML, and web UI
Implements phases 1-7 of docs/dmi-configurator-plan.md. YAML does not replace
DMI's existing configuration mechanisms; it sits in front of them:
config.yaml -> load_config() -> DMIConfig -> compile_config()
-> CompiledDMIConfig -> existing DMI runtime
load_config() knows nothing about the runtime; compile_config() knows nothing
about YAML.
New package dmi.configuration:
schema.py DMIConfig, ObservationConfig, LayerSelection, descriptors
manifest.py descriptor load/validate; the only place HF naming meets
ModelShapeConfig naming
catalog_adapter.py projects HOOK_DEFS into labels, UI groups, availability
architecture.py structural metadata driving the diagram
validation.py issues addressed to the control that caused them
yaml.py canonical serialization; parse(dump(c)) == normalize(c)
compatibility.py to/from the existing comma-separated selection string
compiler.py DMIConfig + ModelContext -> CompiledDMIConfig
The hook catalog stays authoritative: availability mirrors the suppression
already in select_hook_specs rather than restating it, and every catalog hook
is reachable from the UI even if it has no presentation entry.
Layer selection is structured, never encoded into hook syntax. It compiles to
a new additive spec filter, dmi.hooks.selection.filter_by_layers, following the
same convention as filter_by_pp_rank/filter_by_tp_rank. Global hooks
(layer_no == -1) are never dropped by a range.
dmi.ui serves a localhost FastAPI app over vanilla HTML/CSS/JS/SVG -- no React,
no Node build. Every endpoint delegates to dmi.configuration, so UI validity is
DMI validity. The browser gets no filesystem access: Save writes only to the
server-side path named on the command line.
dmi ui MODEL_DESCRIPTOR [--config CONFIG] [--host] [--port]
The web dependencies are an optional [ui] extra; a normal install pulls in no
web framework.
Runtime policy (phase 8) is carried and serialized but not connected to runtime
behaviour, and the UI says so explicitly rather than implying an effect it does
not have.
Tests: 141 new CPU tests covering descriptors, schema, validation, YAML
round-tripping against golden files, the legacy bridge, compilation, and the
HTTP contract.
* [Feat] Derive model descriptors from framework configs
Descriptors were hand-typed, which was both tedious and unsafe: a wrong
num_layers lets the UI offer layers the model does not have, validation passes,
and filter_by_layers then silently keeps fewer layers than requested. The
framework already knows the model, so read it from there.
dmi.configuration.introspect derives a descriptor from a Hugging-Face-shaped
config. It reuses make_model_shape_from_hf_config, which already extracts 8 of
the 9 topology fields, and adds the layer count (num_hidden_layers / n_layer /
num_layers) plus identity. Reading a config.json needs no transformers install:
the extractor uses getattr, so parsed JSON in a namespace works. transformers
is imported lazily and only to resolve a bare model id.
Two entry points:
dmi describe-model ./Qwen3-8B --output qwen3-8b.yaml
dmi ui ./Qwen3-8B # also config.json, a model id, or a descriptor
dmi ui now resolves any of those sources, so a descriptor file is an override
rather than a required input. Encoder-decoder configs are refused rather than
mis-rendered as decoders.
A descriptor file still earns its place: the configurator runs where the model
is not loaded, so it stays the portable design-time record. The runtime never
reads it -- compile_config takes its shape from the adapter's live
detect_model_shape(model).
Examples: qwen3-8b.yaml and qwen3-30b-a3b.yaml carried numbers I could not
verify and are gone. The MoE case moves to tests/data/moe-decoder.model.yaml,
labelled synthetic rather than claiming to be a real model. llama3-8b.yaml
stays as the one worked example, and a test now pins it against what
describe-model produces for a real Llama 3 config.
Tests: 23 new CPU tests covering extraction across HF naming variants
(llama/gpt2), MoE fields, head_dim emission, identity from directory vs
config.json, and every rejection path.
* [Fix] Make configuration parsing strict and CI actually run the suite
Five review findings, all about failures that were silent.
CI was red on every PR touching this feature. The workflow installs
`--no-deps -e .` plus a hand-picked list, so requirements.txt is never
consulted and pyyaml was absent: nine test modules failed to collect and
`make check` exited 2. Adding pyyaml fixes it. Verified by blocking every
non-CI dependency locally, which reproduces the failure exactly, then
restoring only pyyaml.
The same workflow never installed the [ui] extra, so
tests/test_configurator_api.py skipped itself rather than failing -- all 41
tests of the HTTP contract, the whole backend, never ran on any PR. fastapi,
uvicorn and httpx are now installed so they do.
Parsing was strict for `schedule` and lax everywhere else, which made a typo
dangerous rather than loud: `observations: {layer: ...}` (missing the plural)
parsed as "no layer range", `_validate_layers` returned early because layers
was None, and a capture meant for 8 layers of a 32-layer model ran on all 32
with no issue reported at any severity. Every section now refuses unknown
keys through one helper.
`version` was defaulted to the current version when absent, which is the
guess that version dispatch exists to avoid -- an unversioned v2 file written
by a newer tool would have been read as v1. It is now required.
A YAML path that does not exist fell through to Hugging Face id resolution
and blamed a missing `transformers` install for what is almost always a shell
typo. A .yaml/.yml suffix is a file path, so a missing one now says so.
The "Other observations" fallback node hardcoded layer scope, so an
unassigned *global* hook would have been drawn inside the "Transformer layer
x N" group -- implying per-layer capture and that the layer range applied to
it. Leftovers now split by scope. The two new scope tests fail against the
old code, confirming they are not vacuous.
* [Fix] Address review comments on the configurator
Three items from the PR review.
The typed-schema and compilation snippets in the plan still showed a
`runtime: RuntimeConfig` field on DMIConfig and CompiledDMIConfig. The
implementation deliberately has no runtime block -- the appendix says so and
explains why -- so the snippets were telling readers the opposite of the
decision. The appendix note about RuntimeConfig overlapping RingConfig stays;
it is the record of that decision, not a stale snippet.
python-multipart is dropped from the [ui] extra. The backend accepts JSON
only: the browser reads a configuration file itself and posts its contents,
so there is no multipart/form-data surface. Verified no UploadFile or Form
usage anywhere under dmi/ui, and the API suite passes without it installed.
A non-integer `version` is now a malformed document rather than an
unsupported one. YAML hands back the string "1" for a quoted value, and
"version '1' is not supported by this build" would send the reader looking
for a build that reads it instead of at the quotes.
* DMI-configurator: apply layer ranges at runtime, estimate payload cost, and make it launchable (#121)
* [Docs] Review the optimization policy capture plan against current runtime
Holds the proposed policy/resolver design against the code on main. Records
three blocking findings: CaptureSchedule is defined and exported but no adapter
enforces it, the transport already blocks losslessly in prepare_step and has no
drop path (inverting the proposal's pressure assumptions), and per-layer
capture selection does not exist. Adds six design-level findings and a
dependency-ordered resequencing of the delivery phases.
* [Feat] Apply configurator layer ranges and estimate payload cost
Two gaps in the configurator, both about the UI telling the truth.
Layer selection was authored but never applied. attach_model had no way to
receive a range, so a range picked in the UI changed nothing at runtime --
the same defect class as the policy control, except that one is disclaimed
and this one was not. attach_model now takes a keyword `layers` argument and
applies filter_by_layers between apply_hook_selection and the PP/TP filters,
driven from a DMIConfig by configuration.attach_config.
The parameter is the range rather than a CompiledDMIConfig as the original
design assumed. Handing attach_model compile_config's spec list would leave
every deselected hook live: HookPoint.enabled defaults to True and only
apply_hook_selection walks the unselected specs to turn them off, so
attach_model has to own selection. Keeping the parameter primitive also keeps
configuration depending on the runtime rather than the reverse.
Payload estimation was out of scope for the MVP but needs no policy resolver,
so it lands now as configuration.estimate plus a UI panel. It reuses
compute_hook_shape and plan_step's align_up(_, 16) rounding, so the figure
shown and the figure prepare_step enforces cannot drift. It reports the peak
single step against min(payload, pinned) -- the constraint that actually
binds, since exceeding it silently drops the adapter to eager CPU-direct
dispatch -- and names the worst rank, because sharded hooks divide by tp_size
while unsharded hooks sit on rank 0 alone and PP_FIRST/PP_LAST skew the
stages. Workload and ring sizes stay UI-local and out of the YAML, keeping
DMIConfig free of a runtime block.
Also: visible focus rings (the architecture nodes carry tabindex but had no
focus style), focus restored across the SVG re-render that node activation
triggers, and a prefers-reduced-motion guard.
65 new CPU tests. The 5 pre-existing fsync failures in tests/test_capture_*
are untouched by this branch.
* [Feat] Make the configurator easy to launch
Running it took a PYTHONPATH prefix, a module path, and a descriptor path you
had to know, then opening a browser by hand. Now:
make ui
from a checkout, with nothing installed. Override with MODEL=./my-model.
- MODEL is optional. With none given, dmi ui uses the descriptor in the
current directory when there is exactly one. Several, or none, and it lists
what it found or points at describe-model rather than guessing.
- The browser opens once the port actually accepts a connection, polled rather
than slept, so a slow first import does not open a dead page. --no-browser
opts out.
- The default port walks forward to the next free one, so a second
configurator does not die on address-in-use. An explicit --port is never
moved: a named port that is busy should fail loudly.
- The startup banner is flushed, since it carries the URL and a piped or
make-wrapped stdout otherwise held it until exit. It now also reports the
model geometry.
- README documents the configurator for the first time.
The repo-specific default model lives in the Makefile, not in descriptor
discovery: the shipped CLI stays generic, and the example descriptor does not
match the *.model.yaml convention anyway.
30 new CPU tests. The port and browser tests drive the logic through injected
seams rather than binding real sockets, which a restricted CI refuses; real
socket behaviour was verified separately.
* [Test] Cover the architecture diagram's unassigned-hook fallback
`architecture_payload` documents that "extending HOOK_DEFS can never make
an observation unreachable from the UI". That guarantee rests entirely on
the trailing "Other observations" node, and nothing exercised it: every
catalog hook is currently assigned to a real block, so the fallback branch
never ran under test and could rot until the day someone adds a hook.
Adds two tests -- every catalog hook is reachable from some node, and a
hook with no assigned block lands in "Other observations". Removing the
`leftovers` branch fails the second one.
* [Fix] Reject a model id that is not a single path segment
`model.id` becomes a filename stem: the configurator writes
`<id>.dmi.yaml` beside the model, and `dmi.ui.app` documents that the
browser cannot cause a write outside the directory the user named on the
command line.
Descriptors derived from a framework config are already safe -- `_slug`
reduces `Qwen/Qwen3-8B` to `qwen3-8b`. A hand-written descriptor YAML is a
documented input too, and reached the same code unchecked: `id:
meta-llama/Llama-3-8B` resolved into a non-existent subdirectory (save
fails with a 500), and `id: ../../etc/pwned` resolved outside the base
entirely, quietly breaking the guarantee above.
Validates in `ModelIdentity.__post_init__` rather than at the save site, so
every consumer is covered. `descriptor_from_hf_config` now wraps the
resulting ValueError as `DescriptorError` for consistency with the rest of
that path.
* [Fix] Address deep-review findings across the configurator
Runtime wiring:
- HuggingFaceAdapter.attach_model now accepts and forwards the layers
keyword, so attach_config no longer raises TypeError on the only
shipped concrete adapter. Covered by tests that drive the real HF
adapter instead of a BackendAdapter stub.
- compile_config filters layers with the pure hook_belongs_to_layers
predicate instead of filter_by_layers, which disabled HookPoints on a
live model and raised on unbound authoring-time specs.
Estimation:
- bytes_per_request divides the whole-batch step total by batch_size in
both tensor conventions; batched shapes carry the leading batch
dimension too, so HF-mode figures were batch_size times too high.
- aggregate_prefill_step_bytes renamed to aggregate_peak_step_bytes: it
sums each rank's peak over the enabled phases, which is a decode
figure whenever prefill capture is off.
UI server:
- Relaunching without --config reloads the default save path instead of
starting blank and letting Save clobber the authored file.
- Loopback binds reject non-loopback Host headers, closing the DNS
rebinding path to the file-writing save endpoint.
- The browser opener uses a wall-clock deadline; the old loop counted
iterations while sleeping twice per step, stretching the stated
timeout to roughly double, and its Event was never set.
UI client:
- The decode rate field parses as a float, so 0.5 steps/s no longer
truncates to 0 and silently drops the sustained estimate.
- The Middle layer preset clamps its start, so 1-2 layer models cannot
produce an inverted range the server rejects.
Imports:
- dmi.hooks resolves its re-exports lazily and introspect defers the
model-shape extractor import, keeping torch off the descriptor,
validation, and CLI paths as designed.
Claude-Session: https://claude.ai/code/session_01J4TK3xEGqqjJuum1GRn6Ns
* [Fix] Report the decode step from the rank the estimate names
decode_step_bytes took a maximum across every rank while peak_step_rank and
bytes_per_request came from the rank selected by peak step. Under pipeline
parallelism those are not the same stage -- a first stage carrying many
layers versus a last stage carrying final_logits -- so one result could
describe two different ranks, and the per-request figure could be computed
from a rank the report never named.
All three now come from the worst rank.
* [Fix] Restore estimate request sequencing and let the rate field hold a float
Two fixes that went missing when the concurrent deep-review commit superseded
an overlapping local one.
Estimate responses are applied in order again. The endpoint walks every rank,
so a slow earlier request could resolve after a newer one and paint figures
for a workload the user had already changed away from. Requests are stamped
and stale replies dropped. Verified by delaying the first request behind the
second: the newer figures survive.
The decode-rate field kept `step="1"` while its handler had moved to
parseFloat, so the JS fix could not take effect: a number input with an
integer step treats 0.5 as invalid, `value` reads back empty, parseFloat
returns NaN and the handler bails, leaving the rate at 0 and the sustained
estimate blank. With step="any" the field validates and 0.5 steps/s now
yields 1.00 MiB/s and 84.4 GiB/day.
* [Fix] Address review comments on the follow-up branch
attach_config no longer passes layers= unconditionally. Adapters are a public
extension point, and one that overrides attach_model without the keyword --
as the shipped Hugging Face adapter did until this branch fixed it -- raised
TypeError for every configuration, including the majority that set no layer
range and need nothing from the argument. The keyword is now passed only when
a range is present. A config that does set one still fails loudly on such an
adapter, which is correct: the range would not be honoured, and silently
dropping it is what this wiring exists to prevent.
The estimate endpoint validates the ring block. A truthy non-mapping (a list,
a string, a bare number) reached .get and surfaced as a 500 AttributeError
rather than a 400, and payload_bytes was tested for truthiness, so 0 silently
skipped the ring-fit result the caller asked for instead of being rejected.
The type check now runs on the raw value, before the `or {}` coercion that
was hiding the falsy cases.
The README and plan claimed `make ui` runs "with nothing installed". The
server hard-requires fastapi and uvicorn and raises RuntimeError without
them; only DMI itself is optional. Both now say so.
Tests: the zero-ring case uses 0 rather than -5, which is what actually
exercised the truthiness bug, with the negative case kept separately and the
non-mapping cases parametrized.
* [Docs] Drop a duplicated install line from the configurator plan
The install command now appears once, up front, where it belongs -- the
"installed" variant only needs to show the command that differs.
* [Fix] Stop the estimate promising a sampling reduction nothing delivers
The payload estimate divided its per-request and sustained figures by
step_stride, and applied request_stride as an assumption, but no shipped
adapter enforces CaptureSchedule: should_capture_step and
should_capture_request have no callers outside dmi.config, and nothing in
dmi.adapters reads CompiledDMIConfig.schedule. So the UI quantified a
reduction the capture never performs. Someone setting stride=4 and sizing
storage from the panel under-provisioned by 4x; at stride=1000 the figure was
a thousandth of reality.
This was worse than the plain no-op it replaced. Before the estimator existed
the unenforced schedule was merely invisible; with it, the UI put a confident
wrong number on screen.
The estimate now reports the full volume and says why, and the Capture panel
carries the same kind of disclaimer the System objective control already had
-- that one disclaimed only the policy radios, leaving the stride and warmup
fields looking functional. Prefill and decode toggles are unaffected: they
gate which shapes fire, which the estimator models directly.
Separately, a layer range outside the model resolved to an empty set and
collapsed per-layer hooks to zero bytes with nothing said. Zero reads as
"free" rather than "selects nothing", so an empty selection and a clipped
range now both explain themselves.
When schedule enforcement lands, apply the stride again and drop the warning;
they are commented as a pair so the next reader finds both.
Reproduced first in tests/test_estimate_runtime_fidelity.py: 9 of its 14
tests failed against the old code, the other 5 being negative controls that
passed throughout. One existing test in test_configuration_estimate.py
asserted the buggy division and is corrected in place, with the reason
recorded in its docstring rather than silently rewritten.
Verified in the browser as well: stride 1, 4 and 1000 now all report
20.0 MiB/s and 1.65 TiB/day, the warning names the offending stride, an
out-of-range selection explains its 0 B, and a clipped range reports
"2 of 21 requested layers exist". No console errors.
* [Fix] Drop stale responses in refreshOutput too
refreshOutput fired two requests per edit and rendered whatever resolved,
with no way to tell a late reply from the current one. Under a slow response
the YAML preview and Issues tab could end up describing a configuration the
user had already changed away from. refreshEstimate got this guard earlier;
this is the same fix on the other render path, and a stale return also skips
the trailing refreshEstimate call, since the newer refreshOutput makes it.
Reproduced in a real browser before fixing: with the first request delayed
1400ms behind two later edits, the stride input read 99 while the preview
showed step_stride: 1 -- a response two edits old painting last. After the
fix the same harness reports 99, and a trace confirms the late reply resolves
and renders nothing.
The harness was then shown to discriminate: with the guard removed the same
script on an equally settled page reproduces the stale paint, and with it
restored it passes. That check mattered, because an earlier "green" run of
mine was invalid -- it executed against the pre-reload page, so it exercised
the unfixed code and reported a failure that was an artifact of the harness
rather than the fix.
tests/test_ui_client_invariants.py adds committed guards for both render
paths. They assert the mechanism is present rather than that it works, which
is weaker than a unit test and deliberately so: the plan gates the static UI
on "no build process" and rules out a Node toolchain, so there is no JS
runner in CI to execute app.js against. The file says as much, and says to
replace them if a runner ever lands. A coverage test derives which paths need
a guard from whether they can overlap -- scheduled or called directly -- so
boot is exempt because it is only ever registered on DOMContentLoaded, and a
new fetch-and-render path cannot be added unguarded without failing.
---------
Co-authored-by: Claude <noreply@anthropic.com>
* [Fix] Address Copilot review: strict section parsing, save error translation, doc snippet
* Close all 14 review findings on the configurator
Blocking:
- Non-loopback binds now mint a per-launch token demanded on every mutating
endpoint (the file-writing API is no longer open to the network).
- Save refreshes the server's reload state, so a refresh restores what was
saved and a second save cannot overwrite it with the launch config.
- Save runs model-aware validation before the write; invalid configurations
get a 400 naming the issues instead of a successful persist.
- The capture schedule is enforced: the adapter driver consults
should_capture_step/should_capture_request before every step (StepContext
carries the phase; HF reports it), so capture_decode=false actually stops
decode capture and strides thin the capture.
- compile_config refuses requested hook types absent from the model's spec
list, so "valid" means executable (pos_embed/mlp_post on a Llama can no
longer compile to an empty selection).
- attach_config diagnoses an adapter that cannot accept `layers` with a
ConfigurationError naming the remedy (pinned vLLM integration update is
a separate-repo follow-up).
Major:
- YAML boundary is type-strict: exact ints (no 2.9-truncation, no bools),
real booleans (no "false" strings), unknown keys in layers/policy refused.
- Descriptor topology requires exact positive integers; head_dim is
validated; num_layers 1.5 is refused at the boundary instead of exploding
in range().
- Introspect refuses known non-decoder model families (BERT, ViT, T5, ...)
instead of labeling every non-encoder-decoder config decoder_transformer.
- The frontend preserves policy absence when loading policy-less YAML.
- Copy serializes the current state instead of the possibly-stale preview.
- Estimator volume figures sum every rank (TP/PP splitting no longer deflates
bytes_per_request); peak stays per-rank for ring sizing.
- Estimator dtype accounting matches the runtime: packed token/topk ids are
int32, topk weights float32; batched token ids int64.
- Workload gains cache_max_len so StaticCache prefill attention can be
sized; without it the estimate says which cache it assumes.
CPU suite: 1373 passed.
* [Fix] Skip API suite without an HTTP transport; flag layer ranges on Packed/vLLM
Copilot's third pass on PR 122 noted that fastapi.testclient needs httpx,
which the [ui] extra does not carry. With Starlette 1.6 the import raises
RuntimeError (it wants httpx2 now) at collection, so the whole API contract
suite errored instead of skipping like every other optional-dependency suite.
Both TestClient-using modules now skip unless httpx2 or httpx is importable;
the transport stays out of [ui] on purpose (the UI never makes HTTP requests)
and pyproject says so. A child-process test pins the skip-not-error behaviour.
Review finding 3919326699: the pinned vLLM integration's attach_model has no
`layers` keyword, so attach_config refuses a ranged configuration on that
backend at launch. The estimator -- where the UI learns which backend is
targeted -- now warns about this on Packed (vLLM) with a layer range, and the
attach_config error links the tracking issue
(ProjectDMX/DMI-vLLM-Integration#20).
CPU suite: 1372 passed; the 5 remaining failures are the /proc/self/fd fsync
tests, which are Linux-only and identical to main.
* Fix the schedule-gate hole the review found, plus three minor findings
The high-severity one: refusing a step in before_forward only skipped
plan/commit -- the model's HookPoints still dispatched producers during that
step's forward, writing unreserved bytes into the ring and desyncing the
task/meta FIFO so later captured records were misattributed or dropped. The
driver now disarms the transport (capture_step=False) before refusing and
re-arms it on a committed step; HookPoint.forward checks the flag first and
returns without dispatching. CPU-level test drives a real HookPoint through
both flag states with the producer call recorded.
- Sustained rate and per-day volume now divide by request_stride as well
(the driver's request gate drops whole requests, matching per_request).
- The estimator states its enforcement assumption when strides or phase
flags are configured: an adapter reporting no phase / non-numeric request
ids applies only part of the schedule.
- Architecture refusal matches real HF spellings: "BertForMaskedLM"
(no underscores) now trips the masked-LM check; unknown-key TypeError
guard for head_dim covers bools exactly.
- Nits: constant-time token comparison in the UI middleware, isdecimal for
scheduler-id gating, and the never-fires compiler check now runs after
selection (shape suppression) but before layer filtering so its message
cannot blame the layer range for an absent hook.
CPU suite: 1377 passed.
* Land the schedule-gate producer fix that a stash had swept aside
Companion to 30f0e8a, which committed only four of the six files before a
concurrent stash took the rest of the working tree. This carries the core of
the fix: the transport gains a capture_step flag that the driver disarms
before refusing a schedule-refused step and re-arms on a committed one, and
HookPoint.forward checks it before dispatching -- so refused steps no longer
launch unreserved producers or desync the task/meta FIFO. Also lands here:
request_stride in the sustained rate, the estimator's enforcement
assumption, the BertForMaskedLM spelling fix, and the regression tests for
all of it.
CPU suite: 1385 passed.
* Export the layer-range API through the v1 integration facade
DMI-vLLM-Integration#21 accepts attach_model(..., layers=LayerSelection(...)),
and it imports everything through dmi.api.v1 — which did not carry the
layer-range pieces. filter_by_layers, hook_belongs_to_layers, and
LayerSelection are now facade exports alongside the rest of the selection
API, the exactness test and integration-api-v1.md cover them, and a pin
test locks the three to their real implementations.
The submodule pin bump and the estimator's vLLM layer-range warning
removal land together after DMI-vLLM-Integration#21 merges.
* Close the independent-review findings: HTTP hardening, atomic save, ring-fit task cap, gate disclosures
Eight independent reviewers audited this PR; every confirmed finding is
addressed here.
HTTP boundary (was raw 500s on malformed input):
- /api/config/parse rejects a non-string yaml value with 400.
- /api/estimate: Workload requires exact ints/finite floats (float 8192.0
exploded as TypeError in the byte arithmetic; bools read as 1), the ring
block requires integer payload/pinned bytes and OverflowError is caught,
and pinned_bytes without payload_bytes is a 400 instead of a silent
no-op. Non-ASCII X-DMI-Token headers compare as bytes (was TypeError ->
500) and /api/config is token-gated on network binds (it names a server
path).
- The loopback bind refuses mutating requests carrying a FOREIGN Origin:
TrustedHostMiddleware stops DNS rebinding, not plain CSRF -- a web page
can send Host: 127.0.0.1 legitimately, and sendBeacon can send
application/json without a preflight. Foreign Origin is the one signal
a browser always gives.
Data safety:
- save_config/save_descriptor write to a temp file and os.replace: a
mid-write ENOSPC used to destroy the previous configuration.
- The descriptor parser now matches the config parser's strictness:
exact-integer schema_version (true/1.0 are refused), unknown keys
refused in the model section and at the root, a bad model.id raises
DescriptorError instead of bare ValueError, and num_experts without
top_k is refused (the routing observations could never fire).
Estimator fidelity:
- check_ring_fit gains task_entries: prepare_step refuses on the task
ring too, so a wide full-preset model "fit" on bytes while every real
step returned OVERSIZED. /api/estimate accepts ring.task_entries and
names the actual breach in the detail text.
- The PP layer split now matches vLLM's get_pp_indices (remainder on the
FIRST stages), so the peak rank is the rank the runtime actually
pressures on non-divisible layer counts.
- Batched final_logits counts logits_to_keep=1 (what HF generate()
materializes) instead of every prefill row -- the old number was
~1000x the reality and set the ring-fit verdict.
Schedule gate:
- The no-phase tail (warmup/stride for adapters that report no phase) is
now pinned by tests, the docstring documents it and the request-gate
units, and an unrecognized phase value captures-as-unreported instead
of crashing the driver and silently disarming all later steps.
- catalog_adapter marks final_logits unavailable when vocab_size is 0
(compute_hook_shape returns [] for it; validation/estimate/compiler all
certified it before).
Frontend:
- Input handlers guard on an initialized flag: listeners bind before the
model fetch resolves, and a keystroke in that window crashed on null
state (and a mid-applyState crash discarded a loaded file).
- Copy carries a stale-response stamp like the other async paths.
- Tabs expose aria-selected and panels are role=tabpanel.
Docs: capture-policy-review's "schedule enforced by no shipped adapter"
finding is marked resolved (it landed in this PR after the doc was
written); the PR body no longer claims the change is additive-only nor
lists #121 as an open follow-up; the pyproject httpx comment matches
reality (it is a core dependency).
CPU suite: 1414 passed.
* Restore the clickhouse-live job the branch accidentally deleted
c00afa4, while adding pyyaml/fastapi to the CPU job's install, removed the
entire clickhouse-live job (its services, its fetch-depth: 0, its skip
gate) from python-checks.yml — so the manual ClickHouse suites this branch
carries ran on nobody's CI again, which is the exact regression that job
was created to prevent. The failure only surfaced once the configuration
pipeline started importing yaml at module level: the live job's install
never had pyyaml, and collection of the new suites died with
ModuleNotFoundError.
The job is restored from main verbatim, with pyyaml and the [ui] deps
added to its install so the configurator suites collect and run there too.
Verified locally against the CI selection: 30 collected, 0 skipped.
* Close the Samfisheryu review: one runtime-apply path, honest packed estimates, fail-closed detection
Every finding was reproduced as a test before any source changed
(tests/test_pr122_round3_review_findings.py). The review was written
against c19eba5; 3dc68ca had already closed three of the fifteen, and
those stay in the suite as regression pins rather than being dropped.
Blockers:
- attach_config now installs the configuration's CaptureSchedule on
adapter.engine.config, which is the only place _schedule_allows reads
it. Before, the documented path (load_config -> attach_config) applied
WHERE to capture and silently dropped WHEN: a YAML declaring
capture_prefill: false or step_stride: 17 attached hooks and captured
everything. An adapter with no engine still attaches under the default
schedule and raises for one that asks for anything, matching the rule
the `layers` keyword already follows.
- Attachment has one owner. BackendAdapter.attach_model records the
owning adapter on the model and HuggingFaceAdapter.detach_model
releases only its own marker; generate_with_monitoring (and the greedy
loop, same hazard) refuses a model somebody else attached instead of
building a second adapter over the shared transport -- which left the
forward with one caller's reservation and this call's producers, then
detached the first attachment on the way out.
- Packed estimates no longer divide by step_stride/request_stride. The
pinned vLLM integrations build MonitoringEngine(config=None) and call
commit_step directly, so no schedule reaches the gate and production
captures every step; dividing reported a reduction the shipped runtime
does not perform, on the UI's default convention. The figures now say
so. Batched (Hugging Face) still divides -- that driver does gate.
Correctness and honesty:
- compile_config refuses a layer range that removes every spec of a
selected hook, naming the live layer span. A stale descriptor claiming
32 layers against a model exposing 16 used to compile layers 20-25 to
an empty spec list that installed nothing and reported nothing.
- Architecture detection fails closed: an allowlist of causal decoder
families replaces the non-decoder blacklist, which could only refuse
what someone had thought to list (dinov2, modernbert and
bert-generation all escaped it and were emitted as
decoder_transformer). A config with no model_type is refused too --
geometry that parses says nothing about causality. Hand-written
descriptors remain the escape hatch and skip detection entirely.
- /api/validate and /api/config/save label their verdict design-time and
name the descriptor as its authority; the UI's "ready to run" wording
follows. Validation cannot certify runtime-readiness from a portable
descriptor: a decoder Llama exposes no pos_embed spec however the
descriptor marks it.
- A strided estimate is disclosed as a long-run average bounded by the
peak step, and offsets/warmups warn that they shift which steps are
captured rather than being silently absent from the arithmetic.
Boundaries:
- Saves serialize the write and the state update under one lock, so a
200 names one committed revision. Racing saves used to leave the file
and the served config disagreeing until restart.
- /api/config/parse and load_config share a loader that rejects
duplicate mapping keys: PyYAML's last-wins would let a merge conflict
change capture scope in silence.
- observations.hooks is a typed list[str] -- [q, 1, true, null] no
longer becomes ["q","1","True","None"].
- Workload requires a real bool for packed ("false" was truthy and read
as the vLLM convention) and a non-bool number for the decode rate.
- An explicit --port outside 1-65535 is refused before the descriptor is
read; port 0 used to advertise a URL naming no bound port.
Two findings are answered with disclosure rather than the reviewer's
first option, both stated in the estimate output: VLLM_PP_LAYER_PARTITION
on the serving host overrides the partition this estimator cannot see,
and the per-request figure is exposed as a long-run average instead of
evaluating the predicates for a stated request window.
Existing tests that pinned stride division on packed workloads were
split by convention rather than deleted, and the packed counter-case is
pinned alongside them.
CPU suite: 1473 passed. The 5 remaining failures are pre-existing and
Linux-only (capture-storage fsync tests read /proc/self/fd), verified
failing identically on a pristine 3dc68ca.
* Close the remaining review findings: compile disclosure, CLI error narrowing, selection taxonomy
- A non-default capture schedule now disables CUDA-graph compilation for
the generate() call, mirroring the capacity-overflow path: the gate is a
Python branch in HookPoint.forward and replay bakes it at the first
decode trace, so a refusal (or capture) at step one would apply to every
later step. Default schedules are unaffected. Pinned by driving
generate_with_monitoring with a stubbed adapter and inspecting the
kwargs the model receives.
- cli.main no longer catches bare RuntimeError -- that class carries
genuine bugs (RecursionError is one) and swallowed them into one stderr
line. The optional-dependency failure gets its own UIDependencyError;
internal raise sites updated.
- compile_config re-raises an unknown hook selection as
ConfigurationError instead of bare ValueError, so callers funneling on
the taxonomy (the UI's 400 mapping) see a 400, not a 500.
CPU suite: 1588 passed (after the main merge).
* Close the round-4 follow-up findings: packed honesty, PP rule, ownership, boundaries
Samfisheryu's follow-up review kept CHANGES_REQUESTED on eight residuals;
all are fixed here with tests, RED first.
Ownership:
- attach_config refuses a second adapter on an already-owned model BEFORE
any mutation (no schedule install, no attach call), naming the current
owner; same-adapter reconfiguration is allowed; detach by a non-owner
leaves the marker (HF adapter path pinned).
Packed honesty (the vLLM runtime executes no schedule):
- capture_prefill/capture_decode no longer shrink packed volume, peak, or
sustained figures -- the runtime captures both phases regardless. The
authored intent stays visible as an explicit warning; batched figures
are unchanged.
- vLLM PP partition rule ported from 0.27.1's get_pp_indices (remainder
backward from the second-to-last stage, never the last; cited cases
[1,2,1], [2,3,3,2], 32/40 verified) plus an explicit, validated
pp_layer_counts override for VLLM_PP_LAYER_PARTITION deployments.
- Offset/warmup disclosure no longer depends on a stride being set.
Boundaries:
- Ring: only absent/None defaults pinned_bytes; every explicit value is
validated (exact non-bool int, >= 0), and check_ring_fit refuses
negatives itself.
- Workload.cache_max_len requires an exact integer.
- YAML: unhashable mapping keys raise ConstructorError (a 400), and the
descriptor file path uses the same strict loader as configs.
- Introspect: any model_type containing "encoder" fails closed instead of
riding a decoder family prefix (qwen2_audio_encoder); decoder+head
configs still accepted.
CPU suite: 1679 passed.
* Disclose the two known ceilings the follow-up review named
- The per-request token-range index records every driven step including
schedule-refused ones; it is the attempted-traffic index, not the
captured-traffic index.
- The _accepts_layers probe cannot distinguish forwarding **kwargs from
tolerating them; the backstop is the v1 contract plus compile-time
live-hook rejection, stated where the probe lives.
* Add Workload.sliding_window for sliding-window attention estimates
Attention-weight hooks (pattern, attn_scores) attend at most window KV
positions on sliding-window models, so they are shaped against
min(context, window) instead of the full context. Only kv_dim is
affected, and only those shapes consume it. Exact-int >= 1 validation,
assumption text when set, no-op when absent or above context.
FP8 KV-cache quantization was investigated and deliberately not modeled:
hooks capture forward activations in model dtype, so cache-store
quantization changes zero hook payload bytes.
* Revert "Add Workload.sliding_window for sliding-window attention estimates"
The knob's core semantic was incorrect in the unsafe direction:
compute_hook_shape returns full [heads, q_len, kv_dim] for
pattern/attn_scores with no window input, plan_step reserves exactly
that, and these hooks fire meaningfully only in eager mode -- where SDPA
materializes the full scores matrix and masks it rather than shrinking
it. Capping kv_dim by the window made the estimate report less than the
runtime reserves (up to context/window under-count), and check_ring_fit
would bless a ring the runtime overflows. The FP8 non-modeling analysis
in the original message stands; the window half is withdrawn until a
backend materializes band-shaped scores.
* Frontend review round: guards, keyboard paths, live regions, curl-only banner
- Save disables itself for the request round trip (re-armed in finally).
- Layer rail ticks are keyboard-operable checkboxes sharing one picking
function with the click path; architecture nodes already had this.
- Estimate panel marks itself loading with aria-busy, cleared on either
guarded outcome.
- Status chip is a polite live region; ring meter exposes value semantics.
- Token-gated binds get a persistent curl-only banner revealed on 401
instead of per-panel curl-hint errors.
* Simplify the config / descriptor write helpers
- _reject_unknown gains an error= kwarg so the config half (ConfigurationError)
and the descriptor half (DescriptorError) share one body. As a bonus
this stops the int-YAML-key AttributeError the descriptor half had -- the
manifest copy already used repr() but the config half still did not.
- save_config and save_descriptor share _write_text_atomic(path, text, what,
error): the only delta is which exception class wraps an OSError.
CPU suite: 1685 passed.
* Record the configurability boundary on PR #122
The review round surfaced three rules that deserve a written rationale
rather than a thread of replies: the configuration artifact is strict
end-to-end; the runtime surface is enforced at the smallest possible
layer (the adapter driver's before_forward) rather than scattered
across every emit point; and the HTTP surface trusts the loopback bind
but refuses the network without a per-launch X-DMI-Token. The
one-owner attach invariant is the same rule applied to a model.
The alternatives weighed (stricter loopback defaults, TLS/OAuth on the
local tool, rejecting packed configs that name a schedule, hooking the
gate at every emit point, an inline JSON or frontmatter idiom) all
get a sentence here so the next round does not relitigate them.
* Finish the shrink round: UI half and matching tests
The yaml.py/manifest.py simplification shipped first without its test
updates, which broke CI: the old atomic-save test raised ENOSPC from
dump_config, and the new save_config evaluates dump_config before
_write_text_atomic's try-block, so the OSError escaped unwrapped. The
test now patches Path.write_text on the .tmp sibling instead, which is
the real I/O boundary the atomic write protects.
Rest of the round:
- UI_DEPENDENCY_MESSAGE moves to errors.py beside UIDependencyError, so
app.py and server.py quote one string.
- The auth banner reuses .notice plus one margin-top override instead
of duplicating seven properties.
- Copy calls a shared serializeCurrentState() rather than reissuing the
/api/config/serialize POST shape refreshOutput already builds.
- The two test-side brace matchers (_function_body /
_plain_function_body) collapse into one with an optional async prefix.
CPU suite: 1685 passed.
* Pin third_party/vllm-integration to main (layer-range attach)
DMI-vLLM-Integration PR #21 merged (squash 23717cb): attach_model
accepts layers=LayerSelection(...), applies it to the local specs and
the model-wide candidate-rank sets, and stays importable on DMI builds
without the layer-range facade. The pin moves old-main 29f26c3 ->
23717cb, picking up both the V2-runner promotion (#19) and the layer
range (#21).
* Drop the packed-backend layer-range refusal warning
The warning told the UI that attach_config refuses a ranged
configuration on Packed (vLLM) until the integration accepted ranges --
that is PR #21, now merged and pinned. The estimator test flips to pin
the absence of the warning (RED against the stale block), and
attach_config's fail-closed guard stays for genuinely older
integration checkouts, with its comment updated to name them.
CPU suite: 1689 passed.
* Document the live verification of the configurator runtime attach on vLLM
The recipe for proving a saved .dmi.yaml artifact drives a real vLLM
0.27.1 model through dmi.configuration.attach_config existed only in
the verification session: build the CI-style env (vllm 0.27.1 + DMI
--no-deps + integration --no-deps), subclass the V1 DMXGPUWorker to
suppress the self-attach (V2 refuses subclasses), run the engine core
in-process, and assert ownership, ranged spec layers, disabled
out-of-range hook points, and a completed generate. Also correct the
install step for shared checkouts: the native build emits ABI-suffixed
extensions per interpreter, so 'make clean' is destructive there.
* Add hf extra for transformers-backed model resolution
dmi ui/describe-model needed transformers installed by hand to resolve a
model directory or bare HF model id; pip install -e ".[hf]" makes that
reproducible, and the missing-transformers error now names it.
* Mention the hf extra in the README configurator section
Keep the top-level entry point consistent with the configurator plan doc.
* fix: count skipped metadata steps in capture schedules
* Drop the .loop scratch state from the tree
The /polish loop's working files are per-run agent bookkeeping, not project
artifacts: nothing in the tree, the build, or CI reads them, and they go
stale once the branch they describe has landed. #133 removes them from
main; dropping them here too so a merge of this branch cannot carry them
back in.
---------
Co-authored-by: Claude <noreply@anthropic.com>
Co-authored-by: Alan Liu <zaoxing@users.noreply.github.com>
Co-authored-by: Samfisheryu <ynnpersonal@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Background
This PR finalizes per-request tracking in the monitoring pipeline:
HF generate() -> monitoring engine -> native backend -> host engine -> ClickHouse.Main objective:
v0_host_sideWhat changed
1) Python-side per-request tracking (monitoring engine)
Files:
monitoring/engine.pymonitoring/hook_points.pymonitoring/generate.pyChanges:
_register_db_step(...)now uses:request_idsper active batchstart/endtoken range per requestattention_maskto compute prefill lengths(start, start)and stops growthpast_key_values is Noneinput_ids.shape[1] > 1(for cache-mode edge cases)(model_id, shard_rank, request_ids, token_ranges, cache_dict)2) Native interface alignment
Files:
monitoring/csrc/native_engine.hmonitoring/csrc/native_engine_internal.hmonitoring/csrc/native_engine.cppmonitoring/csrc/hooks.cppmonitoring/csrc/bindings.cppChanges:
BackendFuturenow carriestask_size(token, task_size)for host queue budgetingBackendFuture.size()3) Host-engine side (delta vs
origin/v0_host_side) — for focused reviewFiles:
monitoring/csrc/dmx_host_engine.hmonitoring/csrc/dmx_host_utils.hmonitoring/csrc/dmx_host_utils.cppmonitoring/csrc/future_process.cppmonitoring/csrc/clickhouse_client.hmonitoring/csrc/clickhouse_client.cppmonitoring/csrc/bindings.cppPost-merge fixes/hardening (no architecture redesign):
std::move(request_ids[i])/std::move(token_range_per_request[i])request_ids.size == token_ranges.sizetensor.size(0) == request_ids.sizestart >= end, negative)start_token_idx/end_token_idxvariables for effective range slicingt_future.size()in host enqueue budgetBackendFuture.size()is available from bindingsTests added
Files:
tests/test_monitoring_engine_request_id.pytests/validate_request_id_pipeline.pybenchmark/data/prompts_varlen_validation.txtCoverage:
--exercise-eos-path)--with-attn-hook)Compatibility / notes
shard_rankis preserved in host->DB schema pathshard_rank=0as placeholder (TP/distributed to follow)Review request