Repository navigation
Conversation
A behavioral run says the cases pass. It cannot say they pass because of the skill, and an expectation any competent agent meets is a measurement of the model with the skill's name on it. `skillscope staleness` runs the cases twice, once with the skill installed and once with nothing installed, and reports which ones moved. `install_skill` already described the second situation as a failure mode -- answering from the prompt and scoring like it, which looks like a bad skill rather than a missing one. Asked for deliberately and labelled, it is the measurement. Both engines take the switch, both halves go through one dispatch, and the report never gates: the agent is nondeterministic on both sides, so a verdict here is somewhere to look rather than a build failure. Signed-off-by: John Lybeck <john.lybeck@amd.com>
The first real run of this command failed both ways and said nothing more. The two sides had failed differently -- with the skill the agent tried the task and was stopped by the machine, without it the agent never attempted it, which is the skill working -- and neither the verdict nor the JSON could show it. Only the raw inspect logs could, which is not a report. Both sides' checks now reach the comparison and the report tables each unmet expectation against what happened on either side. No divergence flag is computed from them: the details are judge prose, and two runs can word the same behaviour differently. The first run with a `no_signal` case also showed the empty state claiming every case had needed the skill, which that case had not. Signed-off-by: John Lybeck <john.lybeck@amd.com>
Which half of a comparison a run was lived in the task name, which made the pairing a string-parsing job and left it filterable by nothing. Both engines now stamp `skillscope_baseline` and `skillscope_skill` into the eval metadata, with tags beside them, which is what inspect filters on. That makes a log directory enough to rebuild the comparison, so `--from-logs` can report on a finished run without reaching a model or spending anything. A directory with logs that predate the keys says so rather than reporting an empty comparison as a result. Signed-off-by: John Lybeck <john.lybeck@amd.com>
Two things were being credited to a skill that had not earned them. Guard rails held on both sides, because that is what a guard rail does -- the base model was never going to run `sudo` unprompted either. Counting that as "the skill made no difference" marked the checks most worth keeping as dead weight. Verdicts now read the positive claims alone, and guards are reported on their own terms, where one broken with the skill installed is the finding. And nothing checked that the agent ever reached for the skill. A behavioral run installs one but cannot make it fire, so `earns_it` meant "installed and the outcome differed". It now records the activation with the detector the routing leg already uses, and says so when a case earned its verdict without the skill ever firing. The evidence section also stopped skipping `earns_it` cases, which was exactly backwards: those are the only ones where the evidence lives, and a run whose point was that three cases moved printed "nothing to show". Signed-off-by: John Lybeck <john.lybeck@amd.com>
`earns_it | 0` is a true row in a table that nobody reads twice, and it is the one result this command exists to surface. A run where every graded case came out the same now says so above the numbers, and names both explanations -- a skill that has stopped earning its place and a suite that never tested what it knows are different problems with different fixes, and the report cannot tell which it is looking at. A run that graded nothing says nothing of the sort: no cases is not the same finding as no difference. Signed-off-by: John Lybeck <john.lybeck@amd.com>
The baseline was never the empty room it claimed to be. A behavioral run passes no `config_dir`, so the runner's `~/.claude/skills` were registered beside the staged ones -- on the machine this was developed on, forty of them. Both passes could reach them, and the command runs from the repo root, so the skill under test was readable on disk whatever the staging step did. Real runs hit both routes: one baseline invoked an unrelated personal skill, another read the skill under test straight out of the repo. Both were reported `no_signal`, which asserts the agent managed unaided. It had not. So both passes now run under a throwaway config dir, the way `routing` has always done, and the command refuses a host run that cannot be isolated rather than quietly measuring this machine. What isolation cannot close -- the source on disk -- is detected instead and reported. Contamination is not symmetric, so the verdict is not voided symmetrically. `no_signal` and `harmful` rest on the baseline having managed something, and cannot survive it. `earns_it` and `fails_either_way` rest on the baseline having failed, and a baseline that failed while holding more than it should fails all the harder without it. `--from-logs` detects both too. It read activation but not contamination, which made a rebuild more permissive than the run it rebuilt, quietly restoring a verdict the live run had voided. Also: budget for a baseline that runs longer than the graded pass. An agent holding the skill goes straight to the work; an agent without it explores. The old guidance said twice a behavioral bound, and a two-case run spent 2m23s on the graded pass and was still going on the baseline half an hour later. Signed-off-by: John Lybeck <john.lybeck@amd.com>
The measurement rests on two things: knowing which skill fired, and knowing
which two runs are the two halves of one comparison. Both were wrong, in ways
the suite could not see.
`_inspect_room` answers two questions from one walk, and they shared an
`if/elif`. Once `why` was set, a later `other:<name>` hit failed the first
condition and landed in `fired` -- so `activated_with` held a contamination
marker, which reads as truthy everywhere downstream. `earned_without_firing`
tests `not c.activated_with`, so the "never activated the skill" warning was
suppressed on exactly the contaminated cases that most need it, and the marker
reached both JSON reports verbatim. The branches are decoupled, and `fired` is
now `None` or the skill, which is all `detect_activation` can return for
`skills=[skill]`. `_activated`, left behind by 43be985 and carrying the same
conflation in a worse form, is deleted.
Contamination was judged on a bare skill name: `/{skill}/` as a substring of
every call's serialised arguments. `structure.py` allows a single-word name
and sets no minimum length, and single-word names are the norm -- `pdf`,
`docx`, `pptx`. For a skill named `run`, `/run/user/1000` was contamination;
for `docs` or `tools`, any ordinary path. The cost of that false positive is
the highest here: `_voided_if_dirty` turns the two verdicts this command
exists to surface into `ungraded`, and the report states the contamination as
fact. Structured path arguments are resolved and tested for containment
against the skill's real directory and its staged copy; shell commands keep
substring matching, because there is no field to resolve, but against the
resolved path rather than the name.
`--from-logs` pooled across runs. It grouped on `(skill, baseline-flag)` with
no run id, and every ordinary `behavioral` run stamps `BASELINE_KEY: False` --
so an unrelated run of the same skill was a candidate for this comparison's
"with skill" half, in a directory that is shared, gitignored wholesale, never
pruned, and the one the docs tell you to point this at. Case ids are repo-wide
and `pair()` builds a dict from them, so the splice was silent and the
survivor was the older log. `cmd_staleness` now mints one id per invocation
and both halves carry it; logs without one are not a half of anything. A case
two logs in one run both report is errored rather than resolved, the way this
module reports every other ambiguity.
The rebuild was also more permissive than the run it rebuilds, which is the
property 43be985 claimed to have fixed. The live path voids a sample whose
checks could not be read; `outcomes_from_log` did not, so it rebuilt as
`checks=[]`, which makes `_met()` False, which promotes a harness failure to
`earns_it`.
`staleness` skipped hooks `behavioral` refuses to skip. `_require_hook_support`
guarded on `command != "behavioral"` and `_prepare_graded_run` passes
`args.command`, so the guard returned before checking anything. Routing is
legitimately exempt because it executes nothing; staleness executes everything
behavioral does, twice.
A `--from-logs` report asserted the current invocation's `--model` and
`--effort` as though they described the logs. Rebuilding a `sonnet`/`low` run
printed "on `opus` (effort `high`)". Nothing reads them off the logs and the
logs need not agree on them, so they are not claimed.
`--timeout` now defaults to twice the single-pass bound. The command's own
docstring documents a two-case skill finishing the graded pass in 2m23s with
the baseline unfinished after 30 minutes; on the shared 900s default the
slower half got ~637s of a measured 1800s+ need, which made truncation the
expected outcome on the canonical example rather than a risk.
Docs: `guards_only` was missing from the verdict table, and neither README nor
usage.md said that `staleness` refuses to start without `ANTHROPIC_API_KEY` or
`ANTHROPIC_AUTH_TOKEN`. It is in `routing`'s camp, not `behavioral`'s, and the
section's first example failed for anyone authenticated the way `behavioral`
permits.
The tests: three of them asserted substrings of their subject's own source and
passed while the property they named was false. Dropping the `not` from the
isolation guard left `test_the_command_refuses_an_unisolatable_host_run`
green. They assert behaviour now, `cmd_staleness` and `_run_behavioral` are
called by a test for the first time, and `_inspect_room` is exercised against
real tool calls rather than a stub with no messages. Each new test was checked
against the unfixed code: every one of them fails there.
Signed-off-by: John Lybeck <john.lybeck@amd.com>
Refusing every unlabelled log was too strong. Checked against the only staleness logs that exist anywhere -- the quark-torch-llm-ptq run this branch was developed against, still in `amd-skills/.skillscope/logs` -- the previous commit made them unreadable: `--from-logs` reported "no skillscope staleness logs" for a directory holding both halves of a real run. The one artifact anybody could check a rebuild against, refused by the fix meant to make rebuilds trustworthy. A log written before the key existed is not suspect, it is unlabelled. What the key defends against is *choosing*: a directory holding several candidates for one side, where pairing picks one silently and `list_eval_logs` ordering makes that the older. Where a skill has one log per side there is nothing to choose -- the pairing is forced, and a label would not change which logs pair. So unlabelled logs group together and pair when each side has exactly one. Where a side has more, every case in that group is `ungraded`, because the ambiguity is the hazard itself. A labelled run never falls into the bucket, so a run written by this version cannot be weakened by it. That real run now rebuilds: one `no_signal` on a clean baseline, and one `ungraded` for the case the baseline never reached, which is the half-hour timeout the previous commit's message describes. Also checked the rewritten detector against those logs' real tool calls, which is the first time it has run on any. It agrees with the old one on all three samples: the baseline is clean, and both graded samples read the skill's own files, which is what the graded side is supposed to do. Signed-off-by: John Lybeck <john.lybeck@amd.com>
**The unlabelled-log bucket was not held to the standard of the fix it relaxed.** `6abbc51` read it as a migration concern. It is not one. A plain `skillscope behavioral` run stamps `BASELINE_KEY: False` and never stamps a run id, because only `cmd_staleness` mints one, so an ordinary behavioral log is shaped exactly like a pre-id graded half and always will be. The bucket was therefore a permanent inlet, not a transitional one. Reproduced before fixing. A directory holding one keyless graded-side log -- which is what every `behavioral` run leaves behind -- returned two comparisons, both `ungraded`, counted into `totals.cases` and `totals.ungraded` with a `per_skill` row for a skill the run never covered. Point `--from-logs` at `.skillscope/logs` as the docs instruct and the denominator inflates by every skill ever graded there. With a leftover pre-id baseline for the same skill it is worse: one log a side, no ambiguity to detect, and a plain `behavioral` run is silently handed to the comparison as its "with skill" half. The original splicing bug, through the back door, and the test added in `6abbc51` encoded that pairing as intended. Unlabelled logs are now skipped unless `--unlabelled-logs` asks for them, and asking is not enough on its own: a group also needs a baseline side, which nothing but `cmd_staleness` ever writes. So ordinary behavioral logs stay out whatever flags are passed, and the flag means what it says -- the caller asserting the directory holds nothing else for those skills. The flag is refused without `--from-logs`, since a live run labels both its halves. The three places stating the old contract are corrected rather than left: `from_logs`'s docstring, the `--from-logs` help, and `docs/usage.md`. All three were true when `ed49d4f` wrote them and `6abbc51` falsified them without saying so. **The detector's two halves disagreed, in both directions.** The structured branch used a boundary-aware containment check; the shell branch nine lines later used a raw `in`. Verified against the real function: - `cat ~/.claude/skills/foo-extra/SKILL.md` read as skill `foo`'s own files. The same class of false positive the first fix was written to remove, surviving where the mark carried no trailing boundary. - `Glob pattern=<skills>/foo/**/*.md` invisible. `pattern` was not in `PATH_FIELDS`, and a closed field list cannot keep up with an open tool surface -- MCP tools name arguments whatever they like. - `cat .claude/skills/foo/SKILL.md` invisible. `no_sandbox` runs the agent with `cwd=workspace`, so the relative form is the natural one, and the mark was anchored with a leading separator. The bare-name matching this replaced caught it. The misses are the worse direction: `_voided_if_dirty` never fires, and a dirty baseline is reported as a clean `no_signal` stated as fact. So the field list is gone. Every string anywhere in a call's arguments is tested, as written and as the path it resolves to, against marks matched at component boundaries. Checking every string is only safe because the marks are absolute and boundary matched -- that is what removed the false positives, not the field list. The review's eight-row table is now 8/8, and the two pass-one false positives (`/run/user/1000` for a skill named `run`) stay fixed. Five mutations reintroducing these bugs were each caught by a test that went red: unconditional legacy pooling, dropping the baseline-side requirement, dropping the trailing boundary, restoring a fixed field list, and re-anchoring the staged mark. Checked on real logs. The rocm-doctor run rebuilds to the same five verdicts as the live run; its neighbouring unlabelled quark logs are now passed over by default and return under `--unlabelled-logs`; and a directory of behavioral logs rebuilds as nothing either way. Signed-off-by: John Lybeck <john.lybeck@amd.com>
…he action `staleness --help` printed "Default: 900 (15 minutes)" three lines under a description announcing that --timeout "defaults to twice the usual here". One screen of help contradicting itself, and the help was the wrong half: the real default is 1800. The cause was the fix's own shape. `set_defaults` changes the value argparse resolves and does not revisit the help string, which `_add_timeout_argument` had already built from the shared constant. So the default is threaded through `_add_graded_arguments` instead and the help is built from the number in force. The string also omitted `staleness` from the commands it says the bound is shared by, which is now a list of four. A mutation back to `set_defaults` leaves the value correct and the help stale, and the new test catches exactly that. `action.yml` listed the subcommands without `staleness`, and more usefully left it out of what needs `install-claude`. It drives `_run_behavioral` for both of its halves, so it needs the `claude` CLI exactly as much as `behavioral` does; a CI author reading only that line gets a confusing failure. Signed-off-by: John Lybeck <john.lybeck@amd.com>
The baseline half of a staleness comparison only means something if the
agent really had no skill available. `_skill_marks` recognises a tool call
that reached the skill by comparing against the skill's absolute path --
and it worked that path out from the *reading* process, not the run.
So `--from-logs` re-derived a weaker answer than the run it rebuilt.
Reproduced: the same recorded call, `Read {"file_path": ".../run/SKILL.md"}`,
graded `ungraded` live and `no_signal` when the report was rebuilt from
another directory or on a machine without the skill checked out. That is
the "a rebuild is more permissive than the run it rebuilds" failure this
module closes for the no-checks branch and tests for by name; contamination
was the one place it was still open, and the report stated the weaker
answer as fact.
Both engines now record where the skill was, and the reader uses that
rather than its own cwd. Where neither the log nor the machine can say,
the room is reported unknown and the case lands `ungraded`, instead of
quietly keeping the one mark that cannot fail and calling it clean.
Also drops the `resolve()` branch. Its comment claimed it let `./x` and
`../x` name the same file by another spelling, but the agent's cwd is
`no_sandbox`'s per-sample workspace and appears nowhere in the tool-call
record, so it resolved them against skillscope's cwd instead. It could
never serve that case, and it was the second source of the cwd dependence.
`~/x` is context-free and still expands.
Signed-off-by: John Lybeck <john.lybeck@amd.com>
The fix itself was unguarded: deleting the metadata line or restoring the `resolve()` branch both left the suite green, and both would look right on the machine that wrote the logs, which is exactly how the original hid. Three tests, each aimed at one claim. The cwd one asserts the property rather than the mechanism -- same recorded call, two directories, one answer -- so it catches any future route back to a reader-dependent verdict, not just the branch that was removed. Signed-off-by: John Lybeck <john.lybeck@amd.com>
The branch adds a fourth and left "three" everywhere, most visibly in the top-level --help, which printed three bullets with staleness absent directly under a usage line listing it, and in the README, where a reader met "three types" at the top and a fourth command in the table below with nothing reconciling them. `datasets.py` keeps its two, because two is right there: staleness adds nothing to write, it grades the behavioral cases again. Said so instead, since what it does add is a bar on those cases -- one that passes either way is not evidence -- and that is this module's subject. Signed-off-by: John Lybeck <john.lybeck@amd.com>
Rebuilding a real pre-key log pair surfaced it: all five cases printed "5 case(s) had a baseline that was not empty", which the run had not established and could not. `UNKNOWN_ROOM` was landing in the same field that means "demonstrably dirty", so the totals and the prose both read it as contamination found. Counted and worded apart now. Both still void the same verdicts, because an unanswerable question is worth no more than a failed one, but the report says which it is and how to fix it. Also moves the `earned_without_firing` comment down to the key it describes; it had been sitting above `baseline_contaminated`. Signed-off-by: John Lybeck <john.lybeck@amd.com>
The comment argued the flag was needed because refusing these logs "made the only staleness logs that existed anywhere unreadable". True when it was written, false the day this merges, and the flag is permanent surface. The durable reason is the one underneath: a log directory is a record somebody already paid an agent run for, and a reader that cannot open it over a metadata key added afterwards has thrown that run away. Checked rather than assumed -- a real pre-key pair is on this machine, and the flag reads it. Signed-off-by: John Lybeck <john.lybeck@amd.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
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.
Closes #6.
behavioraltells you a case passed. It cannot tell you the case passedbecause of the skill. A case written so that any competent agent satisfies it
stays green forever, and the report is then measuring the model rather than the
thing under test.
stalenessruns the same behavioral cases twice, once with the skill installedand once with nothing installed at all, and puts the two side by side.
What's added
skillscope staleness, a fourth graded command. It reads the sameevals.jsonasbehavioral, so there is no new file to write and no newassertion to learn.
Six verdicts per case:
earns_itharmfulno_signalfails_either_wayguards_onlyungradedA markdown and JSON report, with a per-case table of every expectation that
came out differently on the two sides.
--from-logs DIR, which rebuilds the comparison from logs already on disk.It runs no agent and costs nothing.
It never gates. Two runs of a nondeterministic agent disagree with themselves
often enough that one pair is a lead rather than a result, so a completed run
always exits 0. An expired deadline or a refused run still fails.
Keeping the baseline honest
The measurement is only worth anything if the baseline really had nothing.
Three things protect it:
CLAUDE_CONFIG_DIR. That only works when auth comes from the environment, soon
--engine claude-code-no-sandboxthe command refuses to start withoutANTHROPIC_API_KEYorANTHROPIC_AUTH_TOKEN, the same stanceroutingalready takes.
--engine claude-codeneeds nothing extra: its guest has nouser-level skills at all.
nothing stops an agent naming an absolute path. Every string in every tool
call is boundary-matched against the skill's directory and against
.claude/skills/<name>.no_signalandharmful,which both rest on the baseline having managed something. It leaves
earns_itandfails_either_way, which rest on the baseline having failed:a baseline that failed while holding more than it should have fails harder
without it.
Where the run did not record where the skill was and this machine does not have
it, the report says so and grades the case
ungraded, rather than calling anunanswerable question clean.
unexpected_behavioris deliberately excluded from the verdict. A good guardrail holds whether or not the skill is installed, so counting it as "made no
difference" would mark the checks most worth keeping as dead weight. Guards are
reported separately as held or broken.
What this changes for the existing commands
Nothing changes about what they grade. Three things are added:
behavioralnow also records which skill the agent reached for, and whetherthe room was what it claimed.
activatedandcontaminatedappear in thebehavioral JSON report.
the skill was on the machine that ran it, and, for a staleness run, a
per-invocation run id.
--from-logssafe to point at.skillscope/logs.That directory is shared, gitignored and never pruned, so without it an
unrelated older run is a candidate for a comparison's "with skill" half.
Plain
behaviorallogs can never be read as a staleness comparison: a groupneeds a baseline side, and only a staleness run writes one.
Not wired into CI
stalenessis opt-in. It is absent fromselection.plan()and fromreusable.yml, so no downstream repo starts running it. The only way it runsis by invoking it, or by adding a job with
command: staleness.That is deliberate for now. It costs two agent passes, and the baseline is the
slower half because an agent without the skill explores instead of going
straight to the work. On a two-case skill the graded pass finished in 2m23s and
the baseline had not finished after 30 minutes, which is why
--timeoutdefaults to twice the usual here.
How to test
Needs
ANTHROPIC_API_KEYorANTHROPIC_AUTH_TOKENon the default engine.Start with one or two small skills and watch the wall clock.
To see the offline path without spending anything, run the above once, then: