feat(scheduler-sim-decision-trace): move units and value formatting helpers into repl_fmt.c

Task 1.2: translate_units_mode, repl_format_numeric_resource_value,
repl_format_consumable_value_and_used and repl_current_units_mode move
unchanged from repl_info.c and are declared in repl_internal.h so the
trace renderer can reuse them.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Rongsong Shen 2026-10-08 07:30:42 +00:00
parent ce757eaaa1
commit 7f40ff27dc
Signed by untrusted user: rshen
GPG key ID: 80533536B0EB651C
5 changed files with 269 additions and 127 deletions

View file

@ -14,7 +14,7 @@
- Done when the new baseline test passes on the unmodified tree under ASAN and the fixtures are committed.
- _Requirements: 10.4, 10.5_
- [ ] 1.2 Move the units and value formatting helpers into the shared REPL formatting module
- [x] 1.2 Move the units and value formatting helpers into the shared REPL formatting module
- Move these helpers out of the info module so a new renderer can reuse them, with no change in behaviour:
- units-mode translation;
- numeric-resource and consumable value formatting;

View file

@ -18,12 +18,12 @@
* directly in repl.c per the original design.md plan and pushed it to 929
* lines, 16.1% over this project's 800-line-per-file cap
* (src/scheduler/CLAUDE.md), even though review confirmed the logic itself
* was correct. This file now holds three groups of pure formatting helpers
* that repl.c's own file header already called "the file's existing
* shared-helper convention" -- none of them touch
* xce_sched_sim_driver_t/xce_sched_core_t or any REPL session state, so
* grouping them here (rather than splitting each into its own file) keeps
* one cohesive, dependency-free unit:
* was correct. This file now holds groups of formatting helpers that
* repl.c's own file header already called "the file's existing
* shared-helper convention" -- none of them modify
* xce_sched_sim_driver_t/xce_sched_core_t or REPL session state, and only
* repl_current_units_mode() reads it, so grouping them here (rather than
* splitting each into its own file) keeps one cohesive unit:
*
* - repl_format_ts_human() (pre-existing, bug-1587) -- epoch-nanoseconds
* to human-readable UTC timestamp.
@ -33,6 +33,12 @@
* - The Resource Value Formatter's classification/scaling helpers (task
* 7.2, design.md's "Resource Value Formatter" component, Req 6.1, 6.2):
* repl_is_bytes_valued_resource()/repl_format_bytes_human().
* - Units-mode value formatting (scheduler-sim-decision-trace spec, task
* 1.2, Req 7.7, moved from repl_info.c so the trace renderer can share
* it): translate_units_mode(), repl_format_numeric_resource_value(),
* repl_format_consumable_value_and_used() and repl_current_units_mode().
* The last reads sim->repl_state->units_mode -- the one helper here
* that looks at REPL session state, read-only.
*
* Every one of these functions is already extern-declared in
* repl_internal.h -- this move changes only which .c file DEFINES them,
@ -357,3 +363,129 @@ int repl_fmt_jobid_range(char* buf, size_t bufsz, const uint64_t* jobids,
return rc;
}
/*
* Units-mode value formatting (moved verbatim from repl_info.c by the
* scheduler-sim-decision-trace spec, task 1.2, Req 7.7; behaviour
* unchanged, now non-static and declared in repl_internal.h) so the trace
* renderer (repl_trace_render.c) formats values exactly as `info worker`/
* `info resource`/`info limit` do.
*/
/*
* translate_units_mode - scheduler-sim-diagnostics spec, task 7 (Req 6.1,
* 6.3, 6.5): bridges the REPL's own units-mode selection
* (xce_repl_units_mode_t, repl_internal.h) to the shared library's
* independently-defined xce_units_mode_t (utils.h) at the one seam that
* needs to cross between them. Deliberately NOT a shared enum -- a
* src/lib type must not be defined in terms of a scheduler-internal
* concept, nor should the scheduler's own already-tested
* xce_repl_units_mode_t plumbing need to change for this (design.md's
* "Where the byte-formatter should live" decision). If
* xce_repl_units_mode_t's three values ever change, this mapping must be
* re-checked by hand -- nothing enforces it mechanically.
*/
xce_units_mode_t translate_units_mode(xce_repl_units_mode_t mode)
{
switch (mode) {
case XCE_REPL_UNITS_RAW:
return XCE_UNITS_RAW;
case XCE_REPL_UNITS_HUMAN:
return XCE_UNITS_HUMAN;
case XCE_REPL_UNITS_BOTH:
default:
return XCE_UNITS_BOTH;
}
}
/*
* repl_format_numeric_resource_value - Requirement 6.1/6.2/6.5: formats the
* text that follows "value=" for a non-consumable ("Load Metrics") numeric
* resource. A byte-valued resource (repl_is_bytes_valued_resource()) or
* `swp` (repl_internal.h's own documented MB-special-case, converted
* x1024x1024 before scaling) renders per `mode` -- raw: the bare "%f" wire
* value; human: repl_format_bytes_human()'s scaled form alone; both (the
* default): "<raw> (<human>)", the exact "%f (%s)" precedent xce_test's own
* cmd_resval already establishes (test.c:2540). Every other numeric
* resource renders with no unit suffix and no trailing ".00" on a whole
* number, keeping two decimal places for a fractional one. Shared verbatim
* by repl_info_worker_one()'s and repl_info_resource_one()'s identical Load
* Metrics rendering so the two commands can never disagree about this
* format.
*
* scheduler-sim-diagnostics spec, task 7 (Req 6.1, 6.3, 6.5): the
* byte-valued branch now delegates its raw/human/both composition to the
* shared library's xce_format_units_value() via translate_units_mode()
* above, instead of switching on `mode` itself -- so this function, its
* `info worker`/`info resource` callers, AND the `info limit` command
* this spec is adding all funnel through one single implementation.
*/
void repl_format_numeric_resource_value(xce_repl_units_mode_t mode,
const char* rname, double value,
char* buf, size_t bufsz)
{
bool is_swp = (rname != NULL) && (strcmp(rname, "swp") == 0);
if (repl_is_bytes_valued_resource(rname) || is_swp) {
double bytes_value = is_swp ? (value * 1024.0 * 1024.0) : value;
xce_format_units_value(translate_units_mode(mode), bytes_value, buf,
bufsz);
return;
}
/* Requirement 6.5: no trailing ".00" on a whole-number value; a
* fractional value keeps its two decimal places. */
if (value == (double)(long long)value) {
snprintf(buf, bufsz, "%.0f", value);
} else {
snprintf(buf, bufsz, "%.2f", value);
}
}
/*
* repl_format_consumable_value_and_used - formats a CONSUMABLE numeric
* resource's value=/used= pair for repl_info_worker_one()'s per-host case
* and repl_info_resource_one()'s GLOBAL/SHARED case. A byte/megabyte-valued
* resource (repl_is_bytes_valued_resource(), or `swp`'s documented MB
* special case -- the SAME classification repl_format_numeric_resource_value()
* already applies to the Load Metrics section) renders BOTH `avail` and
* `used` through that same formatter, respecting `mode` -- so
* `used=87960930222080.00` (a live-reported bug, 2026-09-11) instead
* becomes e.g. `used=87960930222080.000000 (80.00 TB)` under the default
* "both" mode, matching how maxmem/maxswap/maxtmp already render alongside
* it. Every other consumable resource (slots, licenses, ...) keeps the
* existing pinned "%.2f"/"%.2f" shape unchanged -- bug-1601/1613/1727's own
* tests assert this exact format and must not regress.
*/
void repl_format_consumable_value_and_used(xce_repl_units_mode_t mode,
const char* rname, double avail,
double used, char* value_buf,
size_t value_bufsz, char* used_buf,
size_t used_bufsz)
{
bool is_swp = (rname != NULL) && (strcmp(rname, "swp") == 0);
if (repl_is_bytes_valued_resource(rname) || is_swp) {
repl_format_numeric_resource_value(mode, rname, avail, value_buf,
value_bufsz);
repl_format_numeric_resource_value(mode, rname, used, used_buf,
used_bufsz);
return;
}
snprintf(value_buf, value_bufsz, "%.2f", avail);
snprintf(used_buf, used_bufsz, "%.2f", used);
}
/* repl_current_units_mode - sim->repl_state may legitimately be NULL (a few
* white-box tests dispatch `info worker`/`info resource` without ever
* calling repl_state_ensure()); default to the same XCE_REPL_UNITS_BOTH
* repl_state_ensure() itself explicitly assigns (Req 6.3), rather than
* leaving repl_format_numeric_resource_value() to guess. */
xce_repl_units_mode_t
repl_current_units_mode(xce_sched_sim_driver_t* sim)
{
return (sim->repl_state != NULL) ? sim->repl_state->units_mode
: XCE_REPL_UNITS_BOTH;
}

