natsukium/felis
Watch
1
Code Issues 27 Pull requests 1 Projects Releases Packages Wiki Activity Actions

[v0.1/P1] Make daemon status resource scopes unambiguous #26

New issue
Closed
opened 2026-09-03 16:18:06 +09:00 by natsukium · 1 comment
natsukium commented 2026-09-03 16:18:06 +09:00
Owner
Copy link

Parent: #12 (P1.6). Supersedes the status-reporting part of #10.

Why

daemon status currently prints every resource as used / limit, although some values are daemon-wide totals, some are deepest-subject samples, and limits may be global, per-session, or per-subscriber. The displayed ratios are dimensionally ambiguous.

Scope

  • Replace generic used/limit with total_used, max_subject_used, per_subject_limit, and global_limit.
  • Omit dimensions that do not apply.
  • Make each human-readable ratio use the matching denominator and name its scope.
  • Keep sampling authoritative rather than introducing drifting hot-path counters.

Acceptance criteria

  • Every status row identifies resource, subject scope, measurement scope, and limit scope.
  • No human ratio compares values with different scopes.
  • Machine output has golden fixtures for global, per-session, and per-subscriber resources.
  • Admission refusals can be diagnosed from status output.
  • CLI/spec/control-surface docs match the serialized fields.
Parent: #12 (P1.6). Supersedes the status-reporting part of #10. ## Why `daemon status` currently prints every resource as `used / limit`, although some values are daemon-wide totals, some are deepest-subject samples, and limits may be global, per-session, or per-subscriber. The displayed ratios are dimensionally ambiguous. ## Scope - Replace generic `used`/`limit` with `total_used`, `max_subject_used`, `per_subject_limit`, and `global_limit`. - Omit dimensions that do not apply. - Make each human-readable ratio use the matching denominator and name its scope. - Keep sampling authoritative rather than introducing drifting hot-path counters. ## Acceptance criteria - [ ] Every status row identifies resource, subject scope, measurement scope, and limit scope. - [ ] No human ratio compares values with different scopes. - [ ] Machine output has golden fixtures for global, per-session, and per-subscriber resources. - [ ] Admission refusals can be diagnosed from status output. - [ ] CLI/spec/control-surface docs match the serialized fields.
natsukium commented 2026-09-03 20:35:15 +09:00
Author
Owner
Copy link

Triage plan (2026-09-03)

Source-grounded triage against main at 69076d42, reviewed through seven rounds of an independent reviewer (pi sol/luna) until it passed with no findings. The dependency order that supersedes the tracker's is posted on #12.

Claim check

Accurate on substance; one part overstated.

  • Field shape. crates/felis-protocol/proto/felis.proto:1389-1401 ResourceReport { resource, unit, used, limit?, scope }, with the comment "used and limit do not always share a denominator, which is what scope states". LimitScope (:1433-1444) says SESSION: "used is the daemon-wide sum ... the pair is not a ratio"; SUBSCRIBER: "used is the deepest single subscriber".
  • What is sampled. crates/felis-daemon/src/serve.rs:928-1010 daemon_status: image_bytes is a sum over sessions against a per-session cap (:944, :975-981); decode_bytes is a sum against the per-session REASSEMBLY_BUFFER_LIMIT (:993-999); deepest_outbox is a max against the per-subscriber cap (:1000-1006). So one field name (used) carries a sum in two rows and a max in a third. The per-session max for image bytes is not reported at all, though the actor already returns it (session_task.rs:51-59 SessionStats.image_bytes), so an operator seeing image_store_bytes 300 MiB / 256 MiB per session cannot tell whether any single session is near its cap.
  • Overstated: "displayed ratios are dimensionally ambiguous". The human renderer names the scope on every row (crates/felis-cli/src/cli_daemon.rs print_status: "{used} / {limit} per {scope}", with the comment "The scope is printed on every row ... a reader who must notice an absent qualifier has been given a trap"), and docs/reference/cli.md:275-281 has a table saying which rows are not ratios. The ambiguity is real but it is the / glyph plus the hidden sum/max inconsistency, not a missing label.
  • Diagnosability. Criterion 4 fails today for image_store_bytes and in_flight_decode_bytes (no max-subject value), passes for sessions (serve.rs:965-971) and subscriber_queue_bytes (already a max).
  • Sampling is already authoritative. Every row is built from SessionCmd::Stats replies (serve.rs:940-951); no hot-path counter exists to drift. Criterion "keep sampling authoritative" is satisfied by construction and stays so if the new fields are computed in the same loop.

