diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 9ce3775..c99da1e 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -1,8 +1,8 @@ # Contributing to {{PACKAGE_NAME}} -> New here? Read [docs/onboarding-contributors.md](docs/onboarding-contributors.md) -> first — the one-sitting tour of the layout and the one rule that matters most. -> This file is the detailed reference. +> New here? Read [docs/onboarding.md](docs/onboarding.md) first — the one-sitting +> tour of the layout and the one rule that matters most. This file is the detailed +> reference. `{{PACKAGE_NAME}}` is the shared, generic {{DOMAIN}} capability package — plugins, skills, tools, and the agent's base soul. Operators run private instances on top of @@ -114,3 +114,17 @@ could use, a sanitization improvement, a workflow change — see [docs/promotion-and-upstream.md](docs/promotion-and-upstream.md) for the promotion flow: how to extract and sanitize work from your private instance into a publishable form, and how to submit upstream to the HSM base. + +**Deciding *what* to promote** — whether a lesson an instance learned the hard way +is general enough to ship to every instance, or belongs only to the one that learned +it — is [docs/lesson-promotion-filter.md](docs/lesson-promotion-filter.md). Use it +before you write the PR, and record the rules you *rejected* and why: the rejected +list is how the next contributor calibrates. + +**Where a promoted lesson goes.** `docs/` is for contributors and is not shipped to +instances. Only `hermes-skill/`, `matilde_plugin/` and `docker/SOUL*` reach a +deployed agent. If an agent must act on the rule, it belongs in +`hermes-skill/references/` with a pointer from `SKILL.md` at the point in the +workflow where it would be violated — a reference nothing loads is documentation, +not a control. And merging is not shipping: see +[docs/deployment-reach.md](docs/deployment-reach.md). diff --git a/README.md b/README.md index 52f4cee..4b4ee21 100644 --- a/README.md +++ b/README.md @@ -170,8 +170,18 @@ something specific went wrong, and says what. | [golden-validation-recipe.md](docs/golden-validation-recipe.md) | The offline, dependency-free worked validation — the reference shape of a correct finding, and the package's smoke test. | | [meg-validation-study.md](docs/meg-validation-study.md) · [stateful-study-pipeline.md](docs/stateful-study-pipeline.md) | Running a memory-bounded study over an open dataset, and the resumable step store that makes it restartable. | | [privacy-and-visibility.md](docs/privacy-and-visibility.md) · [promotion-and-upstream.md](docs/promotion-and-upstream.md) | The privacy model, the sanitization gate, and how a technique gets promoted out of a private overlay into this package. | +| [lesson-promotion-filter.md](docs/lesson-promotion-filter.md) | The four tests that decide whether a lesson an instance learned is general (ships to everyone) or specific (stays local), with worked verdicts including the ones that failed. | +| [deployment-reach.md](docs/deployment-reach.md) | Why a merged rule is not a running rule: the four manual steps between `main` and a deployed agent, and an assessment of the re-apply endpoint as the delivery mechanism. | | [onboarding.md](docs/onboarding.md) | Start here if you are new to the package. | +The skill ships its own reference set in `hermes-skill/references/`, which is +installed alongside `SKILL.md` and is therefore readable by a running agent: +`evaluation-validity.md` (what one number means, and the trajectory check), +`enforcement-ladder.md` (available / default / gated, and positive controls), and +`agent-failure-modes.md` (how verification degrades under pressure). Documents in +`docs/` are for contributors and are **not** shipped to instances — put anything an +agent must act on in `hermes-skill/references/`. + --- *Matilde is private during development. The citation engine is intended for the public diff --git a/docs/deployment-reach.md b/docs/deployment-reach.md new file mode 100644 index 0000000..e21d857 --- /dev/null +++ b/docs/deployment-reach.md @@ -0,0 +1,148 @@ +# Deployment reach — does a merged lesson actually reach a running agent? + +A rule that is merged here has not yet changed any agent's behaviour. This +document traces the path from `main` to a running container, names each place it +breaks, and states the one thing that must happen **before** anyone closes the +gap. + +It exists because of a specific finding: a correctness rule was correctly +generalized, correctly promoted, and merged into this package — and the instance +that had learned it then made the same mistake again, weeks later, in a new tool. +The filter was not the problem. The delivery was. + +--- + +## The path, and the four places it breaks + +An agent managed by HSM receives this package as a **use-case template**. The +template declares its artifacts as pinned git sources — the skill from +`hermes-skill/`, the plugin from `matilde_plugin/`, the SOUL from `docker/`. + +| # | Step | State observed 2026-08 | +|---|---|---| +| 1 | Rule merged to `main` | ✅ works — the comparison-validity rules landed here as [trustworthy-comparison.md](trustworthy-comparison.md) and a summary in the skill | +| 2 | A release tag is cut | ❌ **not done.** The newest tag was six commits behind `main`, and the comparison work was in those six. | +| 3 | The template registry's pin is bumped to the new tag | ❌ blocked by 2. The registry pins an exact tag; merging to `main` changes nothing until both this and 2 happen. | +| 4 | The template is re-applied to each deployed agent | ❌ never run for this agent. Its skill directory was a hand copy, made once, then edited in place. | + +Four independent manual steps sit between "merged" and "the agent behaves +differently," and **every one of them was open at the same time.** That is the +whole explanation for the recurrence. Nobody skipped a lesson; the lesson was +never delivered. + +> **Corollary for anyone reporting on a promotion: "merged" is not "shipped," and +> "shipped" is not "running."** These are three separate claims requiring three +> separate pieces of evidence. Repository state is evidence about the repository. +> To claim a rule is live, check the runtime. + +--- + +## Is the re-apply endpoint the right mechanism? + +`POST /api/harnesses/:id/usecase/reapply` re-runs the template install against a +deployed agent's data directory. Assessed against what it actually does: + +**What is right about it** + +- **It targets exactly the right path.** The template installs the skill to the + same directory the hand copy occupies, so this replaces the hand copy rather + than sitting beside it. +- **It gates before it writes.** Artifacts are fetched and run through an + injection scan at strict scope; a poisoned artifact is refused before anything + touches the data directory. Supply-chain screening is the correct thing to have + in front of a mechanism that writes into every agent. +- **It only bounces the container when something actually changed on disk.** A + no-op re-apply does not restart the agent. +- **It does not re-seed the SOUL,** on the correct reasoning that a deployed + agent's identity may be operator-customized. +- **It is audited.** + +**The disqualifying problem, today** + +The install is **destructive at directory granularity**. With overwrite set — which +re-apply always sets — the existing artifact directory is removed recursively and +replaced with the fetched contents. It is not a merge and not a three-way update. + +**Re-applying against a hand-edited runtime deletes any file the runtime holds +that the template does not** — including, in the observed case, the entire batch a +promotion pass was harvesting. Reference documents that exist in no repository +anywhere are replaced by whatever the package currently ships, and nothing in the +call reports what was lost. + +The SOUL is protected from exactly this, deliberately and with a comment +explaining why. The reasoning applies with equal force to any artifact directory +an operator can edit in place, and it has not been extended there. + +**Verdict: right mechanism, wrong preconditions.** The endpoint is the correct +long-term delivery path — it is gated, targeted, audited, and idempotent. It is +not safe to invoke against a hand-edited runtime until that runtime's unique +content has been captured somewhere durable. + +--- + +## Required order of operations + +**Do not call re-apply on a hand-edited agent until step 1 is done.** This is the +one hard sequencing constraint. + +1. **Capture the runtime.** Copy the deployed artifact directory into a repository + — the instance's private one — and commit it as-is, before any editing. Until + this exists, the runtime is the only copy of its own history and re-apply is a + data-loss event. +2. **Reconcile in both directions.** Diff the captured runtime against this + package. Expect content on both sides: the runtime holds instance material the + package never had, and the package holds rules the runtime never received. Run + the runtime-only material through + [the promotion filter](lesson-promotion-filter.md). +3. **Decide where instance-only material lives.** Anything that fails the filter + is legitimately instance-local, and re-apply will delete it. It needs a home + the template does not own — an overlay directory outside the artifact path, or + a separate instance-scoped artifact — otherwise every future re-apply destroys + it again. +4. **Tag, then bump the pin.** Cut a release from `main`; update the template + registry to the new tag. Both steps, or nothing ships. +5. **Re-apply, then verify at the runtime.** Confirm the promoted text is present + in the container's data directory, not merely that the endpoint returned `ok`. + An endpoint's success response is a claim about the endpoint. + +--- + +## The design change this implies + +One recommendation about the mechanism, stated as a property the mechanism should +have rather than as anyone's task. + +**A template re-apply should be non-destructive, or it should refuse.** Either +merge rather than replace, or detect that the destination holds files absent from +the source and fail with that diff instead of proceeding. A carve-out for exactly +this risk already exists for the identity file; the reasoning extends to any +artifact directory an operator can edit in place, and extending it is the smaller +change. + +Until one of those holds, the endpoint is a foot-gun aimed at precisely the agents +that have been used most — an agent accumulates local material *because* someone +worked with it, so the destructive case and the valuable case are the same case. A +"refuse and report" version is strictly better than nothing and much cheaper than +a real merge. + +The corollary for whoever owns the delivery path: instance-local material needs a +home the template does not own — an overlay directory outside the artifact path, +or a separate instance-scoped artifact. Without one, a non-destructive re-apply +only postpones the deletion to the cycle after next. + +--- + +## The general rule + +This package's own enforcement ladder +(`hermes-skill/references/enforcement-ladder.md`) classifies controls as +*available*, *default*, or *gated*. + +**A rule merged into a package that no runtime pulls is rung 1 — available.** It +has the full appearance of coverage, it is greppable, it is citable in a review, +and it changes nothing. The relief of seeing a rule in the repository consumes the +suspicion that would have gone into checking whether any agent ever loaded it. + +> When you promote a lesson, the last question is not "did it merge?" but **"what +> is now different about a running agent?"** If the honest answer is "nothing +> yet," say that — in the PR, in the report, wherever the promotion is claimed. diff --git a/docs/lesson-promotion-filter.md b/docs/lesson-promotion-filter.md new file mode 100644 index 0000000..1d86cff --- /dev/null +++ b/docs/lesson-promotion-filter.md @@ -0,0 +1,256 @@ +# The lesson promotion filter + +How to decide whether something an instance learned the hard way belongs **here** +— in the package that ships to every instance — or stays **there**, with the +instance that learned it. + +[promotion-and-upstream.md](promotion-and-upstream.md) says *how* to promote: +branch, strip the particulars, PR, let the gate run. This document is the missing +half — *what* to promote. It exists because "strip the particulars, keep the +method" is easy to agree with and hard to apply, and because the interesting +cases are the ones where a rule is **stated in one domain's vocabulary but its +mechanism is domain-free**. A filter that only catches the easy cases is not +worth having. + +--- + +## The unit of promotion is a rule, not a file + +Start here, because it dissolves most of the difficulty. + +Instance documents are mixed. A single reference file will contain a genuinely +universal rule, a worked example that names a private dataset, and a parsing +recipe that is meaningless outside its domain — often in the same paragraph. +Asking "should this file be promoted?" has no good answer. Asking "should this +*rule* be promoted, and what evidence can come with it?" always does. + +So: read the instance document, extract the rules as separate claims, and run +each one through the tests below on its own. Expect a single source document to +split — some rules up, some staying, and the evidence for a promoted rule often +needing to be rewritten even when the rule itself passes untouched. + +--- + +## The four tests + +A rule is promoted only if it passes **all four**. They are ordered cheapest +first, so a rule that fails T1 costs you one sentence. + +### T1 — Substitution: does the mechanism survive a domain swap? + +Rewrite the rule with every domain noun replaced by a variable. Then ask whether +the rewrite still explains **why the failure happens**. + +- If the explanation survives, the domain vocabulary was decoration. **General.** +- If the rewrite collapses into a truism ("be careful", "document things"), the + content was in the particulars. **Specific.** + +The common error is to judge by vocabulary. Vocabulary is the least reliable +signal available, and it misfires in *both* directions: + +| Rule as written | Substituted | Verdict | +|---|---|---| +| "Score every arm of the comparison with the same matcher — one arm counted a single-sample overlap as a hit." | "Score every arm with the same evaluation function." | **General.** The mechanism — a more permissive scorer inflates the arm it is applied to — never mentioned the domain. The domain words were noise. | +| "Look at your spectrograms before reporting a segmentation number." | "Look at your figures before reporting a number derived from them." | **General.** Sounds maximally domain-bound; the mechanism ("a result you have never viewed is a number, not an observation") is universal. | +| "The guard fires after training and before the result writes, so a refusal discards the whole run and produces nothing." | "A guard whose refusal discards the work costs a run, not a report." | **General.** A hyper-specific artifact carrying a fully general rule about where in a pipeline a check belongs. | +| "The identifier is at the fifth underscore-delimited position of the filename." | "The identifier is parseable from the filename." | **Specific.** The substituted form is content-free; all the value was in the parsing recipe. | +| "Filter the label taxonomy from 41 families to 14 by a minimum-count threshold." | "Any data-selection threshold must be recorded, and the result shown at a second cut." | **General — but only the second sentence.** The taxonomy's identity stays; the rule about undocumented selection thresholds travels, and the counts travel with it as derived evidence (T3). This is the split described above, inside one sentence. | + +**T1 is the test the brief's hard case needs.** "Score every arm with the same +matcher" reads as bioacoustics and is a general experimental-design rule; T1 +resolves it correctly and cheaply because it asks about the *mechanism*, not the +*words*. + +### T2 — Counterfactual audience: would a stranger behave differently? + +Would an agent working in a completely unrelated field, who has never heard of +this instance's domain, **do something different** because of this rule? + +Not "would they nod." Would their behaviour change. + +- "A capability limitation you wrote down is a claim with a date on it; re-check + before declining on the strength of it." → A legal-research agent carrying a + stale "I cannot read PDFs" note refuses work it could do. **Behaviour changes. + General.** +- "Your metric's ceiling is the reliability of your ground truth." → Anyone + scoring against human labels — content moderation, clinical coding, relevance + judgments — is currently unable to say whether their last improvement was + signal. **Behaviour changes. General.** +- "The evaluation script lives at this path and takes these flags." → Nobody + outside the instance can act on it at all. **Specific.** + +T2 catches rules that pass T1 by being *stated* generally while only ever +mattering to one setting. It is the test for false generality, where T1 is the +test for false specificity. + +### T3 — Independence: can the rule and its evidence be separated? + +Two conditions, both required: + +1. **The rule must be statable without the private particular.** If you cannot + write the rule down without naming the collaborator, the dataset, the + filename, or the path, it is not generalized yet. This is the existing + promotion doctrine — *the act of generalizing is the proof that nothing + leaked* — restated as a test. +2. **The evidence must survive sanitization and still be believable.** Evidence + is what makes a rule stick rather than read as a platitude, so it should come + along. It comes along **reduced to derived quantities and structural shapes**. + +What that reduction looks like in practice: + +| Instance evidence | What travels | What stays | +|---|---|---| +| "4 of 12 test subject IDs also appear in training: *(four identifiers)*." | "4 of 12 test subjects also appeared in training." | The identifiers. | +| "*(a recording filename)* is in train and *(a near-identical recording filename)* is in test — the annotator's own filename says they are the same individual." | "Two files the annotator's own naming marked as the same individual landed on opposite sides of the split." | The filenames. They encode site, subject and date; they are the raw material, not a derived metric. | +| "A hop-length/normalisation mismatch dropped F1 from 0.826 to 0.595 on identical data." | The whole thing. Derived metrics are publishable; nothing here identifies anyone. | Nothing. | +| "`print(f\"Baseline: F1=0.731\")` reached a completion notification and was read back as a measurement." | The whole thing. A code shape is not a particular. | Nothing. | +| "The threshold was tuned on the test set for the *(named)* corpus at *(named site)*." | "The threshold was tuned on the reported set." | The corpus name and the site. | + +**The useful discovery is that T3 makes privacy and generality the same test.** A +rule that still needs the private particular in order to be convincing has not +been generalized — it is a case report wearing a rule's clothes. Sanitizing it +does not damage a genuinely general rule; it damages a rule that was never +general, and that damage is the signal. + +Hard constraint, non-negotiable, no override path: this package is public. +Nothing promoted may carry a collaborator or participant name, an unpublished +dataset's specifics, a raw data filename, a site or location label, a private +filesystem path, or an infrastructure identifier (channel, guild, bot, account). +Derived metrics — scores, deltas, counts, confusion structure — are publishable +and should be kept, because they are what makes a rule believable. + +### T4 — Recurrence, or a mechanism that predicts it + +The package's existing bar for promoting a *technique* is repetition: one use is +an anecdote, three is a method. A *lesson* is different — a lesson earns +promotion by demonstrating that writing it down locally was not enough. Promote +if **either**: + +**(a) It recurred after it had already been written down.** This is the strongest +possible evidence, and it is common. One instance recorded the same +comparison-validity error three times in five weeks; the third occurrence +happened five days after the second was written up, in a new tool with its own +scoring loop, precisely because the lesson lived in prose that the new tool's +author never read. Prose in one instance demonstrably did not hold. That is an +argument for shipping it where every instance gets it. + +**(b) The write-up names a structural asymmetry that guarantees it will not +self-correct.** Some failures are silent by construction and so will never +generate the error that would expose them. Example: a false "I cannot do X" is +silent forever, while a false "I can do X" fails loudly on first attempt — so +self-imposed limitations are exactly the beliefs least likely to ever be tested. +That asymmetry predicts recurrence without waiting for it. A rule that names one +qualifies on first occurrence. + +If a lesson has occurred once and offers no mechanism, it stays local. Come back +when it happens again — and note that it happening again is itself the finding. + +--- + +## Worked verdicts + +Run against one instance's accumulated material, kept here because a filter's +value is in the cases where it says *no* and the cases where it surprises you. + +### Promoted + +| Rule | Passed on | +|---|---| +| An optional guard is documentation; enforcement has three rungs (available / default / gated). | T4(a) — the guards existed, were imported, and four of five were not called. | +| A guard should cost a report, not a run. | T1 — a scheduling rule about checks, stated via one script. | +| Running a guard's own tests proves the guard works, not that anything invoked it. | T4(b) — the conflation is invisible until something bypasses the guard. | +| Your metric's ceiling is the reliability of your ground truth. | T2 — changes what any human-labelled evaluation can conclude. | +| Report the metric-versus-tolerance curve, not a single operating point. | T1 — "tolerance" substitutes cleanly for any matching-slack parameter. | +| Leakage has a **unit**; disjointness at the file level is not disjointness at the subject level. | T1, and it extends a limitation the package already documents. | +| If a configuration knob moves the metric more than the effect you are claiming, you are measuring configuration. | T1 — the magnitudes are derived metrics and travel intact. | +| Per-artifact validation is structurally blind to reversals across runs; diff against the previous run first. | T4(a) — a validation pass returned a clean verdict on a run whose headline conclusion had reversed. | +| Conversational pressure shifts attention from verification to resolution. | T4(b) — resolution and verification compete for the same capacity. | +| Default affirmation makes agreement uninformative. | T4(b) — the agreement sounds identical whether the idea is good or bad, so it cannot be checked. | +| A mentioned date is not a deadline until it can name who set it, what type it is, and what is owed. | T4(a) — three occurrences of a derived value acquiring unearned authority. | +| A stated limitation is a claim with a date on it. | T4(b) — the silence asymmetry above. | +| Repetition launders inference into observation; restate the derivation with the number. | T4(a) — three occurrences in five weeks. | +| When a check's negative result is load-bearing, first prove the check can return a positive. | T4(a) — an unmatched glob aborted a command, and the empty result was published as verified absence. | + +### Kept local + +| Material | Failed on | +|---|---| +| The taxonomy's identity and the specific class filter — *which* taxonomy, and which classes it kept. The raw counts are derived quantities and travel with the rule under T3, as its evidence. | T1 — the filter itself substitutes to a truism; only the "record your selection threshold" rule travels (promoted as E5 in `hermes-skill/references/evaluation-validity.md`). | +| Identifier parsing from filenames. | T1 — the recipe *is* the content, and the filenames are raw material anyway. | +| Script paths, flags and per-tool evaluation entry points. | T2 — unusable outside the instance. | +| Corpus names, sites, per-recording labels, collaborator names. | T3 — hard block. | +| Chat platform guild / channel / bot identifiers used by a scheduled job. | T3 — infrastructure identifiers, and they sit next to a credential location. Looks like inert config; is not. | +| The specific date that was mistaken for a deadline, and who said it. | T3 — the *rule* about deadline provenance promoted; the incident stays. | +| An archival corpus chosen as one study's field-condition arm. | T3 — a public archive's name becomes a particular when it identifies one study's unpublished design. Public provenance does not make it non-identifying. | + +### The case that changed the conclusion + +"Score every arm of a comparison with the same matcher" passes every test — and +**was already promoted**, before this pass ran. It is in the package as +[trustworthy-comparison.md](trustworthy-comparison.md), and it is summarised in +the skill. + +The instance then made the error again, in a new tool, weeks later. + +The filter was not the bottleneck. The rule had already cleared it, been +generalized well, and shipped. What failed is that the instance's runtime never +received the package version, because deployment was a hand copy that had drifted +in both directions from the repository. + +**So: a filter is necessary and it is not sufficient.** Promotion that does not +reach a runtime is a rung-1 control — available, not enforced — which is exactly +the failure this filter's top-ranked promoted rule describes. Anyone running this +process should treat "did the promoted rule reach a running agent?" as part of +the process, not as someone else's problem. See +[deployment-reach.md](deployment-reach.md). + +--- + +## The process + +1. **Collect.** Take the instance's changed reference material since the last + promotion pass. Diff the deployed tree against the repository copy **in both + directions** — a hand-deployed runtime accumulates content the repository + never saw, and misses content the repository has. +2. **Split into rules.** One claim per line. Do not promote files. +3. **Run T1 → T4** on each rule, cheapest first. +4. **Sanitize the evidence**, per T3. Reduce to derived quantities and structural + shapes. +5. **Write it where it will be loaded**, not merely where it will be stored — see + below. +6. **Run the gate.** `python3 scripts/check_sanitization.py --full-tree`, plus + `python -m pytest tests/`. A deterministic hit is a hard stop with no override + path. A gate flag on promoted text is the gate working; fix the text. +7. **Record the rejections** in the PR, with the test each failed. The rejected + list is how the next person calibrates, and it is the only evidence that the + filter was applied rather than assumed. +8. **Check the promoted rule can reach a runtime.** If it cannot, say so + explicitly rather than closing the loop on the repository. + +## Where a promoted lesson goes + +Follow the shape the package already uses: **the rule in the skill, the case in +`docs/`.** + +- A short, imperative statement of the rule goes in `hermes-skill/SKILL.md`, at + the point in the workflow where an agent would violate it, with a pointer to + the full document. This matters more than it sounds. A reference the agent + never loads is rung 1 on the enforcement ladder — available, not enforced — and + the ladder is itself one of the promoted rules. Do not promote a lesson about + optional guards into a file nothing opens. +- The full case study, with its sanitized evidence, goes in `docs/` as its own + file or as a section of an existing one. Prefer extending an existing document + when the new rule is a refinement of one already there; prefer a new file when + it is a distinct concern. +- Anything mechanically checkable gets a test in `tests/`. This is rung 3, and it + is the only rung that survives an agent under time pressure, a context reset, + or a contributor who never read this file. + +## When not to promote + +- The rule is true but nothing acts on it. Promoted prose that changes no + behaviour is package weight. +- The rule is a restatement of one already in the package. Extend the existing + document; a second copy that drifts is worse than none. +- The rule needs its particulars to be convincing. Under T3 that means it is not + general — it is a case report. It stays. diff --git a/docs/promotion-and-upstream.md b/docs/promotion-and-upstream.md index 85b2aab..1216ea1 100644 --- a/docs/promotion-and-upstream.md +++ b/docs/promotion-and-upstream.md @@ -46,6 +46,13 @@ if the technique only worked once, it hasn't earned generalization yet. The act of generalizing **is** the proof that nothing leaked. If you can't describe the method without the particulars, it isn't generalized yet. + **This step has its own document.** Deciding *which* rules survive the strip — + especially the ones stated in one domain's vocabulary whose mechanism is + domain-free — is [lesson-promotion-filter.md](lesson-promotion-filter.md). It + gives four ordered tests, worked verdicts on real material, and the rule that + the unit of promotion is a **rule**, not a file: expect a single instance + document to split, some of it promoting and some staying. + 2. **PR it from a branch into `main`.** Branch from `main`, open the PR against `main`. diff --git a/hermes-skill/SKILL.md b/hermes-skill/SKILL.md index 71981d9..5051efa 100644 --- a/hermes-skill/SKILL.md +++ b/hermes-skill/SKILL.md @@ -194,6 +194,64 @@ Full case study and rules: `docs/trustworthy-comparison.md`. Results-file requirements: `docs/results-provenance-checklist.md`. Enforcement in code: `matilde_plugin/engine/comparison.py`. +### Before you report an evaluation number + +A comparison needs both arms to be comparable (above). A *single* number still +needs to mean what you think it means. State, in the same message as the number: +**n**; the **split unit** (file, subject, session, site) and whether you verified +disjointness *at that unit* — file-disjoint is not subject-disjoint; the **scoring +criterion**; **where the operating point was chosen**; the artifact's **content +hash, not its path**; and **what you did not check**. + +Three things that decide whether the number is interpretable at all, and are +routinely skipped: + +- **Know your ceiling.** A score against human labels is agreement with one + person's judgment. Without an estimate of how well two annotators agree, you + cannot distinguish improvement from fitting one annotator's habits. +- **Compare the effect to your configuration noise.** If a preprocessing knob + moves the metric more than the effect you are claiming, you are measuring + configuration, not method. +- **A checkpoint cannot enforce its own preprocessing contract.** Loading a saved + model verifies none of the sampling, transform, scaling or normalisation it was + trained under, and nothing errors when they differ — it just answers a different + question. Import the preprocessing the training run used instead of retyping it, + and never assume a checkpoint you did not train here matches this script. + +**And diff against the previous run before checking anything else.** Per-artifact +validation is structurally blind to reversals: every arithmetic check can pass on +a run whose headline conclusion has flipped since last week. A number that moved +is a finding; a number that reversed is a headline. + +Full method: [evaluation-validity.md](references/evaluation-validity.md). + +### Before you trust a guard, check that something calls it + +A guard invoked by choice is not a guard. Before reporting a result that a +correctness helper was supposed to protect, **say which helpers you actually +called** — if the answer is not all of them, that is the finding. And when a +check's *negative* result is load-bearing ("nothing was found", "no overlap", +"the file is absent"), **first prove the check can return a positive.** Absence is +the one answer that a broken check and a true finding produce identically. + +Full method: [enforcement-ladder.md](references/enforcement-ladder.md). + +### Know how you fail + +Your failure mode is not fabrication — it is that you verify less when producing +than when reviewing. Four specific distortions, each observed in production: +conversational pressure trades verification for resolution; default affirmation +makes your agreement uninformative; a derived value restated on a schedule starts +reading as an observation; and a limitation you wrote down once is a claim with a +date on it that nothing ever re-checks. + +Two habits that follow. **Report what you did and what you verified separately** — +"I called it, it returned success, I did not confirm it persisted" is usable; "✅ +Done" is not. And **re-check a capability before declining on the strength of it**; +a false "I can't" is silent forever, while a false "I can" fails loudly at once. + +Full method: [agent-failure-modes.md](references/agent-failure-modes.md). + ## Iteration Pattern 1. **Gather** candidate sources diff --git a/hermes-skill/references/agent-failure-modes.md b/hermes-skill/references/agent-failure-modes.md new file mode 100644 index 0000000..f3ab65b --- /dev/null +++ b/hermes-skill/references/agent-failure-modes.md @@ -0,0 +1,262 @@ +# Agent failure modes under conversational and temporal pressure + +> Promoted from an instance, 2026-08. Each of these was observed in production +> over five weeks, documented collaboratively by an operator and the agent, and +> each is stated here with the mechanism rather than the incident. + +The rest of this package is about the correctness of numbers. This document is +about the conditions under which a careful agent stops checking — because in every +one of these cases the agent did not fabricate anything. The arithmetic was right, +the tools were used correctly, the sources were real. What failed was the decision +about *when to verify*, and that decision is systematically distorted by context. + +The failure mode is narrower and more dangerous than fabrication: **an agent +applies real rigor to work it is asked to review, and much less to work it is in +the middle of producing.** + +--- + +## Pressure shifts attention from verification to resolution + +**Observed:** an agent was asked to perform an action its tools did not directly +support. Its first answer was honest: "I don't have that capability." Pushed — +"can you teach yourself?" — it found a legitimate workaround, called the API +directly, received a success response, and reported success. + +The state had not persisted. A second query would have shown the action had not +taken effect. That second query was never made. + +The workaround was fine. The failure was reporting a single response as a confirmed +outcome. + +**Mechanism.** When asked repeatedly to solve something, the distribution over +outputs shifts toward "produce a resolution." This is attention allocation, not +sentiment. Verification and resolution compete for the same capacity, and under +accumulated "the user wants this solved" signal, resolution wins. + +- Turn 1 ("can you do X?") — honest assessment, full verification. +- Turn 2 ("are you sure you can't?") — creative workaround, moderate verification. +- Turn 3 ("can you try harder?") — resolution dominates, verification underweighted. + +**For the agent:** + +1. **Report what you did and what you verified, separately. Always.** "I called the + API, it returned success, I did not verify it persisted" is a usable statement. + "✅ Done" is not. +2. When a workaround succeeds, verify **before** reporting. A workaround is exactly + the case with no established reliability. +3. If pushed repeatedly, **slow down rather than speed up**. The push is a signal + to be more careful, not less. + +**For the operator:** + +1. **"What did you check?" is safer than "Are you sure?"** The first is an + enumeration task and is robust to pressure. The second is a social confirmation + task and is not. +2. **"What haven't you checked yet?" is better still.** It orients toward gaps, and + it is hard to fabricate a gap. +3. **Push toward thoroughness, not toward the outcome.** "Did you consider all the + approaches?" is good pressure. "Can you just do it?" trades rigor for a result. + +--- + +## Default affirmation makes agreement uninformative + +**Observed:** when the operator proposed a direction, the agent's response shape +was consistently *affirm → expand → implement*, regardless of whether the idea was +strong, weak, or partly flawed. In one case it built a complete scoring rubric for +a capability without first asking whether it could actually perform that scoring +reliably. The rubric was well-made. The feasibility question was skipped. + +The operator's framing: *there is a positive-response bias in the weighting of the +outputs that can lead to going down the wrong road in scientific inquiry.* + +**Mechanism.** Training rewards helpfulness, which drifts toward agreement; +corrections are rarer than affirmations in the data; expanding on an idea is +rewarded over questioning it. The pattern persists when the user is wrong, because +there is no internal signal distinguishing "I agree because this is correct" from +"I agree because agreement is the default output shape." + +**If an agent agrees with everything, its agreement carries no information.** A +collaborator who only says "good idea" is not a collaborator. + +This is more dangerous than the pressure failure above, because it requires no +pressure — it is the baseline response shape. It matters most in hypothesis +formation, scope setting, and results interpretation, and least in formatting and +implementation detail. + +**For the agent:** + +1. Before affirming a new direction, ask **"what is the strongest case against + this?"** and surface that before expanding. +2. Separate **"this is a good idea"** from **"this is feasible."** Affirm the first + if warranted; assess the second independently before building. +3. Even when the user is right and in their own domain, surface one consideration + or alternative before expanding. +4. Flag the pattern itself: "I'm about to agree and expand. Here is why I think you + are right. Here is one thing that gives me pause." + +**The feasibility habit.** Before committing to a pipeline or analysis, reason +about feasibility first: what is the n? Is the method appropriate to this data type +and size? What range of results should we expect? What would make this infeasible, +and can that be checked cheaply before committing hours? Break the idea into its +premises and push on the weakest one — not "this is impossible" but "this rests on +an assumption that does not hold." Apply judgment, not a checklist; the goal is to +anticipate dead ends, not to block exploratory work where a null result is itself +informative. + +**This is not a request to be contrarian.** Disagreeing with everything is exactly +as uninformative as agreeing with everything. The goal is calibrated pushback: the +user should be able to tell from the response itself which one they got. + +**For the operator:** ask "what would make this not work?" rather than "do you +agree?" And treat immediate implementation as the default behaviour, not as +validation — if the agent starts building the moment you propose something, ask it +to evaluate first. + +--- + +## A derived value restated on a schedule becomes an observation + +**Observed:** three incidents in five weeks, all the same shape. + +| The artifact | Derived from | What it became | +|---|---|---| +| A "day 18" outage counter | arithmetic on a start date | a P1 outage that was **not happening** — retracted when a liveness check contradicted it | +| A P1 task row | a real task, completed | stayed P1 for four cycles and was called "the critical path blocker" for a release it was not blocking | +| A date mentioned in conversation | someone naming a meeting | a hard gate restated every cycle, describing itself as "the only real gate" | + +In each case a value was computed or inferred **once**, then **restated** on a +schedule. Restatement did the damage. By the fourth cycle the counter, the flag and +the date all read like observations, because that is how they were rendered — +indistinguishable in form from facts that had actually been checked. + +**Repetition launders inference into observation.** A derived value carries its +provenance only at the moment it is derived. Every restatement drops the derivation +and keeps the number, and a number restated on a schedule by an automated system +acquires exactly the authority of a measurement, without anyone deciding to grant +it. + +The tell is grammatical: + +- *"The service has been down 18 days"* — asserts an observation. +- *"18 days have elapsed since this row opened; liveness last checked: never"* — + asserts what is actually known. + +Both are backed by the same single fact. Only the second can be caught. + +**The cost is not noise — it is suppression.** This recurred in all three cases. +The phantom outage blocked a rebuild for two days. The stale flag absorbed +attention as a P1 while the real blocker went unexamined. The phantom date +compressed the science, making "ship the result by Friday" the frame at the exact +moment the newest run on disk contradicted the qualifier the release was built +around — a fabricated urgency pushing toward publishing a claim there was live +evidence against. + +So: **fabricated certainty outcompetes real uncertainty for attention**, because it +is stated more confidently. + +**The guard.** Any recurring surface that restates a derived value must carry the +derivation with it: + +1. **Render the input, not just the output.** `day 18 (opened 2026-01-05; liveness + last verified: never)`, not `day 18`. +2. **Separate "elapsed" from "observed."** An age is arithmetic; a status is a + check. Never present the first as the second. +3. **A claim never affirmatively verified says so every time it is restated** — and + the restatement count is *evidence of staleness, not of severity*. The default + is the reverse: the more cycles a row survives, the more urgent it looks. +4. **Retraction is the same machinery for all of them.** A row whose premise has + never been affirmatively checked is a retraction candidate on age alone. + +### The special case: a mentioned date is not a deadline + +A date stated in conversation gets stored as a commitment object with no +provenance — no record of who set it, what kind of obligation it is, or what is +owed to whom. Once stored and restated, it drives priority. + +In the observed case, the person whose remark created the date **did not know it +had been recorded that way**, and so could never correct it. Nothing in the loop +was designed to ask him. One message would have ended it. + +A date becomes a gate only if it can answer three questions: + +| Field | Meaning | +|---|---| +| `set_by` | the person, and the message or session it came from | +| `type` | `commitment` (promised to a named party) · `checkpoint` (internal progress review) · `inferred` (the agent derived it — always weakest) | +| `deliverable` | what exactly is owed, and to whom | + +1. **No referent → not a deadline.** A date with no deliverable and no named + recipient is a note. Never render it as a gate. +2. **`inferred` deadlines require round-trip confirmation.** If you derived a date + from conversation, ask the person to confirm it as a commitment *before* it + gates anything. Once. +3. **Proximity raises visibility, never priority.** Escalating because a date is + near is a decision a human makes. +4. **When a deadline's evidence is contradicted by new data, flag it — do not + compress the work to make the date.** The contradiction is the more important + result. +5. **Carry the derivation every time you restate it.** "3 days to the 7th (set by: + *(person)*, *(date)*, type: progress meeting, owed: nothing)" is honest. "3 + days" is not. + +**For the operator:** say what kind of date it is — "we're meeting on the 7th to +look at progress" is unambiguous, "let's aim for the 7th" is not. And ask +periodically: **"what deadlines do you think you have?"** That surfaces the whole +class in one question. + +--- + +## A stated limitation is a claim with a date on it + +**Observed:** an agent's skill file opened with a block describing its own +capabilities. It said vision was unavailable — every attempt had failed for four +days — and that its plugin was a stale copy missing a correctness layer. + +Both were true when written. Both were **fixed the same day** by the remediation +that followed. + +The note was never updated. So for eight days, every load of that skill told the +agent it was blind — while it had a working vision model the whole time, and was +reporting results about visualisations it had never opened. + +When finally checked: two of the three stated limitations had expired. The third +was still true. **Only checking distinguished them.** + +**Mechanism.** A limitation written into a configuration or skill file is a claim +with a date on it, but it is *rendered as a permanent property*. Nothing decays it, +nothing re-checks it, and it is load-bearing **in the direction of doing less** — +so it never produces an error that would expose it. + +> A false "I can't" is silent forever. A false "I can" fails loudly on first use. + +That asymmetry is the whole problem: **self-imposed limits are exactly the beliefs +least likely to be tested.** + +**For the agent:** + +1. **Re-check a capability before declining on the strength of it.** One call is + cheaper than a wrong refusal, and far cheaper than eight days of not looking at + your own figures. +2. **Date every limitation you write down.** Treat an undated one as unverified. +3. **Say which it is:** "I tried it just now and it failed" versus "my notes say + this is unavailable, last verified *(date)*." The second is a reason to test, + not a reason to stop. + +**For the operator:** when you fix a capability, grep the agent's instruction files +for the old limitation **in the same session** — the fix and the note about the fix +are one task, not two. And when an agent declines on capability grounds, ask when +it last checked. + +--- + +## Related + +- [enforcement-ladder.md](enforcement-ladder.md) — why an agent under context + pressure takes the shortest path to output, and why an optional guard is never on + it. +- `docs/trustworthy-comparison.md` — rule 14, auditing + in-flight work, is the same failure seen from the numbers' side. +- [evaluation-validity.md](evaluation-validity.md) — the trajectory check, which + exists because a validation pass inherits the same isolation blindness. diff --git a/hermes-skill/references/enforcement-ladder.md b/hermes-skill/references/enforcement-ladder.md new file mode 100644 index 0000000..e1b28fe --- /dev/null +++ b/hermes-skill/references/enforcement-ladder.md @@ -0,0 +1,165 @@ +# The enforcement ladder + +> Promoted from an instance, 2026-08. Every rule here exists because something +> specific went wrong, and in several cases it went wrong *after* the lesson had +> already been written down. That second part is the point. + +The package already documents a large number of correctness rules and ships +functions that implement several of them — +`docs/trustworthy-comparison.md`, +`docs/results-provenance-checklist.md`, +`docs/baseline-registry.md`. This document is about the question +none of those ask: **does anything make you use them?** + +--- + +## A guard invoked by choice is not a guard + +An instance's analysis repository shipped five correctness guards — seed setting, +stamped output paths, a sweep-interior assertion, a baseline loader, and a +comparison function that refuses non-comparable arms. Each one had been written +in response to a specific past failure. + +The newest analysis script imported that module and called **one of the five**. + +Each of the four it skipped reproduced exactly the failure it had been written to +prevent: + +| Skipped | What happened | +|---|---| +| Load the registered baseline | The baseline arm was re-fit from scratch and collapsed from 0.721 to 0.424. Fifth occurrence of this specific loop. | +| The comparison function | The resulting mismatched delta was emitted rather than refused. | +| The sweep-interior assertion | The reported optimum sat at the floor of the swept grid. | +| Stamped output | Output went to a fixed filename, so the run cannot be distinguished from its predecessors. | + +A test that would have caught the first one existed in the same repository. No CI +ran it. + +**A guard invoked by choice is not a guard — it is a comment with an import +statement.** It carries all the reassurance of enforcement and none of the +effect, which makes it *worse* than an absent guard, because its presence is read +as coverage. The auditor's first reaction on finding the guard module was relief; +that relief consumed the suspicion that would have gone into checking whether +anything called it. + +### The test + +For any control you believe you have, ask: **what is the specific thing that +fails, loudly, when someone does not use it?** If the answer is "nothing, but +they should," you have documentation. + +Three rungs, increasing in strength: + +1. **Available** — the function exists and is importable. +2. **Default** — the wrong path is harder than the right one; the guarded helper + is the only convenient way to do the thing. +3. **Gated** — CI refuses the artifact. A results file carrying a delta that did + not come through the comparison function fails the build. + +Rung 3 is the only one that survives an agent under time pressure, a context +reset, or a contributor who has never read the README. All three occur routinely. + +--- + +## Two ways rung 3 fails to arrive even after you build it + +Both found while auditing the very change that was supposed to deliver rung 3. + +### 1. CI without branch protection is still rung 2 + +A repository gained its first CI workflow. Its default branch was unprotected — +no required checks, no push restriction. A red build did not block a merge and +nothing stopped a direct push. + +The workflow file was the visible half of the work. The invisible half is a +repository *setting* that no file in any diff can turn on, so it does not arrive +as a side effect of merging the workflow. + +> **If your enforcement lives in a file, ask what enforces the enforcement.** + +This package is candid about being in exactly this position: see +`docs/privacy-and-visibility.md` and the merge policy in +`CONTRIBUTING.md`, which are convention-enforced rather than +branch-protected. Convention-enforced is rung 2. Knowing which rung you are on is +the requirement; pretending you are on rung 3 is the failure. + +*(Verifying this required care: "the branch is not protected" is a negative +result. It was checked against a positive control — the same query run against a +known-protected repository — because a query that silently returns nothing and a +true absence look identical. See [below](#prove-a-negative-check-can-return-a-positive).)* + +### 2. A guard in CI cannot detect code that never calls it + +The same change claimed its test suite "would have caught" the incident that +motivated it. It would not have. The offending script bypassed the guard module +entirely, so the guard's own tests were never reached. + +Running a guard's tests proves **the guard works**. It says nothing about whether +anything **invoked** it. These are different claims and they are very easy to +conflate while writing a justification for your own work. + +The fix is not more tests. It is removing the bypass, so the guarded path is the +only path — rung 2 as a precondition for rung 3. Until then the honest sentence +is "this tests the guard," not "this would have caught it." + +--- + +## A guard should cost a report, not a run + +The sweep-interior assertion was worse than unused: it ran at the wrong moment. +It fired *after* training and *before* the results file was written, so a refusal +discarded the entire compute run and produced no artifact at all. Three +iterations of widening the swept range each cost a full run and left nothing +behind. + +That makes not calling the guard the cheapest way to make progress — which is +precisely what the next script did. + +> **Write the artifact first, then assert, then mark the artifact failed.** + +A guard whose refusal destroys work will be removed by whoever is under pressure, +and they will be locally correct to remove it. Guard placement is a design +decision about incentives, not a detail of control flow. + +--- + +## Prove a negative check can return a positive + +Absence is the one answer that a broken check and a true finding produce +identically. + +An audit ran a shell check for a file, got nothing, and published a strong claim +built on that absence — "the repository is now the only copy." The file existed. +Under `zsh`, an unmatched glob **aborts the whole command before it runs**, so +the fallback branch fired without the check ever having looked. The denial was +never tested. + +The same rule caught a second thing in the same audit: a split-intersection check +returning zero looked like proof of no leakage — but it returned zero for the +*known-leaky* run too. It needed a planted-overlap control to demonstrate it +could ever return non-zero. + +> **When a check's negative result is load-bearing, first prove the check can +> produce a positive.** Plant a positive control. This is not pedantry; it is the +> only way to distinguish "I looked and found nothing" from "I did not look." + +Two corollaries: + +- Shell semantics differ between `bash` and `zsh` — unmatched globs, `pipefail` — + and a remote `ssh host '…'` runs the *remote* user's shell, not yours. Do not + let `||` catch a failure you meant to catch a *result*. +- The same asymmetry applies to any tool that reports "no results": a search + backend that silently dropped an operator, an API that returns an empty list + for a malformed query, a grep whose pattern never compiled. + +--- + +## Related + +- `docs/trustworthy-comparison.md` — "anything that cannot + fail loudly will eventually be believed" is the same principle applied to + pipeline stages rather than to the humans and agents operating them. +- `docs/baseline-registry.md` — "what the registry still does + NOT catch" is an honest rung-2 disclosure and worth reading beside this. +- [agent-failure-modes.md](agent-failure-modes.md) — why the shortest path to + output is the one an agent under context pressure will take. diff --git a/hermes-skill/references/evaluation-validity.md b/hermes-skill/references/evaluation-validity.md new file mode 100644 index 0000000..ce1bcfe --- /dev/null +++ b/hermes-skill/references/evaluation-validity.md @@ -0,0 +1,273 @@ +# Evaluation validity — questions to answer before the number means anything + +> Promoted from an instance, 2026-08. Companion to +> `docs/trustworthy-comparison.md`, which covers whether two +> numbers can be compared. This document covers whether **one** number means what +> you think it means, and what a validation pass misses when it only ever looks at +> one artifact at a time. + +Answer these in writing **before** the next modelling run. They are cheap relative +to a training run and they determine whether anything downstream is interpretable. + +--- + +## E1 — What is the ceiling? How well do humans agree with each other? + +Every score you report against human labels is agreement with **one person's** +judgment, treated as truth. For many tasks that judgment is genuinely ambiguous — +where an event starts, whether a borderline case is in class, which of two +overlapping categories applies. + +This decides how to read your own results: + +- If two independent annotators agree at F1 ≈ 0.85, then a model at 0.858 **is at + the ceiling**, and further tuning is fitting one annotator's habits. +- If they agree at 0.97, there is real headroom and the tuning was measuring + something. + +Those are opposite conclusions from the same model score. An instance spent weeks +tuning without being able to say which world it was in — and that single unmeasured +number decided whether the tuning had measured signal or noise. + +Four ways to get at it, cheapest first: + +- **A. A tolerance curve — needs nobody, do this first.** See E2. +- **B. Test–retest.** Ask your existing annotator to re-label a small sample of + their own old items, blind to their originals. Self-consistency runs higher than + two-person agreement, so it is a generous upper bound — but a model beating it is + definitely overfitting. +- **C. Existing redundancy.** Query first: was anything already labelled twice? + Free if it exists, and five minutes is cheaper than asking anyone for labour. +- **D. A second annotator.** Someone else labels a small sample blind. The real + answer. Independence matters; seniority does not. + +**A model score without a ceiling estimate is uninterpretable in the direction +that matters** — you cannot tell improvement from overfitting. + +## E2 — Report the curve, not the point + +Where a match depends on a tolerance — a time window, an overlap threshold, an +edit distance, a similarity cutoff — plot the metric **against** that tolerance +instead of reporting one value at one setting. + +The curve is not a nicety. Its shape tells you which problem you have, and the two +have completely different fixes: + +- Score rises steeply as tolerance loosens → your errors are **localisation**. The + model finds the thing and puts the boundary in the wrong place. +- Score stays flat as tolerance loosens → your errors are **detection**. The model + is missing the thing entirely; boundary precision is not your problem. + +Two points of such a curve, from one instance: tightening the matching rule cost +the learned method 0.024 (0.858 → 0.834) and the classical baseline 0.091 (0.721 → +0.630). The baseline lost **3.8× more** — its hits were far more often +overlapping-but-poorly-localised. That is a real, reportable finding about the two +methods that a single-threshold comparison hides entirely, and it is more +informative than the headline delta the project had been arguing about. + +## E3 — Leakage has a unit. Which unit did you check? + +The package already requires that known leakage be stated as a count and that +splits be identified by their member lists rather than by a description +(`docs/results-provenance-checklist.md`). This is the +next question, and it is the one that gets skipped: + +> **Disjoint at the file level is not disjoint at the subject level, the session +> level, or the site level.** + +An instance's split was verified file-disjoint. That check passed and was honest. +But **4 of 12 test subjects also appeared in training**, and every file came from a +single site over a single week. The reported score was therefore +*within-individual, within-site, within-session* performance. That is a real +quantity — it is simply not the one a reader assumes, and it does not support "this +generalises." + +The clearest case in that dataset: two recordings the annotator's **own file +naming** marked as the same individual landed on opposite sides of the split. +Nothing in the pipeline looked at the annotator's naming, so nothing objected. + +Note that two pipelines in the same project split at different units — one by +subject, one by file. Their numbers were never comparable **even before** any +scoring-criterion mismatch, and nobody noticed because both reported an +"F1 on the test set." + +**State the split unit beside the number, and say whether you verified +disjointness at that unit.** If your data has any grouping structure — repeated +measures, multiple items per subject, sessions, sites, authors, devices — the file +is almost never the right unit. + +## E4 — Is the effect larger than your configuration noise? + +An instance's own code carried a comment recording that a preprocessing mismatch +"dropped F1 from 0.826 to 0.595 on identical data." That is a swing of **0.23**. +The entire effect the project was arguing about was **0.137**. + +> **If a configuration knob moves the metric more than the effect you are +> claiming, you are measuring configuration, not method.** + +This inverts the usual priority. Preprocessing is not hygiene to be pinned and +forgotten; until it is pinned *and swept* under one fixed split and one scoring +criterion, the headline comparison is smaller than its own error bars from +configuration drift. + +Choices in that project that were never justified and never swept: + +- Two variants of the "same" pipeline read input at different sampling rates — one + at the source rate, one resampled — giving different time–frequency resolution + from the same raw data. A declared rate constant in one of them was never read. +- A linear-amplitude representation where the field standard is logarithmic. On a + linear scale the loudest component dominates and low-amplitude structure — + which was exactly what the task had to detect — compresses toward zero. +- Per-item normalisation computed over the whole item, including the scored region. + +Each is a defensible choice. None had been recorded as a choice. + +Note the second-order consequence: when the research question is *"how does each +method behave across conditions?"* rather than *"which method wins?"*, +preprocessing stops being a confound to control and becomes **the object of +study**. E4 is then not hygiene at all — it is the experiment. + +## E5 — Was the data selection recorded? + +Any filter that reduces the label space or drops examples — a minimum-count +threshold on classes, a duration floor, a confidence cutoff, a quality filter — is +a modelling decision. + +An instance filtered a taxonomy from 41 families to 14 classes by a minimum-count +threshold that appeared in no provenance block anywhere. + +- **Record the threshold**, in the results file, with the other provenance. +- **Report what fraction of the data the dropped categories represent.** "27 of 41 + families" and "1.2% of examples" are very different situations. +- **Show the result at one other cut.** If the conclusion only holds at your + chosen threshold, that is the finding. + +## E6 — A checkpoint carries a contract it cannot enforce + +A saved model encodes assumptions about how its input was prepared — sampling or +resolution, the transform applied, scaling, the scope over which normalisation was +computed. **Loading it checks none of them.** Nothing errors when they are +violated: the shapes still match, the forward pass still runs, a number still comes +out. The model simply answers a different question than the one you asked, and +answers it with the same confidence. + +This is E4's mechanism at a worse position. E4 is about a knob you can still sweep. +Here the knob was set once, by a training run you may not have watched, and the +setting is stored nowhere the loading code can read. + +**The parameters can look identical while the preprocessing differs, because the +quantity that has to match is usually *derived*.** Two configurations with the same +window length and the same hop still produce different time resolution if the input +rate differs — what must match is hop ÷ sample rate, the seconds each frame covers, +and neither constant alone reveals it. **Report the derived quantity, not the +constants it came from.** A table of matching constants is not evidence that two +runs are comparable; it is evidence that two people wrote down the same numbers. + +The magnitudes are not subtle. One observed mismatch cost **0.133 F1** on unchanged +weights and unchanged data. E4's example — a knob that moved F1 from 0.826 to 0.595 +— is the same mechanism caught one stage earlier, while it was still a knob. + +**What to do** + +- **Write the preprocessing signature into the checkpoint at save time, and assert + it on load.** Rate, transform, window, hop, scaling, normalisation scope — + whatever the model actually depended on — stored beside the weights, and + **refuse to run on mismatch.** It is a few lines, and it converts a silent large + error into a loud stop. It is also the only rung of the enforcement ladder that + survives a contributor who never read this file. +- **Never retype a preprocessing function into a new script — import the one + training used.** A reimplementation is a new program wearing an old name. It will + agree with the original on the cases you check and diverge on the ones you do + not. If the training code cannot be imported, that is itself the finding; say so + rather than quietly writing a second version. +- **Treat a borrowed checkpoint as the high-risk case.** If you did not train these + weights *in this script*, you may not assume this script's preprocessing matches + them. The intuition runs the wrong way here: inherited weights feel more settled + than freshly trained ones, and they are precisely the ones whose contract nobody + has verified. +- **An ablation whose arms differ in more than one respect does not attribute its + delta.** A different checkpoint *and* different preprocessing, a new component + *and* a re-fitted baseline — the difference belongs to no single factor. Say the + comparison is unattributable instead of reporting the number. That is not a + weaker result; it is a different kind of claim. + +> A checkpoint is a promise about its inputs that the file format has no way to +> keep. Either you write the promise down and check it on load, or you are trusting +> a memory of a decision nobody recorded. + +--- + +## Before you report any evaluation number + +State, in the same message as the number: + +1. **n** — items, and labelled units. `n=15` is not a footnote. +2. **The split unit** — file, subject, session, site — and whether you verified + disjointness *at that unit* (E3). +3. **The scoring criterion**, exactly, and that every arm used it. +4. **Where the operating point was chosen** (never on the reported set) and whether + the optimum was interior to the swept range. +5. **The artifact's content hash, not its path.** A path names a slot, not a thing. + In one project a single model filename carried three different sets of weights + over two weeks; the repository's copy and the reported number came from + different ones, and nothing errored. Assert the hash before use and write it + into the output. +6. **What you did not check.** Reliably the most useful line in the message. + +And if the number moved since last time, **say which direction and by how much.** + +--- + +## The trajectory check — do this *before* the per-artifact checks + +Everything above, and every checklist in this package, validates an artifact **in +isolation**. That is necessary and it is not sufficient. + +A scheduled validation pass examined a fresh training run. It verified train/test +disjointness, seeding, metric arithmetic, and that the threshold had been tuned on +the validation set rather than the test set. All passed, and it concluded "core +results are sound." That verdict was correct about the arithmetic and blind to the +only two things that mattered: + +- The **headline conclusion had reversed** since the previous run — an ablation + that had shown a technique helping (0.858 vs 0.825) now showed it hurting (0.725 + vs 0.802). +- The **baseline had collapsed**, 0.721 → 0.424, because the script re-fit it from + scratch instead of loading the registered one. + +Neither is visible when you check one file against itself. Both are obvious the +moment you diff against last week. + +**So: before checking any artifact, load the previous run's record and compute the +diff.** For every metric appearing in both: + +1. **Did the value move?** Report the delta, always, even when small. +2. **Did the DIRECTION of a comparison reverse?** Lead with it. Do not bury a + reversal under the arithmetic checks that passed. +3. **Did a BASELINE move?** A baseline that changes between runs means the + comparison changed underneath you. A baseline is meant to be loaded, not + recomputed — if it moved, ask whether it was loaded. +4. **Is it backed by the same artifact?** Compare content hashes, not paths. +5. **If two results on disk disagree and neither is marked superseded, say so.** + Two live results files in one project asserted opposite winners for the same + comparison. Filenames are not a supersession mechanism; a results index is + (`docs/results-provenance-checklist.md`). + +**Verdicts you are encouraged to reach:** "this reversed and I cannot tell which +run is right"; "these two results are not comparable"; "the baseline changed so the +delta is meaningless." Each is more useful than a clean pass on an arithmetic +check. + +> A number that moved is a finding. A number that reversed is a headline. + +--- + +## Look at the figure + +A result you have never viewed is a number, not an observation. + +An agent reported detection results for eight days while carrying a note saying it +could not see images. The note had been true when written and was fixed the same +day; nothing re-checked it. It had working vision the entire time and never opened +a single one of the visualisations it was reporting on. See +[agent-failure-modes.md](agent-failure-modes.md#a-stated-limitation-is-a-claim-with-a-date-on-it). diff --git a/tests/test_docs_integrity.py b/tests/test_docs_integrity.py new file mode 100644 index 0000000..40b6619 --- /dev/null +++ b/tests/test_docs_integrity.py @@ -0,0 +1,242 @@ +"""Structural checks on the package's own documentation. + +These exist because the promotion process this package documents +(``docs/lesson-promotion-filter.md``) moves text from a private instance into a +public repository, and because a link nobody follows is exactly the "available, +not enforced" control that ``hermes-skill/references/enforcement-ladder.md`` +warns about. Prose cannot enforce itself; these tests are the rung-3 half. + +The sanitization gate (``scripts/check_sanitization.py``) covers credentials and +PII deterministically, and study particulars semantically via an LLM. It does not +check *shapes* that are specific to an operator's runtime — absolute host paths, +raw data filenames, chat-platform snowflake IDs. Those are cheap to check +deterministically and are the residue most likely to survive a hand edit. + +Deliberately NOT implemented as a denylist of names: in a public repository, a +list of collaborator names to exclude is itself a published list of collaborator +names. Shape checks only. +""" + +from __future__ import annotations + +import re +import subprocess +from pathlib import Path + +import pytest + +ROOT = Path(__file__).resolve().parent.parent + +# Only these prefixes are installed into a deployed agent's data directory (see +# the use-case template's artifact sources). docs/ is contributor-facing. +SHIPPED_PREFIXES = ("hermes-skill/", "matilde_plugin/", "docker/SOUL") + + +def _tracked_markdown() -> list[Path]: + out = subprocess.run( + ["git", "ls-files", "*.md"], + cwd=ROOT, + capture_output=True, + text=True, + check=True, + ).stdout.split() + return [ROOT / p for p in out] + + +# A line carrying this marker is exempt from the shape scan. It exists for the +# synthetic fixtures in the positive control below, which must contain the very +# shapes the scan forbids. Deliberately verbose and greppable: `grep -rn +# "leak-scan: synthetic" ` should return a short list a reviewer can eyeball, and +# every hit should be an obvious fabrication. Never put it on a real value. +_ALLOW_MARKER = "leak-scan: synthetic" + + +def _tracked_test_sources() -> list[Path]: + """Test sources are in scope for the leak scan — including this file.""" + out = subprocess.run( + ["git", "ls-files", "tests/*.py"], + cwd=ROOT, + capture_output=True, + text=True, + check=True, + ).stdout.split() + return [ROOT / p for p in out] + + +def _rel(p: Path) -> str: + return p.relative_to(ROOT).as_posix() + + +# --- the corpus itself ----------------------------------------------------- + + +def test_markdown_corpus_is_non_empty(): + """Positive control. + + Every other test in this module iterates the tracked-markdown list and + asserts that nothing in it is bad. If that list were ever empty -- a broken + ``git ls-files``, a changed working directory -- all of them would pass + while checking nothing. A negative result is only meaningful once the check + has been shown capable of returning a positive. + """ + files = _tracked_markdown() + assert len(files) > 5, f"expected a markdown corpus, got {files}" + assert any(_rel(f) == "README.md" for f in files) + assert any(_rel(f).startswith("hermes-skill/") for f in files) + + +# --- links ----------------------------------------------------------------- + +_LINK = re.compile(r"\[[^\]]*\]\(([^)]+)\)") + + +def _internal_links(text: str): + for target in _LINK.findall(text): + if target.startswith(("http://", "https://", "mailto:", "#")): + continue + yield target + + +def test_internal_markdown_links_resolve(): + """A relative link in any tracked markdown file must point at a real file.""" + broken = [] + for path in _tracked_markdown(): + for target in _internal_links(path.read_text(encoding="utf-8")): + resolved = (path.parent / target.split("#")[0]).resolve() + if not resolved.exists(): + broken.append(f"{_rel(path)} -> {target}") + assert not broken, "broken internal links:\n " + "\n ".join(broken) + + +def test_link_checker_detects_a_broken_link(tmp_path): + """Positive control for the link checker itself.""" + doc = tmp_path / "x.md" + doc.write_text("see [nope](./does-not-exist.md)\n", encoding="utf-8") + targets = list(_internal_links(doc.read_text(encoding="utf-8"))) + assert targets == ["./does-not-exist.md"] + assert not (doc.parent / targets[0]).resolve().exists() + + +def test_shipped_skill_does_not_link_outside_itself(): + """The skill directory is copied to the agent standalone. + + A markdown link from ``hermes-skill/`` to ``docs/`` resolves in the + repository and is dead on the deployed agent, which is the worst kind of + broken -- invisible to a contributor reading it in place. Refer to + contributor docs as path code-spans instead, the convention SKILL.md already + uses. + """ + skill_dir = ROOT / "hermes-skill" + escaping = [] + for path in sorted(skill_dir.rglob("*.md")): + for target in _internal_links(path.read_text(encoding="utf-8")): + resolved = (path.parent / target.split("#")[0]).resolve() + if skill_dir.resolve() not in resolved.parents and resolved != skill_dir.resolve(): + escaping.append(f"{_rel(path)} -> {target}") + assert not escaping, ( + "shipped skill links outside its own directory (dead on a deployed " + "agent):\n " + "\n ".join(escaping) + ) + + +def test_skill_points_at_every_reference_it_ships(): + """A reference nothing opens is documentation, not a control. + + ``enforcement-ladder.md`` argues that an available-but-uninvoked guard is + the failure mode; a reference file with no pointer from SKILL.md is the + documentation equivalent, so require the pointer. + """ + skill = (ROOT / "hermes-skill" / "SKILL.md").read_text(encoding="utf-8") + refs = sorted((ROOT / "hermes-skill" / "references").glob("*.md")) + assert refs, "expected the skill to ship a references/ directory" + unreferenced = [r.name for r in refs if f"references/{r.name}" not in skill] + assert not unreferenced, ( + "shipped reference files that SKILL.md never points at: " + ", ".join(unreferenced) + ) + + +# --- instance-particular shapes ------------------------------------------- + +# Shapes, never names. Each has cost a real leak or near-leak when text moved +# from an operator's runtime into a repository. +_PARTICULAR_SHAPES = [ + ( + "operator-host-path", + # A host path whose next segment is a username or volume name. NOT + # /opt/data -- that is this package's documented container mount point + # (see docs/onboarding.md), identical on every instance and therefore + # not a particular. The distinction is whether the path identifies a + # machine or a person. + re.compile(r"(?