diff --git a/.kiro/specs/scheduler-sim-repl-verbosity/design.md b/.kiro/specs/scheduler-sim-repl-verbosity/design.md index 664cb85f..75f4d9ff 100644 --- a/.kiro/specs/scheduler-sim-repl-verbosity/design.md +++ b/.kiro/specs/scheduler-sim-repl-verbosity/design.md @@ -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 diff --git a/.kiro/specs/scheduler-sim-repl-verbosity/requirements.md b/.kiro/specs/scheduler-sim-repl-verbosity/requirements.md index f0014d42..0da0fa11 100644 --- a/.kiro/specs/scheduler-sim-repl-verbosity/requirements.md +++ b/.kiro/specs/scheduler-sim-repl-verbosity/requirements.md @@ -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