Verdict

accept-with-changes. The four-field shape is right because #14 (P0, lands first) introduces daemon-wide budgets beside per-subject caps, at which point a row genuinely has two ceilings and a single limit+scope pair can no longer say which is which. Without #14 a global_limit/per_subject_limit pair would be two optionals with at most one set, the optional-bag shape #48 is removing elsewhere. So: land after #14 and read the ceilings from its admission object.

Changes to the written scope:

  1. Keep scope (as subject scope: daemon | session | subscriber), since max_subject_used is meaningless without naming the subject. Drop the "limit scope" meaning; the two limit fields carry that now.
  2. total_used is always present; max_subject_used and per_subject_limit are absent for daemon scope; global_limit is absent where #14 sets no aggregate budget. proto3 optional on all three.
  3. Human output: one line per row with the ratios that exist, each labelled with its own denominator (image_store_bytes total 300 MiB · max session 120 MiB / 256 MiB per session · 1 GiB per daemon); never total / per-subject.

Approach

Protocol: ResourceReport { resource, unit, uint64 total_used, optional uint64 max_subject_used, optional uint64 per_subject_limit, optional uint64 global_limit, SubjectScope scope } (felis.proto:1389-1401); rename LimitScope → SubjectScope and reword its comments; update OpsStatusReply comment (:1369-1387). Add bool draining for #25. messages.rs/convert.rs follow.

