docs(scheduler-sim-repl-verbosity): fix two documentation gaps from final validation

Found by /kiro-validate-impl's cross-task requirements-coverage and
design-alignment checks, both documentation-only (no code changes):

1. AC 6.5 (whole-number/no-trailing-.00 formatting) is implemented as
   scoped to non-consumable (Load Metrics) values only - a consumable
   resource's value=/used= pair deliberately keeps its unconditional
   %.2f formatting to preserve bug-1601/1613/1727's already-pinned
   display shape. This carve-out existed only as an inline code
   comment (repl_info.c) and a test-file header comment, never
   sanctioned in requirements.md/design.md itself. Amended AC 6.5's
   text to state the scope explicitly and added a design.md Correction
   note cross-referencing it.

2. design.md's File Structure Plan never documented task 6.1's proactive
   split of the prompt-content tests into a new test_repl_prompt_context.c
   (every other analogous split in this spec got a Correction note;
   this one didn't), and task 9.1's later Correction note repeated the
   stale claim that test_repl_execution_control.c "already carries" the
   prompt-content tests - it doesn't, they moved to the new file at task
   6.1. Added the missing task 6.1 Correction note and fixed both stale
   references.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01YAWMhfRJFnmmnG8h97yUDd
This commit is contained in:
Rongsong Shen 2026-09-02 21:55:50 +00:00
parent e87c0b3fc9
commit a51790dcbb
No known key found for this signature in database
GPG key ID: 5BCDAAE0123E9FE2
2 changed files with 8 additions and 3 deletions

View file

@ -119,8 +119,11 @@ flowchart TD
### New/Modified Test Files
**Correction (post-review, task 2.1)**: `test_repl_execution_control.c` was already over this project's 800-line cap (937 lines) before this feature's work began. Landing task 2.1's Round Rollup tests there per the original plan below pushed it to 1563 lines — nearly double the cap, and past the ~1000-line threshold this exact spec family already treats as a split trigger (`test_repl_snapshot_params.c` was split out of `test_repl_breakpoints_inspection.c` for precisely this reason; see that file's own meson.build comment). Round Rollup's tests (task 2.1, and any Round-Rollup-internal portion of later tasks, e.g. Requirement 3.5's flush-on-structural-event/breakpoint interaction) now live in a new `src/scheduler/testsuite/test_repl_round_rollup.c` instead, duplicating (not sharing) whatever pre-existing static fixture helpers it needs — the established convention this same file pair (`test_repl_breakpoints_inspection.c`/`test_repl_snapshot_params.c`) already follows, rather than factoring out a shared test-helper header. `test_repl_execution_control.c` remains at its restored 937-line pre-feature size (still over cap on its own, a pre-existing condition this feature doesn't need to fix) and is the target only for the Requirement 5 no-regression checks, the new prompt-content tests (Requirement 5.5), and `help`-listing structure tests (Requirement 7.1-7.4) — re-check its line count before landing each of those; if it approaches ~1000+ lines again, split further rather than growing past it.
**Correction (post-review, task 6.1)**: the new prompt-content tests (Requirement 5.5) do NOT land in `test_repl_execution_control.c` as this section originally directed (line 123 below). That file was already at 937 lines (task 2.1's Correction above having just restored it to its pre-feature size); the pty-based prompt tests (`readline()` only writes its prompt to stdout when both stdin and stdout are real ttys — a distinct, non-trivial capture technique from this file's existing `freopen()`-from-file harness) are a self-contained addition, not an extension of any existing test in this file. They instead live in a new `src/scheduler/testsuite/test_repl_prompt_context.c`, duplicating pre-existing fixture helpers per this file family's established convention, decided proactively before landing rather than after a review round — the same practice task 2.1 already established. `test_repl_execution_control.c` remains untouched by task 6.1 and stays the designated home only for the Requirement 5 no-regression checks it already carries.
- `src/scheduler/testsuite/test_repl_prompt_context.c` (new, per the Correction above) — the dynamic REPL prompt's plain-before-any-session and session-count/timestamp-after-a-session content, in both `step` and `continue` modes, over a real pty (Requirement 5.5).
**Correction (post-review, task 3.1)**: applying the same reasoning that split Round Rollup's tests out in task 2.1 — Requirement 1's Event Echo Gate tests are comparable in scope (4 behavioral cases: routine-event suppression, step-mode-unaffected, structural-event-ends-range, breakpoint-ends-range) and would push the already-over-cap `test_repl_execution_control.c` (937 lines) well past 1300+ lines. They instead live in a new `src/scheduler/testsuite/test_repl_event_echo.c`, duplicating pre-existing fixture helpers per this file family's established convention, decided proactively before implementation rather than after a review round.
- `src/scheduler/testsuite/test_repl_execution_control.c` — the Requirement 5 no-regression checks (this file already drives `sched_sim_driver_run()` end-to-end and captures real process output via `child_continue_then_inspect`-style helpers); also gains the new prompt-content tests (Requirement 5.5) and `help`-listing structure tests (Requirement 7.1-7.4) — the natural home given it already exercises `readline()`/prompt-adjacent behavior and no `help` test exists anywhere today.
- `src/scheduler/testsuite/test_repl_execution_control.c` — the Requirement 5 no-regression checks (this file already drives `sched_sim_driver_run()` end-to-end and captures real process output via `child_continue_then_inspect`-style helpers). (Post-review: the prompt-content tests, Requirement 5.5, and the `help`-listing structure tests, Requirement 7.1-7.4, originally planned for this file both landed elsewhere instead — see the task 6.1 and task 9.1 Corrections above.)
- `src/scheduler/testsuite/test_repl_round_rollup.c` (new, per the task 2.1 Correction above) — child-process output-capture tests for Requirements 2 and 3 (per-round decision summary, collapsing, the shared flush entry point, and its call from `xce_repl_on_replay_complete`).
- `src/scheduler/testsuite/test_repl_event_echo.c` (new, per the task 3.1 Correction above) — child-process output-capture tests for Requirement 1 (mode-aware event echo classification, ending an active collapsed range on structural events/breakpoint hits).
- `src/scheduler/testsuite/test_repl_output_compression.c` (new) — focused unit tests for the jobid-range-compression helper (Requirement 4): a pure function, tested without the process harness.
@ -131,7 +134,7 @@ flowchart TD
- `src/scheduler/testsuite/test_repl_resource_table.c` (new, per the Correction above) — `info worker`'s/`info resource`'s resource-listing table/grouping layout, `units_mode`-aware rendering, and `info worker`'s assigned-job-listing jobid-range-compression adoption (Requirements 6.4, 6.5, 6.6, 4.5).
- `src/scheduler/testsuite/test_repl_snapshot_params.c` — extended with `set units raw|human|both` parsing/validation tests (Requirement 6.2, 6.3), alongside its existing `set speed`/`set output`/`set verify` tests.
**Correction (post-review, task 9.1)**: `help`-listing structure tests (Requirement 7.1-7.4) do NOT land in `test_repl_execution_control.c` as this section originally directed (line 123 above). That file was already at 949 lines going into task 9.1 — within reach of the ~1000-line split trigger this same Correction note (task 2.1) told this task to watch for — and the new section/column/event-type-listing tests would have pushed it to ~1500 lines. They instead live in a new `src/scheduler/testsuite/test_repl_help.c` (555 lines), duplicating pre-existing fixture helpers (`capture_stdout_around`) per this file family's established convention, decided proactively before landing rather than after a review round — the same practice tasks 2.1, 3.1, and 8.1 already established. `test_repl_execution_control.c` remains untouched by task 9.1 and stays the designated home only for the Requirement 5 no-regression checks and the prompt-content tests (Requirement 5.5) it already carries. `break`'s own enriched unrecognized-event-type error test (Requirement 7.5) landed in `test_repl_breakpoints_inspection.c` exactly as line 128 above already specified — no correction needed there.
**Correction (post-review, task 9.1)**: `help`-listing structure tests (Requirement 7.1-7.4) do NOT land in `test_repl_execution_control.c` as this section originally directed (line 123 above). That file was already at 949 lines going into task 9.1 — within reach of the ~1000-line split trigger this same Correction note (task 2.1) told this task to watch for — and the new section/column/event-type-listing tests would have pushed it to ~1500 lines. They instead live in a new `src/scheduler/testsuite/test_repl_help.c` (555 lines), duplicating pre-existing fixture helpers (`capture_stdout_around`) per this file family's established convention, decided proactively before landing rather than after a review round — the same practice tasks 2.1, 3.1, and 8.1 already established. `test_repl_execution_control.c` remains untouched by task 9.1 and stays the designated home only for the Requirement 5 no-regression checks (the prompt-content tests, Requirement 5.5, never landed there either — they live in `test_repl_prompt_context.c` per the task 6.1 Correction above). `break`'s own enriched unrecognized-event-type error test (Requirement 7.5) landed in `test_repl_breakpoints_inspection.c` exactly as line 128 above already specified — no correction needed there.
- `src/scheduler/testsuite/test_repl_help.c` (new, per the Correction above) — `help`'s four labeled sections in order, one consistent column width across all listed commands, `info config`'s placement in State Inspection, `set units`'s placement in Snapshot & Parameters, and the `break` line's complete `BP_KIND_TABLE`-derived event-type listing (Requirements 7.1, 7.2, 7.3, 7.4).
## System Flows
@ -338,6 +341,8 @@ void repl_build_prompt(xce_sched_sim_driver_t* sim, char* buf, size_t bufsz);
**Accepted residual (post-review, task 8.2)**: implementing the table/grouping rendering pushed `repl_info.c` from 985 to 1267 lines (58% over the 800-line cap). Unlike `repl.c`/`repl_break.c`, `repl_info.c` was already 23% over cap (985 lines) before this spec touched it at all — this is inherited debt, not something this feature introduced from a clean baseline. Review empirically tried the narrow extraction (moving just this task's new classification/formatting helpers — `repl_classify_worker_resval_section`/`repl_classify_resource_def_section`/`repl_format_numeric_resource_value`, 137 lines — into the existing `repl_fmt.c`): it only reaches 1130 lines, still 41% over cap, the same "doesn't achieve compliance" outcome that got task 6.2's single-function split rejected as disproportionate. Achieving genuine compliance would require extracting the entire pre-existing `repl_info_worker_one()`/`repl_info_resource_one()`/`repl_info_resource()` display bodies (~690 lines) into a new domain file — a materially larger structural change (new `meson.build` entries, ~20 test files' `#include` lists) restructuring code this feature doesn't otherwise touch, which is a `scheduler-core`-level cleanup decision, not something to fold into this feature's own scope. Accepted in place; revisit only if a future `info`-command spec (e.g. `scheduler-sim-diagnostics`) adds enough new content here to make a full domain split worth doing on its own merits.
**Correction (post-review, task 8.2, requirements.md AC 6.5 amended to match)**: Requirement 6.5's whole-number/no-trailing-`.00` rule, as implemented, applies to non-consumable (Load Metrics) numeric values only. A consumable/capacity resource's `value=`/`used=` pair keeps its pre-existing unconditional `"%.2f"` formatting and is not `units_mode`-aware at all — deliberately, to preserve bug-1601/1613/1727's already-pinned display shape (`test_repl_breakpoints_inspection.c`) rather than risk a second, unrelated regression in code this feature doesn't otherwise need to touch. `requirements.md`'s AC 6.5 text has been updated to state this scope explicitly rather than leaving it as an unsanctioned deviation documented only in `repl_info.c`'s own code comment and `test_repl_resource_table.c`'s file header. No bytes-valued CONSUMABLE resource is exercised anywhere in this codebase's tests today, so the corollary (AC 6.1's unit-scaling not applying to a hypothetical bytes-valued consumable) is currently dormant, not demonstrably wrong — revisit only if such a resource is ever introduced.
**Dependencies**
- Inbound: `repl_info_worker()`, `repl_info_resource()` (repl_info.c) (P0); `repl_cmd_set()` (repl_params.c) (P0)
- Outbound: none beyond standard C formatting — a self-contained classification/formatting pair, same shape as the Jobid Range Compressor

View file

@ -67,7 +67,7 @@ xce_scheduler's `--interactive` simulation REPL (shipped by the `scheduler-core`
2. The REPL shall support a `set units raw|human|both` option: `raw` shows only the numeric wire value, `human` shows only an adaptively-scaled unit (B/KB/MB/GB/TB/PB), and `both` shows the raw value with the human-readable form alongside.
3. Absent an explicit `set units` command, the REPL's units display mode shall default to `both`.
4. The REPL shall not alter the underlying stored/wire value of any resource, nor the deterministic outcome of a replay — only its console display.
5. The REPL shall display numeric resources with no canonical unit (for example load averages, factors, and counts) without a unit suffix, and without a trailing `.00` when the value is a whole number.
5. The REPL shall display numeric resources with no canonical unit (for example load averages, factors, and counts) without a unit suffix, and without a trailing `.00` when the value is a whole number. This rule applies to non-consumable (Load Metrics) values; a consumable/capacity resource's `value=`/`used=` pair keeps its existing unconditional two-decimal display (bug-1601/1613/1727's pinned shape) and is out of scope for this AC.
6. The REPL shall display `info worker`'s and `info resource`'s resource listings as an aligned table, grouping resources by category (consumable/capacity, load metrics, static hardware attributes) where applicable to that listing, rather than one flat list.
### Requirement 7: Discoverable Help and Breakpoint Event Types