Rongsong Shen
1374540954
Follow-on to scheduler-sim-repl-verbosity: collapses the REPL's per-event "[sim] t=... session=..." preamble to print once per distinct rendered (timestamp, session) run of consecutive echoed events, instead of once per event, in both step mode and continue mode's structural-event echo. Requirement 1.3 deliberately keys the dedup on the rendered, second-granularity timestamp rather than the raw nanosecond clock value, which ticks on every event and would otherwise never repeat -- confirmed against a real user-supplied transcript before writing requirements (see research.md). Requirement 2 preserves event selection, ordering, content, every other REPL command, and all scheduling decisions/state exactly as scheduler-sim-repl-verbosity's Requirement 5 already established. Generated via kiro-spec-quick (single-pass, no approval gates) with an inline requirements review gate, an inline design review gate, and an independent task-graph sanity review (one NEEDS_FIXES round, repaired: added file-size watch bullets to tasks 1.1/1.2 per design.md's own housekeeping directives). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YAWMhfRJFnmmnG8h97yUDd
37 lines
4.6 KiB
Markdown
37 lines
4.6 KiB
Markdown
# Research & Design Decisions
|
|
|
|
## Summary
|
|
- **Feature**: `scheduler-sim-step-verbosity`
|
|
- **Discovery Scope**: Extension (existing REPL Output domain, `scheduler-sim-repl-verbosity`)
|
|
- **Key Findings**:
|
|
- The redundant preamble is printed unconditionally by `repl_print_event_summary()` (`repl_break.c`), which has exactly one call site (`xce_repl_on_event()`) — no cross-cutting fan-out to design around.
|
|
- The raw nanosecond timestamp (`xce_core_now()`) is effectively unique per event (the virtual clock ticks per decoded event via `sim_advance_virtual_clock()`), so any dedup key based on the raw value would almost never actually trigger — this was confirmed directly against a real user-supplied transcript before writing requirements, and materially changed Requirement 1's acceptance criteria (see Decision below).
|
|
- `xce_sched_repl_state_s` already has an established, low-risk pattern for adding session-scoped display-only cache fields (the Round Rollup's collapse state, `units_mode`) that this feature reuses without new lifecycle code.
|
|
|
|
## Research Log
|
|
|
|
### What actually repeats between consecutive echoed events?
|
|
- **Context**: The initial feature description assumed collapsing on "(t, session)" where `t` was the raw timestamp value shown in the preamble.
|
|
- **Sources Consulted**: `repl_print_event_summary()` (`repl_break.c:672-682`); a real interactive-session transcript supplied by the user (`--sim`/`--interactive`, session 47) showing several consecutive events.
|
|
- **Findings**: In the real transcript, the raw `t=<ns>` value differed on every single line (e.g. `t=1788169162308792367` then `t=1788169162308787761`), while the parenthesized human-readable ISO8601 timestamp and the `session=` number were identical across many consecutive lines.
|
|
- **Implications**: A literal "(t, session)" grouping key as originally worded would essentially never collapse anything, since raw `t` never repeats. Requirement 1's acceptance criteria were corrected to key on the rendered, second-granularity human timestamp and the displayed session number — confirmed with the user before writing requirements.md (see Design Decision below).
|
|
|
|
## Design Decisions
|
|
|
|
### Decision: Dedup key is the rendered (human timestamp, session) pair, not the raw nanosecond value
|
|
- **Context**: Requirement 1.3 needed to state precisely what "same context" means for two consecutive echoed events.
|
|
- **Alternatives Considered**:
|
|
1. Raw nanosecond timestamp + session — rejected: virtually never repeats between events, so it would satisfy the letter of "dedup" while delivering none of the actual verbosity reduction the feature exists for.
|
|
2. Session number alone (ignore timestamp for grouping) — considered but not selected: presented to the user as an option; they chose option 3 below instead.
|
|
3. Rendered second-granularity human timestamp + session number — **selected**. Matches what an operator actually perceives as "the same displayed context," and matches what genuinely repeats in real output.
|
|
- **Selected Approach**: Cache the last-printed rendered timestamp string and displayed session number on `xce_sched_repl_state_s`; compare before printing a new preamble; print and update the cache only on a change (or on the very first echoed event, naturally covered by the cache's zero-initialized default).
|
|
- **Rationale**: Directly addresses the literal, evidence-based redundancy pattern observed in real output, without introducing a new formatting concept.
|
|
- **Trade-offs**: None material — this is a strict readability improvement with no behavioral cost; the cache adds 40 bytes to a per-session struct.
|
|
- **Follow-up**: None — the design's Testing Strategy already covers the "raw ns differs, rendered value doesn't" case explicitly (the case that motivated this decision).
|
|
|
|
## Risks & Mitigations
|
|
- `repl_break.c` is already at an accepted 949-line residual from `scheduler-sim-repl-verbosity` task 9.2; this feature's addition is small but non-zero — mitigated by design.md's explicit "watch, don't pre-split" note and the established empirical-test-before-splitting practice this file family already follows.
|
|
- `test_repl_event_echo.c` (705 lines) may approach the ~1000-line split trigger once this feature's tests land — mitigated by design.md's File Structure Plan flagging a proactive split into `test_repl_step_verbosity.c` if needed, following the exact precedent already set for sibling test files in `scheduler-sim-repl-verbosity`.
|
|
|
|
## References
|
|
- `.kiro/specs/scheduler-sim-repl-verbosity/design.md` — the sibling spec this one extends; source of the file-size-cap precedent and the `xce_sched_repl_state_s` state-field convention reused here.
|