Daemon (serve.rs:928-1010): in the same sampling loop, track max_image_bytes = max(...) and max_decode_bytes beside the sums; deepest_outbox becomes max_subject_used and a new sum of backlogs becomes total_used (SessionStats.max_subscriber_backlog needs a sibling total_subscriber_backlog, session_task.rs:1220-1228). Rows: sessions (daemon: total, global_limit), image_store_bytes (session: total, max, per_subject_limit=image_cap, global_limit from #14), in_flight_decodes (daemon), in_flight_decode_bytes (session), subscriber_queue_bytes (subscriber).

Client core / CLI: DaemonStatus/ResourceReport passthrough; cli_output.rs:570-585 ResourceObject gains the fields (skip_serializing_if = Option::is_none); cli_daemon.rs resource_object, print_status, and render_amount reworked per verdict item 3; scope_token unchanged.

Docs cascade: docs/reference/cli.md:253-300 (example block, machine object, the scope table becomes a field-presence table), docs/reference/spec.md:262 REQ-1102 wording ("pairing observed usage with the configured limit" → the four dimensions), docs/reference/ipc.md Ops section and :1817 (subscriber_queue_bytes sentence), ledger/2.0 base, docs/reference/control-surfaces.md:33, docs/explanation/architecture/control-surfaces.md:140-160 (record why a max-subject sample and not a per-subject table: the table is sessions info's job; "Revisit if" an operator needs to name the offending session from status alone), skills/felis/SKILL.md:213-237 (the "Read scope before comparing" paragraph), CHANGELOG.md.

Tests: serve/tests.rs:4432 and :4522 extended with two sessions holding different image byte counts (assert total == a + b, max == max(a, b)); cli_daemon.rs tests: golden JSON for one row of each scope (field presence exact), and a human-render test asserting no line contains a total value followed by a per session/per subscriber denominator; every_resource_has_its_documented_token keeps the tokens.

Dependencies

  • #14 first (source of global_limit; deciding the shape before it means guessing which rows get an aggregate).
  • #25 (adds draining to the same reply; land together or #26 first).
  • #30 after (renaming used is a break).
  • Tracker step 8 holds.

Risk/effort

S–M. Mechanical once #14's ceilings exist. Main risk: doing it before #14 and then re-breaking the reply when the aggregate budgets arrive, which is the "clean baseline immediately accumulates another break" failure #52 warns about.

Labels

Keep release/v0.1.0 (the machine object freezes at the tag). Priority could drop to P2 on user impact alone, since scope is already labelled on every row, but the field rename must precede #30, so priority/P1 stays for sequencing.

## Triage plan (2026-09-03) Source-grounded triage against `main` at `69076d42`, reviewed through seven rounds of an independent reviewer (`pi` sol/luna) until it passed with no findings. The dependency order that supersedes the tracker's is posted on #12. ## Claim check Accurate on substance; one part overstated. - **Field shape.** `crates/felis-protocol/proto/felis.proto:1389-1401` `ResourceReport { resource, unit, used, limit?, scope }`, with the comment "`used` and `limit` do not always share a denominator, which is what `scope` states". `LimitScope` (`:1433-1444`) says `SESSION`: "`used` is the daemon-wide sum ... the pair is not a ratio"; `SUBSCRIBER`: "`used` is the deepest single subscriber". - **What is sampled.** `crates/felis-daemon/src/serve.rs:928-1010` `daemon_status`: `image_bytes` is a **sum** over sessions against a **per-session** cap (`:944`, `:975-981`); `decode_bytes` is a sum against the per-session `REASSEMBLY_BUFFER_LIMIT` (`:993-999`); `deepest_outbox` is a **max** against the per-subscriber cap (`:1000-1006`). So one field name (`used`) carries a sum in two rows and a max in a third. The per-session max for image bytes is not reported at all, though the actor already returns it (`session_task.rs:51-59` `SessionStats.image_bytes`), so an operator seeing `image_store_bytes 300 MiB / 256 MiB per session` cannot tell whether any single session is near its cap. - **Overstated:** "displayed ratios are dimensionally ambiguous". The human renderer names the scope on every row (`crates/felis-cli/src/cli_daemon.rs` `print_status`: `"{used} / {limit} per {scope}"`, with the comment "The scope is printed on every row ... a reader who must notice an *absent* qualifier has been given a trap"), and `docs/reference/cli.md:275-281` has a table saying which rows are not ratios. The ambiguity is real but it is the `/` glyph plus the hidden sum/max inconsistency, not a missing label. - **Diagnosability.** Criterion 4 fails today for `image_store_bytes` and `in_flight_decode_bytes` (no max-subject value), passes for `sessions` (`serve.rs:965-971`) and `subscriber_queue_bytes` (already a max). - **Sampling is already authoritative.** Every row is built from `SessionCmd::Stats` replies (`serve.rs:940-951`); no hot-path counter exists to drift. Criterion "keep sampling authoritative" is satisfied by construction and stays so if the new fields are computed in the same loop. ## Verdict **accept-with-changes.** The four-field shape is right *because* #14 (P0, lands first) introduces daemon-wide budgets beside per-subject caps, at which point a row genuinely has two ceilings and a single `limit`+`scope` pair can no longer say which is which. Without #14 a `global_limit`/`per_subject_limit` pair would be two optionals with at most one set, the optional-bag shape #48 is removing elsewhere. So: land after #14 and read the ceilings from its admission object. Changes to the written scope: 1. Keep `scope` (as *subject* scope: `daemon | session | subscriber`), since `max_subject_used` is meaningless without naming the subject. Drop the "limit scope" meaning; the two limit fields carry that now. 2. `total_used` is always present; `max_subject_used` and `per_subject_limit` are absent for `daemon` scope; `global_limit` is absent where #14 sets no aggregate budget. proto3 `optional` on all three. 3. Human output: one line per row with the ratios that exist, each labelled with its own denominator (`image_store_bytes total 300 MiB · max session 120 MiB / 256 MiB per session · 1 GiB per daemon`); never `total / per-subject`. ## Approach **Protocol:** `ResourceReport { resource, unit, uint64 total_used, optional uint64 max_subject_used, optional uint64 per_subject_limit, optional uint64 global_limit, SubjectScope scope }` (`felis.proto:1389-1401`); rename `LimitScope` → `SubjectScope` and reword its comments; update `OpsStatusReply` comment (`:1369-1387`). Add `bool draining` for #25. `messages.rs`/`convert.rs` follow. **Daemon (`serve.rs:928-1010`):** in the same sampling loop, track `max_image_bytes = max(...)` and `max_decode_bytes` beside the sums; `deepest_outbox` becomes `max_subject_used` and a new `sum` of backlogs becomes `total_used` (`SessionStats.max_subscriber_backlog` needs a sibling `total_subscriber_backlog`, `session_task.rs:1220-1228`). Rows: `sessions` (daemon: total, global_limit), `image_store_bytes` (session: total, max, per_subject_limit=`image_cap`, global_limit from #14), `in_flight_decodes` (daemon), `in_flight_decode_bytes` (session), `subscriber_queue_bytes` (subscriber). **Client core / CLI:** `DaemonStatus`/`ResourceReport` passthrough; `cli_output.rs:570-585` `ResourceObject` gains the fields (`skip_serializing_if = Option::is_none`); `cli_daemon.rs` `resource_object`, `print_status`, and `render_amount` reworked per verdict item 3; `scope_token` unchanged. **Docs cascade:** `docs/reference/cli.md:253-300` (example block, machine object, the scope table becomes a field-presence table), `docs/reference/spec.md:262` REQ-1102 wording ("pairing observed usage with the configured limit" → the four dimensions), `docs/reference/ipc.md` Ops section and `:1817` (`subscriber_queue_bytes` sentence), ledger/2.0 base, `docs/reference/control-surfaces.md:33`, `docs/explanation/architecture/control-surfaces.md:140-160` (record why a max-subject sample and not a per-subject table: the table is `sessions info`'s job; "Revisit if" an operator needs to name the offending session from status alone), `skills/felis/SKILL.md:213-237` (the "Read `scope` before comparing" paragraph), `CHANGELOG.md`. **Tests:** `serve/tests.rs:4432` and `:4522` extended with two sessions holding different image byte counts (assert `total == a + b`, `max == max(a, b)`); `cli_daemon.rs` tests: golden JSON for one row of each scope (field presence exact), and a human-render test asserting no line contains a `total` value followed by a `per session`/`per subscriber` denominator; `every_resource_has_its_documented_token` keeps the tokens. ## Dependencies - **#14 first** (source of `global_limit`; deciding the shape before it means guessing which rows get an aggregate). - **#25** (adds `draining` to the same reply; land together or #26 first). - **#30** after (renaming `used` is a break). - Tracker step 8 holds. ## Risk/effort **S–M.** Mechanical once #14's ceilings exist. Main risk: doing it before #14 and then re-breaking the reply when the aggregate budgets arrive, which is the "clean baseline immediately accumulates another break" failure #52 warns about. ## Labels Keep `release/v0.1.0` (the machine object freezes at the tag). Priority could drop to P2 on user impact alone, since scope is already labelled on every row, but the field rename must precede #30, so `priority/P1` stays for sequencing.
Sign in to join this conversation.
No Branch/Tag specified
Branches Tags
main
ci/darwin-smoke
No results found.
Labels
Clear labels   
priority/P0
Must be resolved before the v0.1.0 tag

  
priority/P1
Must be settled before the first public contract freezes

  
priority/P2
Explicitly deferred until after v0.1.0

  
release/v0.1.0
Work tracked for the first tagged release

  
status/blocked
Cannot start now: waits on an upstream release, another issue, or a recorded revisit trigger

  
status/planned
Triaged with a source-grounded plan and a passed pi review; ready to implement

  
type/bug
Shipped behavior that contradicts the docs or the code's own intent; needs a product code change

  
type/design
An open scope or trade-off question; needs a recorded decision in the owning explanation doc before code

  
type/test-gap
Documented or reviewed behavior that no test pins; usually split from a review finding, needs no product code change

  
type/tracker
Parent issue that tracks child work

No labels
priority/P0
priority/P1
priority/P2
release/v0.1.0
status/blocked
status/planned
type/bug
type/design
type/test-gap
type/tracker
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
natsukium/felis#26
No description provided.