Update session-tools to 1.1.0 - #4
Draft
akcd1 wants to merge 15 commits into
Draft
Conversation
Port the personal ~/.claude Slurm sizing system into two session-tools skills. slurm-sizing consults a measured usage table before a job is submitted and logs what each job actually ran; slurm-digest merges a weekly usage digest into that table. Enforcement is the skill description (trigger-heavy, wrapper-aware) since a plugin cannot write to a user's CLAUDE.md. User data (table, log, digest archive) is not shipped — it lives under the user's own ~/.claude/slurm-sizing/ and is located via a config contract (reference/config.md) with no default for digest_cluster, since guessing the wrong cluster silently corrupts sizing advice (job IDs are not unique across clusters). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Jjcp9DNrdrmFxDadgCFNrC
…document digest provenance Whole-branch review fixes before PR. Critical: - The sizing table's schema existed nowhere; bootstrap invented one. reference/config.md now carries the literal nine-column header and the "How to read a row" prose, so bootstrap is deterministic and later merges write into known columns. - rec_mem could recompute BELOW the row's recorded peak: it was derived from this week's digest while the only stored peak was the 1-decimal peak_G the recompute forbids using. Added a full-precision peak_MB column as the authoritative running max; peak_G is now its display rounding, and rec_mem is a pure function of stored peak_MB, so it cannot decrease while peak_MB is unchanged. - Documented where a weekly digest comes from (emailed weekly to Slurm users at this institute) and that both skills are inert until the first merge. Important: - Bootstrap now runs the cluster query and offers the exact string rather than asking in free text; the log's cluster column must carry that same exact string. - One identical "determine the local cluster" procedure in all three files; hostname is dropped from the text and from allowed-tools (it names a node, not a cluster) and Bash(scontrol *) is added so the documented fallback is executable. - Replaced the unimplementable shell join (whitespace-split free-text scope, unsorted input, single key field) with one tab-delimited awk lookup on the composite (cluster, jobid) key. - Documented how the IO-BOUND flag is SET (in-process evidence only); made the CPUPct note additive and exempted IO-BOUND rows from the CPU step so the flag cannot be erased. - The digest's week-ending date is now a required argument; ask if missing, never assume today (two pastes of one digest could otherwise both pass the duplicate check). - Added a decline branch: "enabled": false silences both skills permanently. - Deleted an unreachable zero-match escape hatch that contradicted the line above it. - README: invocation namespace is the plugin, /session-tools:<skill>, not /whitehead:. Minor: sample config uses placeholders, not real values; supporting files cited as markdown links; (M) documented as a unit annotation, not required header text; dropped unused Bash(cat *)/Bash(ls *); reworded the circular "config may relocate itself" line.
…IO-BOUND exception Three residuals from the scoped re-review. - N1: restore an executable pre-plugin layout probe. Dropping Bash(ls *) left bootstrap unable to test ~/.claude/slurm-digests/, which is a DIRECTORY that Read cannot distinguish from a missing path. Added Bash(test *) to slurm-sizing's allowed-tools and made the probe explicit: test -f on the two files, test -d on the directory. slurm-digest is unaffected (its mkdir -p is idempotent). - N4: the migration seed peak_G * 1024 was NOT "never lower". peak_G rounds to nearest, so the bare seed can sit up to 51.2 MB below the true peak: a true peak of 16394 MB displays as 16.0, giving a seeded rec_mem of 32G where 36G is correct -- a 4G drop in exactly the direction peak_MB exists to prevent. Corrected to (peak_G + 0.05) * 1024, the top of the rounding interval, and the false claim is gone from both files that carried it. - N5: "peak_MB only ever rises" / "never lower peak_MB" contradicted the IO-BOUND procedure, which lowers it on purpose when the sacct figure was inflated. Both statements now scope the guarantee to merging and name the human-set IO-BOUND correction as the single exception. Also drops a dangling self-reference in Step 2 to ReqMem(M)/UsedMem(M) forms that no longer appear elsewhere in the file after the M3 rewrite.
…, complete allowed-tools Three defects from a sandbox end-to-end execution of slurm-digest by a fresh agent, plus two minors from the same run. - F1: rec_cpu was undefined when one job name has several rows in a digest. A real name appearing 9 times spanned CPUPct 1.55..49.08; pairing the CPU figures with the peak-MEMORY row gave rec_cpu 2 where the peak-CPU row gives 12 -- a 6x spread with nothing in the text to choose between them, and the low answer would serialise a job that has used ~8 cores. Ruled: take the row with the MAXIMUM CPUPct and pair it with that same row's ReqCPU. The peak-memory and peak-CPU runs are generally different runs; under-provisioning CPU is the harmful direction; and this matches the running-max the memory side already uses. Stated in slurm-digest Step 9 and in config.md. - F2: digest_user was written into the config from $USER without ever being shown. config.md argues this must be a config key precisely because the Slurm account and login name can differ -- and for such a user the wrong value filters out every one of their own rows, merging nothing with no error. Bootstrap now shows the value, names $USER as its source, states it must match the digest's User column, and offers correction. - F3: sixth instance of the allowed-tools class. slurm-digest runs the same bootstrap as slurm-sizing but did not get the Bash(test *) added there for the pre-plugin layout probe. Added, the delegation to that probe is now explicit in the body, and both skills were re-audited command-by-command. Minors: the parse step's "any bare shell or interpreter name" is now an enumerated, explicitly extensible list; and a blank CPUPct is defined as absence of evidence (excluded from the maximum), not as zero, which would drive rec_cpu to the floor on no data.
…lise digest array IDs
Seven findings from a second sandbox execution test (F7 needs no code change).
Important:
- F1: Step 10 ranked by "(ReqMem - rec_mem) x n", but ReqMem is megabytes and
rec_mem is a STRING in gigabytes with a G suffix and often a >= prefix --
204800.00 - ">= 28G" is not arithmetic. Now states the parse and both unit
conversions explicitly and ranks by waste_GB. This ranking decides which jobs
the user is asked to scope, which is the only path from >= to a real
recommendation.
- F4: Step 6 assumed digest JobIDs are bare base IDs. Real digests carry
array-task IDs, so the _<task> normalisation written for the sacct side now
applies to the digest side too, stated as "suffixes appear on BOTH sides". The
array bullet's contrary claim is corrected. Previously harmless only because
sacct returned nothing for every fixture ID.
- F5: Step 12's "reservation delta" had no formula, units or scope. Defined as
sum(ReqMem_GB - rec_mem_GB) over rows merged this week, reported as TWO
subtotals -- actionable (no >= prefix) and pending-scope (>= rows). One merged
figure would contradict the same step's statement that >= rows are not
actionable.
Minor:
- F2: a first-time CPU note is exactly "CPUPct <value>"; on a populated cell it
is appended after "; ", and appending never rewrites what is there.
- F3: an unexpanded shell variable in a job name (${...} or a bare $) is NOT
skipped -- it is a real job with real measurements -- but is reported as a
probable quoting bug, since the job name is the merge key and an unexpanded
name never aggregates with its intended siblings.
- F6: UsedCPU holds CPU time, not a core count, and is deliberately unused.
F7 (three of Step 8's four scope branches untested) is a test-coverage gap being
addressed by a seeded execution test; Step 8 is unchanged, as ruled.
Also replaces three real cluster job IDs that entered the F4 text from the test
fixture with the invented IDs already used in that step.
…default 0.30)
A second digest at ~45% overall memory utilisation -- versus the ~8% one this
system was calibrated on -- exposed a policy defect. The flat 2x margin
overshoots once utilisation is high: 6 of 27 job names got a rec_mem HIGHER than
what they had requested, e.g. a 512G request that peaked at 341.8G (well sized)
was told to ask for 684G. Because an unknown-scope row "may justify raising a
request", those are actionable, so the tool would push already-well-sized jobs
upward -- spending queue time and per-user memory cap, which is the exact harm it
exists to prevent, inverted.
rec_mem = roundup_to(mem_round_gb, max(mem_floor_gb, peak_GB * (1 + headroom_frac)))
- policy.margin_multiplier is REMOVED; policy.headroom_frac (default 0.30)
replaces it. mem_floor_gb (8) and mem_round_gb (4, still rounding UP) keep
their meanings.
- Measured: raises-above-request 6/27 -> 1/27 on the high-utilisation digest,
unchanged (2/65) on the low-utilisation one. 341.8G peak: 684G -> 448G.
Three things a future editor would get wrong, now stated in both files:
1. peak_GB * 1.30 IS a 1.3x multiplier. The two forms differ in size, not in
kind. Said plainly so nobody "restores" 2x believing they differ.
2. The margin applies to a running max -- 30% above the worst reading ever seen
for that job name, not above a typical week. That is what makes it defensible.
3. Recommending above the request is still possible and still intended; what was
removed is the systematic overshoot, not the ability to flag real
under-provisioning.
Every worked example that hardcoded a 2x result is recomputed: the Step 8 example
(13.2G -> 20G, not 28G), the monotonicity example (14963.82MB -> 20G, and 8G if
wrongly recomputed from a 5000MB week), and the migration-seed example, which had
to be replaced outright -- under 1.3x the old 16394MB case no longer crosses a
4G rounding boundary, so it demonstrated nothing. The new 9462MB case does
(16G true vs 12G bare-seeded).
rec_cpu, the scope rules, the duplicate gate, peak_MB monotonicity and the
enrichment procedure are untouched.
…venance A third execution test with a seeded submission log made the scope-join match for the first time, exposing a Critical defect and five smaller ones. C1 (CRITICAL): the scope column was recorded but never compared, so a subset measurement was presented as an actionable recommendation for a full run. A row measured at "5 of 100 units" carried no >= and read as usable, sizing a full 100-unit run from 5 units of evidence -- the exact under-provisioning the asymmetric rule exists to prevent. The root error was treating "we know what this measurement covered" as "this is safe to size a future run from"; those are different claims. The comparison also lived in the merge step, which is the one place it cannot be evaluated, since at merge time nobody knows what will be run next. Moved it to the point of use: slurm-sizing §2 gains "The scope check", which states the scope prominently and compares it to the intended run -- same scope is usable as-is, LARGER must not be sized down and is treated as a lower bound, SMALLER is safe but generous, undeterminable means ask. C2: scope provenance was undefined when a job name's rows disagree. Adopted: the scope that governs a row is the scope of the run that SET peak_MB, since the recommendation is derived from the peak. If that run has no declared scope the row stays >= regardless of what siblings declared, and a displaced scope is preserved in notes rather than discarded. C3: the reservation delta can go negative; now defined as under-provisioning relative to the job's own observed peak, never to be presented as a saving, absolute-valued or clamped. C4: row order was unspecified, so two merges of the same data could produce different files. Specified: sort by job_name ascending, every write. C5: the asymmetric rule had no first-ever-row branch. Stated as the same provenance rule with no history. C6: the scope prompt had no non-interactive behaviour. Unanswered names stay unknown, the merge completes, and a scope is never invented to fill the gap. rec_cpu, peak_MB monotonicity, headroom_frac, the duplicate gate and the enrichment procedure are untouched.
…xplain pinned rows Fourth execution test -- the first to run slurm-sizing itself, against a table seeded to force every branch. It CONFIRMED the C1 scope-check fix: a row recorded at "5 of 100 samples (SUBSET)", asked to size a 100-sample run, was correctly treated as a lower bound instead of certified at its table value. Exact-match, unknown-scope, IO-BOUND and absent-row branches all behaved correctly, the last two for the first time. Three documentation findings. - F1: "do not size down" never said what to request INSTEAD, so a correct refusal left the researcher with no number and moved the guess somewhere invisible. §2's scope check now states plainly that the table cannot answer this -- it holds a measurement of a different-sized run -- and gives three ordered routes: scale by a known parameter and LABEL THE RESULT AN ESTIMATE, or start from a larger run that previously succeeded, or measure first and merge that digest. Plus: never present the table's number as if it covered the larger run, not even silently as a floor, and record the scope at submission either way. - F2: the ask-the-user points were not self-enforcing -- an unattended agent proceeded past two of them. Both skills gain a "Hard stops" section: a HARD STOP is a halt, not a suggestion, and running unattended is not permission to continue. Marked at the undeterminable-scope branch, the missing date, the already-archived digest, the first multi-user encounter, and every bootstrap question. Step 10's scope prompt is documented as deliberately NOT a hard stop, since its unanswered outcome is the conservative one. - F3: an IO-BOUND row's rec_mem does not satisfy the formula, and nothing said it shouldn't -- the tester reasonably reported it as a data-integrity gap. Both files now state that a pinned row's rec_mem is hand-set from in-process evidence and deliberately NOT derived from peak_MB, that a mismatch is expected, and that "correcting" it discards the only evidence the row has. PR body updated: four execution tests and what each found, why only a seeded-fixture test could reach the Critical, and an honest unverified list (wrapper-script triggering, SubmitLine retention, and the absence of a scaling model when a run exceeds its measured scope).
akcd1
marked this pull request as draft
August 4, 2026 19:00
A live install check on a second machine found that the (cluster, jobid) join key
does not actually separate clusters. Two hosts at one institute report the SAME
ClusterName while having different controllers, different accounting databases
and independent job-ID spaces: the same job number named two different jobs,
submitted a year apart, depending on which host you asked. Because ~/.claude is
shared NFS storage, one submission log receives rows from both, tagged
identically -- precisely the corruption the key was introduced to prevent.
A Slurm ClusterName is a locally chosen label with no uniqueness guarantee across
the clusters one person can reach, so a bare name is not a safe key.
RULING implemented: cluster identity is now the composed string
<ClusterName>@<SlurmctldHost>, both fields read from `scontrol show config`. It
is what digest_cluster holds, what the log's cluster column carries, and what the
local-vs-digest gate compares. Two clusters with independent job-ID spaces
necessarily have distinct controllers, so the suffix separates them; where
ClusterName is already unique it changes nothing but makes the identity explicit.
Applied across all three files -- the config key, the shared determination
procedure, the bootstrap step, the join-key discussion, slurm-sizing's §1 gate /
§4 log-append / §5 rationale, and slurm-digest's lookup step and enrichment
guard -- and checked against each other. Bootstrap derives and DISPLAYS the
composed string, showing both source fields so the user can tell which machine
they are on.
Legacy bare names are treated as UNVERIFIED, not upgraded: such a value may
belong to any cluster with that name, so it cannot satisfy a scope match (the row
stays >=), and it is never rewritten into a composed identity -- no evidence
survives about which cluster wrote it, so either would manufacture provenance.
The count is reported so a drop in matches reads as the transition rather than a
malfunction.
Also: sacctmgr is no longer instructed anywhere now that scontrol supplies both
fields, so Bash(sacctmgr *) is removed from BOTH skills. Per the lesson from the
last permission removal, both bodies were then re-audited command by command
rather than only the file being edited.
Shipped text describes the finding by shape ("two clusters reachable from one
shared home reported the same ClusterName") and uses invented identities
(alpha@ctl-1, alpha@ctl-2); no real hostnames appear.
… fallback chain The cluster identity is now <ClusterName>@<short hostname> (e.g. alpha@login-1) rather than <ClusterName>@<SlurmctldHost>. The controller name is invisible to the people using this; the submit hostname is what they say and what already appears in their logs. The trade-off is stated in all three files rather than hidden: a controller identifies the cluster, a hostname identifies where the command was typed, so a cluster with several submit hosts acquires several identities. What happens then is benign and visible -- the second host's identity will not equal the configured digest_cluster, so the gate stands down with a message instead of silently mis-keying rows. A site with several login nodes sets digest_cluster per host or standardises on one submit host. ClusterName is now derived by a three-step chain so an unreadable scontrol degrades the identity instead of making the skill inert: scontrol show config; failing that sacctmgr -n -P list cluster format=Cluster; failing both, the bare short hostname is used as the whole identity and the run says so in its report. Legacy bare cluster names are unchanged: unverified, never rewritten, cannot satisfy a scope match. The one place that exclusion is no longer automatic (a hostname-only fallback identity carries no @) is called out where it lived, in slurm-digest Step 5. allowed-tools: Bash(sacctmgr *) restored to both skills -- the fallback chain makes it instructed again -- and Bash(hostname *) added to both, since the suffix is now read with hostname -s. Both bodies re-audited command by command; every instructed command is covered and nothing declared is unused.
…op compound probes
Inside an allocation the hostname is the compute node, so the identity was
wrong and the skill silently stood down for the whole session. Measured on a
real cluster: on the login node `hostname -s` = login-1 and SLURM_SUBMIT_HOST
is unset; inside `srun --pty` on the same cluster `hostname -s` = the compute
node while SLURM_SUBMIT_HOST is still login-1. Interactive srun --pty sessions
are ordinary working practice, not a corner case, so every such session minted
<ClusterName>@<compute-node>, matched no configured digest_cluster, and stood
the skill down until the session ended -- looking broken rather than degraded.
The host component is now ${SLURM_SUBMIT_HOST:-$(hostname -s)}: Slurm sets that
variable exactly when hostname is the wrong answer and leaves it unset on a
login node, so one expression is right in both contexts. Applied in all three
files, including the fallback chain's step 3 (bare host as the whole identity),
which now resolves the host the same way. The reason is documented at each site
so a future reader does not "simplify" it back to a bare hostname.
allowed-tools: Bash(echo *) added to both skills -- reading the variable means
printing it. The bootstrap layout probe no longer uses `test -f ... && echo`;
it is three bare `test` commands whose exit status is read, so it stays inside
Bash(test *) instead of relying on a prefix pattern to cover a compound. Both
skill bodies and config.md re-audited command by command; every instructed
command is covered and nothing declared is unused.
…ied the host SLURM_SUBMIT_HOST and `hostname -s` can disagree in form: some sites record an FQDN in the variable while `hostname -s` returns the short name. Normalising only one of them would make the login-node identity and the in-allocation identity differ by a domain suffix, standing the gate down inside every allocation -- the exact failure SLURM_SUBMIT_HOST was introduced to remove, on sites we cannot test. The rule is now: strip any domain suffix from whichever source supplied the value, taking the first dot-separated field, so SLURM_SUBMIT_HOST= login-1.example.edu and `hostname -s` = login-1 both yield login-1. Applied at every derivation site in all three files, including each fallback-chain step 3. No command changed -- this is a transformation of a value already being read -- so both allowed-tools lists are untouched.
…probe rationale
The layout probes were mandated as three bare `test` commands on the grounds that
"a compound command is not reliably covered by the Bash(test *) permission this
skill declares" — while the same documents prescribed `scontrol show config |
grep -E '^ClusterName'` and `echo "${SLURM_SUBMIT_HOST:-$(hostname -s)}"`. The
stated objection applied to those two just as much, so the rule contradicted
itself and one of the two had to be wrong.
The pipe was the wrong target. A pipe whose halves are separately declared is
covered — `Bash(scontrol *)` together with `Bash(grep *)` — so it stays. What
cannot be checked before it runs is command substitution, so the submit-host
probe becomes two bare commands: `echo "$SLURM_SUBMIT_HOST"`, then `hostname -s`
only if that printed nothing. Same resolution order, no `$(...)`.
The bare-`test` rule is kept, but on its real justification rather than the
permission story: `&&` short-circuits, so the later probes never run, and a
chain collapses three independent answers into one exit status that cannot say
which path is missing.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`claude plugin update` decides whether to refresh an install by comparing the version string, not the content. The probe fix in 313ae7d therefore reached no installed copy: update reported "already at the latest version (1.1.0)" and left the recorded commit sha pointing at the pre-fix tree. Bump so the fix actually propagates. Any content-only change to a skill needs one of these; without it the marketplace source moves ahead while every install silently keeps serving the old text — the same staleness that left slurm-sizing and slurm-digest missing from an install pinned at 1.0.0. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…rsing traps Step 1's missing-date hard stop asked the user because defaulting to today breaks idempotency: the same digest pasted on two different days would archive under two filenames, pass the duplicate check both times, and double the n column. Deriving the date from the digest's own job IDs via sacct preserves that exact property while removing the question — today's date is a property of when you pasted (changes every time), while a derived date is a property of what you pasted (stable across any future paste of the same digest). It's also more reliable than a human retyping a date out of an email. The hard stop is retained for the only case that still needs it: sacct absent/erroring, no job ID resolving, or every row carrying an Unknown end. Step 1 also now checks row overlap against the most recent archived digest, since a filename check alone can't see a digest re-sent under a different date with overlapping rows. Step 6 documents two parsing traps found in a real run: sacct's SubmitLine can embed real newlines (an srun ... python -c multi-line script splits one record across several output lines, truncating a line-by-line parse — observed live on a *_mn_verify row), so parsing must be continuation-aware, keyed on a new-record regex anchored to JobID rather than line boundaries. Second, filtering step rows with a bare grep -v '\.' is wrong because submit lines are full of dots (paths, versions) — it's the JobID field that must be tested for a dot, not the whole line. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds two skills to
session-tools(1.0.0 → 1.1.0), porting a working personal~/.claude/Slurm-sizing system into something any user of this marketplace can install:
slurm-sizing— consults a per-user table of measured--mem/--cpus-per-taskusagebefore a Slurm job is submitted, and recommends sizing from that instead of habit. After
submission it logs what the job actually ran (
cluster,jobid,submitted,job_name,scope,cwd) so a later digest merge can attach real workload scope to the measurement.slurm-digest— user-invoked (/session-tools:slurm-digest), merges a weekly Slurm usagedigest into the table
slurm-sizingreads.Where the digest comes from
Neither skill generates a digest, and neither shells out to build one. The institute already
emails a Slurm usage digest, weekly, to users who ran jobs — requested vs. used memory and CPU
per job. That email is the input: you paste its table into
/session-tools:slurm-digestalongwith the week-ending date it covers.
The direct consequence, stated in the README and in both skills: the system is inert until the
first digest is merged. A fresh install has an empty table, so every job is "unknown" — which
slurm-sizingreports as a reason to measure, never as licence to guess. There is nosacct-based substitute;sacctis used later only to annotate rows a digest already contains.What the table looks like
The sizing table (
~/.claude/slurm-sizing/table.md, user-owned, not shipped) is a nine-columnmarkdown table. The literal header and the "How to read a row" prose are specified in
plugins/session-tools/skills/slurm-sizing/reference/config.mdso that bootstrap isdeterministic rather than invented per user:
peak_MBis the authoritative full-precision running max in megabytes;peak_Gis a displayrounding of it;
rec_memis a pure function of the storedpeak_MBand thepolicyvalues, soit cannot fall below the row's own recorded peak when a later week happens to run a smaller
input.
notesis additive free text and carries the load-bearingIO-BOUNDflag.Note for people who already have
session-toolsinstalledThis is an update, not a new plugin, and it changes behaviour for existing users.
self-assessis invoked explicitly;
slurm-sizingis written to load on its own, before and afteranything that submits a cluster job. Updating to 1.1.0 therefore introduces pre-submission
behaviour that was not there before. Bootstrap offers a decline branch — answering "not
applicable / I have no weekly digest" writes
{"enabled": false}to~/.claude/slurm-sizing/config.json, and both skills then stay silent permanently. This iscalled out in the README too.
Invocation namespace
Skills are invoked as
/<plugin-name>:<skill-name>— the namespace is the plugin, not themarketplace. So
/session-tools:slurm-digest, not/whitehead:slurm-digest. The READMEpreviously documented the marketplace form; this PR corrects it. That matters here because
slurm-digest's only entry point is that slash command — a reader following the old READMEwould type a command that does nothing.
User data is not shipped
The sizing table, submission log, and raw digest archive all live under the installing user's own
~/.claude/slurm-sizing/and are not part of this plugin. A fresh install starts withnothing — both skills bootstrap the config, an empty table, and an empty log on first use and
report exactly what they created.
A grep for personal identifiers (usernames, home/lab paths, project names) across
plugins/session-tools/,README.mdand.claude-plugin/returns zero matches, includingfor the author's own cluster name — the sample config now ships
"<your-cluster-name>"/"<your-slurm-account>"placeholders per the repo's PR checklist. The only cluster name anywherein the repo is the pre-existing, already-public
fry-python-toolplugin's own description, whichthis PR does not touch.
Config contract
Both skills read
~/.claude/slurm-sizing/config.json(full schema inplugins/session-tools/skills/slurm-sizing/reference/config.md):enabledtruefalsesilences both skills permanently (see above).digest_clusterdigest_user$USERtable/log/archive~/.claude/slurm-sizing/{table.md,jobs.tsv,digests}policy.headroom_frac0.30rec_mem = stored peak_GB × (1 + headroom_frac).policy.mem_floor_gb/policy.mem_round_gb8/4policy.cpu_headroom/policy.cpu_floor1.5/2rec_cpuis capped at the job's ownReqCPU.digest_clusterdeliberately has no default. Guessing it wrong is the one failure mode thatsilently corrupts every recommendation downstream: Slurm job IDs are not unique across clusters,
so sizing data built from the wrong cluster can attach to a same-numbered job elsewhere and be
simply wrong, with no error to notice. It is also exact-matched — against the local cluster
name and against every log row's
clustercolumn — so bootstrap does not ask for it in freetext: it runs
sacctmgr -n -P list cluster format=Cluster(falling back toscontrol show config | grep ClusterName), shows the exact string, and offers it as the answer.hostnameis explicitly not used for this anywhere — it names a node, not a cluster.Multi-user digests are handled explicitly: first encounter with other users' rows halts and asks
whether to filter to
digest_user; the answer is remembered in config (multi_user_digest) sothe merge doesn't halt every week thereafter; declining leaves the config unset so it re-asks
rather than silently merging another person's peaks into your table.
Safety properties (do not regress these in future edits)
These are structural guarantees of the merge, as distinct from the
policynumbers, which aretunable defaults a site may change in config (
headroom_frac0.30,mem_floor_gb8,mem_round_gb4,cpu_headroom1.5,cpu_floor2 — reviewed defaults, not law; lowering themtrades OOM risk for queue priority and should be a deliberate config change, not a file edit).
The memory margin is
peak_GB × (1 + headroom_frac), i.e. 1.3x the running max across everydigest merged for that job name. It replaced a flat
2x, which had been calibrated on jobs at3–15% memory utilisation and, on a digest of jobs near 45% utilisation, recommended more than the
job had requested for 6 of 27 names — pushing already-well-sized jobs upward. The fractional form
is arithmetically still a multiplier; it is simply a smaller one.
(cluster, jobid), neverjobidalone — job IDs collide across clusters.Implemented as one tab-delimited
awklookup that tests cluster equality before jobidmembership.
>=prefix) — may raise a recommendation, never lower one.peak_MBis a running max andrec_memis a pure function of it — sorec_memcan neverfall below the row's own recorded peak, whatever this week's digest says. Rounding is always
up; a margin that rounds down is not a margin.
sacctMaxRSSon I/O-heavy jobs is not trusted — it's kernel page cache charged to thecgroup, not demand. Such rows are hand-pinned
IO-BOUNDfrom in-process evidence(
getrusage/psutil-style sampling) and are left entirely untouched by the merge — peak,recommendations and notes alike, so the flag cannot be erased by a note rewrite.
inflates
n. The date is a required argument, never defaulted to today.sacctenrichment queries one field at a time, splitting on the first|only — never acombined multi-field query or
--delimiter.rec_cpuis capped at the job's ownReqCPU— without the cap the formula can recommendmore CPUs than were requested, defeating the tool's purpose.
Why enforcement lives in the skill description — and the open question
A plugin has no mechanism to append to an installing user's
CLAUDE.md. The equivalent personalsystem worked by a
CLAUDE.mdsection telling Claude when to consult it; ported into a plugin,that instruction has to live somewhere Claude reads unprompted before the plugin is even
mentioned — the skill's
descriptionfrontmatter field, loaded into every session's skill index,is the only such surface. Its trigger language is deliberately wrapper-aware (covers
--backend slurm-style launchers,submit_*.sh, any#SBATCH-bearing script, and "Submittedbatch job" in output) rather than keying only on the literal
sbatch/srun/sallocverbs, sincemost real submissions go through a wrapper.
State this as design intent, not as a guarantee. Skill loading is model discretion; there is
no hook backing it up, and it has not been empirically verified that the description reliably
fires on indirect (wrapper-script) submission. That is the one open question for maintainers to
watch after install, and the reason the README says "written to load on its own" rather than
"loads automatically". If it proves unreliable in practice, the fix is a
PreToolUsehook, whichis out of scope here.
Testing
No automated tests (markdown skills, no code).
The repo's mandated pre-PR check was run —
claude --plugin-dir ./plugins/session-tools—and all three skills register under the plugin namespace:
session-tools:self-assess,session-tools:slurm-digest,session-tools:slurm-sizing.Also verified statically: both JSON manifests parse; skill directory layout matches the existing
self-assessskill; YAML frontmatter has all four convention keys withnamematching eachdirectory;
reference/config.mdresolves from both citing skills; each skill'sallowed-toolscovers every command its body instructs and contains nothing unused; the nine-column table schema
in
reference/config.mdmatches every column reference in both skills; the personal-identifiergrep returns zero matches.
sacctenrichment mechanics were verified against livesacctoutput — specifically that acombined
--format=JobID,SubmitLine,WorkDir --parsable2query interleaves fields whenever asubmit line contains a
|, that step rows (.batch/.extern) return emptySubmitLine, thatarray tasks return one row per task with an identical
SubmitLine, and thatsaccton onecluster does not resolve another cluster's job IDs.
Four execution tests
Beyond static checks, four independent execution tests were run, each by a fresh agent with no
knowledge of the build, against real digest data. They are the reason most of what follows exists.
slurm-digestend to end, three times (fresh merge, repeated digest, a week where a job peaked lower)rec_cpuundefined when one job name has several rows in a digest (CPUPctspanning 1.55–49.08 gave a 6x spread with nothing to choose between them),digest_usernever solicited at bootstrap, and a missingallowed-toolsentry.slurm-digestwith a seeded submission log, so the scope-join matched for the first timeslurm-sizingitself — the skill no earlier test had run — against a table seeded to force every branchIO-BOUNDand absent-row branches all correct. Three documentation gaps, all fixed.The Critical, and why only test 3 could reach it.
scopewas being recorded but nevercompared, so a subset measurement was presented as an actionable recommendation for a full run — a
row measured on 5 of 100 units read as a clean recommendation for all 100, the exact
under-provisioning the design exists to prevent. It survived three earlier reviews and two
execution tests because every earlier run had an empty submission log: with no log, every row
carried the conservative
>=marker and the declared-scope path was never taken. The safe defaulthid the unsafe branch. Only a seeded fixture reached it. The fix moved the comparison out of the
merge step — the one place it cannot be evaluated, since at merge time nobody knows what the
user will run next — and into the point of use.
Honestly not verified
slurm-sizing's description reliably fires onindirect submission (a launcher that calls
sbatchinternally) is untested and is modeldiscretion, not a hook. This is the single most consequential open question: if it does not
fire, the skill is simply absent at the moment it matters, and no other safeguard here
compensates. Watch it after install; the fix, if needed, is a
PreToolUsehook.SubmitLineretention on the digest's own cluster. The enrichment mechanics are verified;whether the accounting database still holds
SubmitLinefor jobs old enough to appear in aweekly digest is not. If purged, enrichment degrades to
scope: unknown— the designed-safeoutcome (a lower bound), not a corruption.
is larger than what the table's row measured, the skill refuses to certify the number and
offers three routes — scale by a known parameter and label the result an estimate, start
from a larger run that previously succeeded, or measure first. Route 1 assumes linearity, which
the skill says out loud and which is often wrong for memory. This is a real limit of the
approach, not an oversight: the system measures, it does not model.
Remaining behavioural checks against a real install — cluster-gate stop, and above all
wrapper-script trigger reliability — should be run before this is considered validated in
practice.