View file

@ -369,126 +369,10 @@ repl_classify_resource_def_section(const xce_resource_t* r)
: XCE_REPL_RES_SECTION_LOAD;
}
/*
* translate_units_mode - scheduler-sim-diagnostics spec, task 7 (Req 6.1,
* 6.3, 6.5): bridges the REPL's own units-mode selection
* (xce_repl_units_mode_t, repl_internal.h) to the shared library's
* independently-defined xce_units_mode_t (utils.h) at the one seam that
* needs to cross between them. Deliberately NOT a shared enum -- a
* src/lib type must not be defined in terms of a scheduler-internal
* concept, nor should the scheduler's own already-tested
* xce_repl_units_mode_t plumbing need to change for this (design.md's
* "Where the byte-formatter should live" decision). If
* xce_repl_units_mode_t's three values ever change, this mapping must be
* re-checked by hand -- nothing enforces it mechanically.
*/
static xce_units_mode_t translate_units_mode(xce_repl_units_mode_t mode)
{
switch (mode) {
case XCE_REPL_UNITS_RAW:
return XCE_UNITS_RAW;
case XCE_REPL_UNITS_HUMAN:
return XCE_UNITS_HUMAN;
case XCE_REPL_UNITS_BOTH:
default:
return XCE_UNITS_BOTH;
}
}
/*
* repl_format_numeric_resource_value - Requirement 6.1/6.2/6.5: formats the
* text that follows "value=" for a non-consumable ("Load Metrics") numeric
* resource. A byte-valued resource (repl_is_bytes_valued_resource()) or
* `swp` (repl_internal.h's own documented MB-special-case, converted
* x1024x1024 before scaling) renders per `mode` -- raw: the bare "%f" wire
* value; human: repl_format_bytes_human()'s scaled form alone; both (the
* default): "<raw> (<human>)", the exact "%f (%s)" precedent xce_test's own
* cmd_resval already establishes (test.c:2540). Every other numeric
* resource renders with no unit suffix and no trailing ".00" on a whole
* number, keeping two decimal places for a fractional one. Shared verbatim
* by repl_info_worker_one()'s and repl_info_resource_one()'s identical Load
* Metrics rendering so the two commands can never disagree about this
* format.
*
* scheduler-sim-diagnostics spec, task 7 (Req 6.1, 6.3, 6.5): the
* byte-valued branch now delegates its raw/human/both composition to the
* shared library's xce_format_units_value() via translate_units_mode()
* above, instead of switching on `mode` itself -- so this function, its
* `info worker`/`info resource` callers, AND the `info limit` command
* this spec is adding all funnel through one single implementation.
*/
static void repl_format_numeric_resource_value(xce_repl_units_mode_t mode,
const char* rname,
double value, char* buf,
size_t bufsz)
{
bool is_swp = (rname != NULL) && (strcmp(rname, "swp") == 0);
if (repl_is_bytes_valued_resource(rname) || is_swp) {
double bytes_value = is_swp ? (value * 1024.0 * 1024.0) : value;
xce_format_units_value(translate_units_mode(mode), bytes_value, buf,
bufsz);
return;
}
/* Requirement 6.5: no trailing ".00" on a whole-number value; a
* fractional value keeps its two decimal places. */
if (value == (double)(long long)value) {
snprintf(buf, bufsz, "%.0f", value);
} else {
snprintf(buf, bufsz, "%.2f", value);
}
}
/*
* repl_format_consumable_value_and_used - formats a CONSUMABLE numeric
* resource's value=/used= pair for repl_info_worker_one()'s per-host case
* and repl_info_resource_one()'s GLOBAL/SHARED case. A byte/megabyte-valued
* resource (repl_is_bytes_valued_resource(), or `swp`'s documented MB
* special case -- the SAME classification repl_format_numeric_resource_value()
* already applies to the Load Metrics section) renders BOTH `avail` and
* `used` through that same formatter, respecting `mode` -- so
* `used=87960930222080.00` (a live-reported bug, 2026-09-11) instead
* becomes e.g. `used=87960930222080.000000 (80.00 TB)` under the default
* "both" mode, matching how maxmem/maxswap/maxtmp already render alongside
* it. Every other consumable resource (slots, licenses, ...) keeps the
* existing pinned "%.2f"/"%.2f" shape unchanged -- bug-1601/1613/1727's own
* tests assert this exact format and must not regress.
*/
static void repl_format_consumable_value_and_used(xce_repl_units_mode_t mode,
const char* rname,
double avail, double used,
char* value_buf,
size_t value_bufsz,
char* used_buf,
size_t used_bufsz)
{
bool is_swp = (rname != NULL) && (strcmp(rname, "swp") == 0);
if (repl_is_bytes_valued_resource(rname) || is_swp) {
repl_format_numeric_resource_value(mode, rname, avail, value_buf,
value_bufsz);
repl_format_numeric_resource_value(mode, rname, used, used_buf,
used_bufsz);
return;
}
snprintf(value_buf, value_bufsz, "%.2f", avail);
snprintf(used_buf, used_bufsz, "%.2f", used);
}
/* repl_current_units_mode - sim->repl_state may legitimately be NULL (a few
* white-box tests dispatch `info worker`/`info resource` without ever
* calling repl_state_ensure()); default to the same XCE_REPL_UNITS_BOTH
* repl_state_ensure() itself explicitly assigns (Req 6.3), rather than
* leaving repl_format_numeric_resource_value() to guess. */
static xce_repl_units_mode_t
repl_current_units_mode(xce_sched_sim_driver_t* sim)
{
return (sim->repl_state != NULL) ? sim->repl_state->units_mode
: XCE_REPL_UNITS_BOTH;
}
/* translate_units_mode / repl_format_numeric_resource_value /
* repl_format_consumable_value_and_used / repl_current_units_mode moved to
* repl_fmt.c (scheduler-sim-decision-trace spec, task 1.2, Req 7.7) so the
* trace renderer can share them; declared in repl_internal.h. */
/* repl_append_u64 - grows `*arr`/`*n` by one element, the same exact-size
* per-append realloc idiom repl_fmt.c's repl_jobid_group_add_idx()

View file

@ -3,6 +3,7 @@
#define _REPL_INTERNAL_H_ 1
#include "repl.h"
#include "utils.h" /* xce_units_mode_t (translate_units_mode) */
/*
* repl_internal.h - private state and cross-file declarations shared by
@ -354,6 +355,32 @@ extern void repl_format_ts_human(uint64_t ns, char* buf, size_t bufsz);
extern bool repl_is_bytes_valued_resource(const char* name);
extern void repl_format_bytes_human(double bytes, char* buf, size_t bufsz);
/*
* Units-mode value formatting (repl_fmt.c, moved from repl_info.c by the
* scheduler-sim-decision-trace spec, task 1.2, Req 7.7, behaviour
* unchanged) -- shared by repl_info.c's `info worker`/`info resource`/
* `info limit` and the trace renderer so both format values identically:
* - translate_units_mode: maps xce_repl_units_mode_t to the shared
* library's xce_units_mode_t (utils.h).
* - repl_format_numeric_resource_value: the text after "value=" for a
* numeric resource; bytes-valued names and `swp` (MB, converted to
* bytes) render per `mode`, others as "%.0f" (whole) or "%.2f".
* - repl_format_consumable_value_and_used: a consumable's value=/used=
* pair; bytes-valued names as above, others the pinned "%.2f"/"%.2f".
* - repl_current_units_mode: sim->repl_state->units_mode, or
* XCE_REPL_UNITS_BOTH when no REPL state exists yet.
*/
extern xce_units_mode_t translate_units_mode(xce_repl_units_mode_t mode);
extern void repl_format_numeric_resource_value(xce_repl_units_mode_t mode,
const char* rname,
double value, char* buf,
size_t bufsz);
extern void repl_format_consumable_value_and_used(
xce_repl_units_mode_t mode, const char* rname, double avail, double used,
char* value_buf, size_t value_bufsz, char* used_buf, size_t used_bufsz);
extern xce_repl_units_mode_t
repl_current_units_mode(xce_sched_sim_driver_t* sim);
/* repl_command_loop (repl.c) - the blocking prompt loop. Breakpoint hits
* (repl_break.c's xce_repl_on_event()/on_begin_round()/on_decision()) call
* back into it directly, the same re-entry xce_repl_post_session_check()

View file

@ -89,6 +89,17 @@
#include <unity/unity.h>
/* scheduler-sim-decision-trace spec, task 1.2 (Req 7.7): the units-mode
* translation, numeric/consumable value formatters and current-units
* lookup moved from repl_info.c (file-static) to repl_fmt.c, declared in
* repl_internal.h so other REPL files (repl_trace_render.c) can reuse them.
* Included here -- a translation unit separate from repl_info.c/repl_fmt.c,
* which this target links as objects -- to prove the declarations are
* reachable from another file. */
#include "repl_internal.h"
#include "sim.h"
#include "utils.h"
/*
* repl_is_bytes_valued_resource / repl_format_bytes_human (task 7.2, NOT YET
* IMPLEMENTED -- see this file's own header comment above for why these are
@ -311,6 +322,90 @@ static void test_swp_value_converted_mb_to_bytes_renders_on_same_adaptive_scale(
"scale: PASS\n");
}
/* ---------------------------------------------------------------------- */
/* scheduler-sim-decision-trace task 1.2 (Req 7.7): the shared units/value */
/* formatting helpers are callable from a separate translation unit through */
/* repl_internal.h, with their pre-move behaviour unchanged. */
/* ---------------------------------------------------------------------- */
static void test_shared_translate_units_mode_maps_each_repl_mode(void)
{
TEST_ASSERT_EQUAL_INT(XCE_UNITS_RAW,
translate_units_mode(XCE_REPL_UNITS_RAW));
TEST_ASSERT_EQUAL_INT(XCE_UNITS_HUMAN,
translate_units_mode(XCE_REPL_UNITS_HUMAN));
TEST_ASSERT_EQUAL_INT(XCE_UNITS_BOTH,
translate_units_mode(XCE_REPL_UNITS_BOTH));
}
static void test_shared_numeric_resource_value_formats_per_units_mode(void)
{
char buf[128];
char want[128];
/* Unitless: whole number has no ".00"; fraction keeps two decimals. */
repl_format_numeric_resource_value(XCE_REPL_UNITS_BOTH, "r1m", 3.0, buf,
sizeof(buf));
TEST_ASSERT_EQUAL_STRING("3", buf);
repl_format_numeric_resource_value(XCE_REPL_UNITS_RAW, "r1m", 2.5, buf,
sizeof(buf));
TEST_ASSERT_EQUAL_STRING("2.50", buf);
/* Bytes-valued: delegated to xce_format_units_value() per mode. */
repl_format_numeric_resource_value(XCE_REPL_UNITS_HUMAN, "mem", 1024.0,
buf, sizeof(buf));
TEST_ASSERT_EQUAL_STRING("1.00 KB", buf);
repl_format_numeric_resource_value(XCE_REPL_UNITS_RAW, "mem", 1024.0, buf,
sizeof(buf));
TEST_ASSERT_EQUAL_STRING("1024.000000", buf);
repl_format_numeric_resource_value(XCE_REPL_UNITS_BOTH, "mem", 1024.0,
buf, sizeof(buf));
TEST_ASSERT_EQUAL_STRING("1024.000000 (1.00 KB)", buf);
/* swp: megabytes on the wire, converted to bytes before formatting. */
repl_format_numeric_resource_value(XCE_REPL_UNITS_BOTH, "swp", 2.0, buf,
sizeof(buf));
xce_format_units_value(XCE_UNITS_BOTH, 2.0 * 1024.0 * 1024.0, want,
sizeof(want));
TEST_ASSERT_EQUAL_STRING(want, buf);
}
static void test_shared_consumable_value_and_used_formats(void)
{
char vbuf[128];
char ubuf[128];
/* Non-bytes consumable keeps the pinned "%.2f"/"%.2f" shape. */
repl_format_consumable_value_and_used(XCE_REPL_UNITS_BOTH, "slots", 4.0,
1.0, vbuf, sizeof(vbuf), ubuf,
sizeof(ubuf));
TEST_ASSERT_EQUAL_STRING("4.00", vbuf);
TEST_ASSERT_EQUAL_STRING("1.00", ubuf);
/* Bytes-valued consumable formats both sides per the units mode. */
repl_format_consumable_value_and_used(XCE_REPL_UNITS_HUMAN, "mem",
2048.0, 1024.0, vbuf, sizeof(vbuf),
ubuf, sizeof(ubuf));
TEST_ASSERT_EQUAL_STRING("2.00 KB", vbuf);
TEST_ASSERT_EQUAL_STRING("1.00 KB", ubuf);
}
static void test_shared_current_units_mode_reads_state_or_defaults(void)
{
xce_sched_sim_driver_t sim;
xce_sched_repl_state_t st;
memset(&sim, 0, sizeof(sim));
memset(&st, 0, sizeof(st));
/* No REPL state: falls back to the documented BOTH default. */
TEST_ASSERT_EQUAL_INT(XCE_REPL_UNITS_BOTH, repl_current_units_mode(&sim));
st.units_mode = XCE_REPL_UNITS_RAW;
sim.repl_state = &st;
TEST_ASSERT_EQUAL_INT(XCE_REPL_UNITS_RAW, repl_current_units_mode(&sim));
}
int main(void)
{
UNITY_BEGIN();
@ -322,6 +417,10 @@ int main(void)
RUN_TEST(test_format_bytes_human_fractional_values_render_two_decimals);
RUN_TEST(
test_swp_value_converted_mb_to_bytes_renders_on_same_adaptive_scale);
RUN_TEST(test_shared_translate_units_mode_maps_each_repl_mode);
RUN_TEST(test_shared_numeric_resource_value_formats_per_units_mode);
RUN_TEST(test_shared_consumable_value_and_used_formats);
RUN_TEST(test_shared_current_units_mode_reads_state_or_defaults);
return UNITY_END();
}