# Designing and Reviewing Signatures and Parameters

One document, three phases. Read the phase you are in.

| Phase | Part | Use when |
|---|---|---|
| Design | [Part 1 — Ambiguous Shapes](#part-1--ambiguous-shapes) | Naming a tool or parameter, or settling a value space, before it exists |
| Audit | [Part 2 — Review Passes](#part-2--review-passes) | Checking tools and parameters that already exist, before a release |
| Fix | [Part 3 — Solving It For The Caller](#part-3--solving-it-for-the-caller) | Deciding what a rejection should say and do |

Part 1 asks whether the value space is well defined. Part 2 asks whether the implementation
honors it. Part 3 is what to do about anything either turns up.

---

# Part 1 — Ambiguous Shapes

Eight shapes that are ambiguous *by construction*. No amount of careful wording fixes them;
each has to be disambiguated explicitly, because a competent reader can derive two different
correct-looking meanings from the same signature.

For each: the collision, the readings it produces, and the facts the description must state.

---

## 1. The signed integer — count, index, and sentinel collide

**The principle: any integer whose sign or zero carries a second meaning is two parameters
wearing one type.** The instances below span languages and surfaces because the collision is in
the type, not the surface.

JavaScript's `indexOf` returns `-1` for not-found, so `if (s.indexOf(x))` is wrong for a match
at position 0 — the most-hit instance of this defect anywhere. C's `read()` returns a byte
count *or* `-1`, and `ssize_t` exists only to make room for that one sentinel; `snprintf`
returns what it *would* have written, not what it did.

**Where the language offers a second channel, use it.** Rust's `Option<usize>` and
`Result<usize, E>`, TypeScript's `number | null`, Python raising rather than returning `-1`,
Go's `(n, err)`. A documented sentinel is a worse fix than a type that cannot carry one.

A single `int` routinely carries three unrelated meanings at once.

| Value | Count reading | Index reading | Sentinel reading |
|---|---|---|---|
| `5` | the first 5 items | the item at position 5 | — |
| `-5` | the last 5 items | the item 5 from the end | — |
| `0` | zero items | the first item (0-based) | **all / unlimited** |
| absent | the default | the default | the default |

Three traps:

**`0` usually reverses its natural reading.** In count-space `0` means "none". APIs
overwhelmingly overload it to "unlimited". A caller reasoning from first principles gets the
*opposite* of the truth, so `0` can never be left to inference — even when it seems obvious.

**"Negative means from the end" is not enough.** It does not say how many items come back.
Python makes the distinction visible: `a[-5:]` returns up to five items — fewer if the sequence
is shorter — while `a[-5]` returns exactly one and raises `IndexError` on short input. Both are
"from the end", yet they differ in arity *and* in short-input behavior:

```
positive keeps its first N lines, negative keeps its last N lines, 0 keeps complete content
```

not

```
negative counts from the end
```

**Count and index cannot share a parameter.** If `5` might mean "five items" or "the item at
index 5", no wording rescues it — rename, or split into two parameters.

**Required facts:** what positive selects, what negative selects, what `0` selects, what
omission selects. Four facts, every signed parameter, every time.

## 2. Bound inclusivity — the invisible off-by-one

`until`, `end`, `max`, `to`, `before`, `seq_to`, `depth`. Nothing in any of those names says
whether the endpoint is included.

Precedent is genuinely split: `range(0, 5)` excludes `5`; SQL `BETWEEN` includes it; slices
exclude; a message-index `seq_to` usually includes. The reader has no basis to guess.

**Required fact:** the word "inclusive" or "exclusive". One word.

Same for `depth`: does `depth=1` mean the node itself, or the node plus one hop?

## 3. Empty, absent, and null are three states, not two

`query=""`, `query` omitted, and `query=null` are distinct inputs that routinely behave
differently. The storage layer bites too: one project found optional columns normalized `NULL`
to `""` on read, so the historical null could no longer be distinguished from empty without a
migration.

Guard the reverse case: does an empty filter match *everything* or *nothing*? Both are
defensible; only one is implemented.

**Required facts:** what omission does, and what an explicitly empty value does when it differs
from omission. If they are identical, say so — the reader cannot derive that either.

## 4. Filter versus presentation — does this change *which*, or only *how much*?

The most consequential ambiguity for AI callers. Given `lines_per_message`, a caller cannot tell
whether trimming displayed lines also drops results, changes ranking, or alters pagination.

Guess "it filters" and they avoid it, blowing their context budget. Guess "cosmetic" when it
actually filters and they conclude "no matches" from a truncated view.

**Required facts:** state the negative space explicitly. A presentation parameter must say what
it does *not* affect:

```
This presentation window does not change matches, ranking, result count, pagination,
context membership, or reference extraction.
```

Long, and it earns the length. This is the one place where enumerating what a parameter does
*not* touch beats a shorter positive statement, because the reader's default assumption is
wrong and expensive.

## 5. Pagination — before or after what, and in what order?

`limit` and `offset` interact with filtering, deduplication, ranking, and context expansion, and
the order is invisible from the signature.

- Does `limit` cap rows scanned, rows matched, or rows returned after dedup?
- Does `offset` skip filtered or unfiltered rows?
- Do context rows count against `limit`?
- Is ordering stable enough that paging is coherent at all?
- **Is the ordering by relevance, or by something arbitrary?**

Unstable ordering makes `offset` silently lossy: page 2 can omit rows that moved to page 1.

The last question corrupts every capped result when unanswered. Observed: one tool documented
"ranked by relevance" while its sibling ordered by `session_id, seq`, disclosing that only in a
response field. A 30-hit sample returned unrelated sessions and missed known-relevant ones —
they sorted later, not lower. An agent reading that concludes the corpus is irrelevant.

**Required facts:** what the count counts, whether ordering is deterministic, and **what the
ordering is** — stated where the caller reads `limit`. A fact disclosed after the call cannot
inform the call.

## 6. Units — the name usually omits them

`size`, `max`, `width`, `timeout`, `context`. Bytes or characters? Characters or graphemes?
Lines or messages? Milliseconds or seconds?

**Fix at the name, not the description:** `preview_chars`, `timeout_ms`, `lines_per_message`.

`context` is worth calling out: in a search API it usually means *neighboring turns*, but a
reader may reasonably take it as characters of surrounding text.

## 7. A filter that can silently become a no-op

When one filter matches against several fields, some inputs select *everything*. The caller gets
a full result set that looks filtered, and nothing in the response says otherwise.

Observed: `path_prefix` matched "working directory, git repo, **or transcript path**". Every
transcript for one agent lived under `~/.claude/`, so `path_prefix="~/.claude"` — the most
natural value to type when scoping to that agent's work — matched every session ever recorded,
including repositories in unrelated trees. Documented behavior; undocumented *consequence*; and
invisible, because plausible results come back.

This is the zero-result defect (pass B5) inverted. That one is an empty answer that looks like a
miss; this is a complete answer that looks narrowed. Both come from the response not saying what
it actually filtered on.

**Required facts:** name each field the filter tests, and state which inputs match broadly. The
sturdier fix is structural — split the filter so the common intent is the default and the broad
match is opt-in, or echo which field matched on each hit.

**Generalizes to:** any "search everywhere" default, case-insensitive matching that collapses
distinct values, short prefixes that match all, and tag filters that OR when the caller expects
AND.

## 8. Composite parameters are namespaces, not parameters

An options object, config block, dict, or nested struct is not one parameter. Every shape above
applies to each member *and* to the container:

- What does an **empty** container mean — no filters, or filter-everything-out?
- What does an **absent** container mean, and does it differ from empty?
- What happens to an **unknown key** inside it — rejected, ignored, or passed through?
- Do members **interact**, and what happens when conflicting ones are both set?
- Does a member **override** or **merge** with a value set elsewhere (config file, env var, a
  sibling flag)?

This is where "it is a high-level object, so the checks do not apply" does the most damage. A
composite has a larger value space than a scalar, so it carries more ambiguity, not less.

---

## Disambiguation Checklist

The checklist lives in `SKILL.md`, so it is loaded whenever the skill triggers rather than
only when this reference is opened. The shapes above are the reasoning behind its questions;
the right-hand column there names the pass in Part 2 that verifies each answer.

---

# Part 2 — Review Passes

## Group B — Does the behavior match the text?

## B1 — Silent ignore

Trace every parameter from parse to use. Three failure modes, escalating:

1. **Clamped** without saying so — you pass `depth=50`, get `depth=3`, and are not told.
2. **Echoed but unused** — validated, clamped, returned in the response, never reaching the
   behavior. Looks honored at every observable layer.
3. **Ignored with a substitute behavior** — an enum value valid on a sibling tool causes a
   different operation instead of an error.

```bash
# Does the name appear anywhere past the parse/validate layer?
rg -n 'depth' src/ | rg -v 'parse|validate|schema|clamp'
```

An empty result from that second command is the finding.

**Seen in the wild [independent]:** `detect_changes.depth` was clamped, echoed, and never
used to traverse, so a bounded impact analysis returned a flat report. An unprojected
`ORDER BY` key was accepted and ignored, giving "apparently successful but incorrectly ordered
results". `search_code` accepted `search_in:"graph"` and ran a *source* search — legitimate on
a sibling tool, so it validated cleanly.

**Fixes:** implement it or delete it; reject rather than clamp, or report the effective value
and say it was clamped; reject invalid *combinations* of individually valid values and name
the tool that does support them.

**Test shape:** assert the *effect*, not acceptance. A test asserting "no validation error"
passes on all three failure modes.

## B2 — Declaration versus implementation drift

Compare what is *declared* against what the code accepts. The declaration is wherever a caller
looks first — a JSON Schema, an OpenAPI document, a C header, a `.pyi` stub, a docstring, a
`--help` string, a trait or interface definition. Both directions fail:

- **Implemented, undeclared** — works, but undiscoverable; closing the input policy breaks it.
- **Declared, unimplemented** — advertised and silently dropped.

This pass costs nothing where the declaration *is* the definition — a Rust or TypeScript
signature sits on its own body. It applies wherever they are separate and something other than
the compiler must keep them aligned: a C or C++ header, a `.d.ts` or `.pyi` stub, hand-written
docs, or any schema not generated from the code it describes. A C prototype absolutely can
drift from its definition across translation units, which is why `-Wmissing-prototypes` exists.

**Seen in the wild [independent]:** closing the input policy produced seven test failures, and
every one was pre-existing drift rather than damage from the change — `verbose`, the
`project_name` compatibility alias, `repo_path`, and `auto_dep_limit` were all accepted by the
code and absent from their canonical schemas.

**Fix:** make the schemas describe the already-supported behavior, then add drift tests so
discovery, dispatch, and CLI cannot diverge again. Weakening validation or deleting working
compatibility is the wrong direction.

**Lesson:** failures that appear when you *tighten* validation usually expose old drift. Read
them before rolling back.

## B3 — Composition

Feature lists imply composition. For every pair of advertised features, confirm the pair works,
and document every restriction — a caller cannot infer it, and cannot tell an unsupported
combination from a bug.

**Seen in the wild [independent]:** a description advertised both `WITH` and `OPTIONAL MATCH`;
`WITH … OPTIONAL MATCH` was rejected as an unexpected trailing clause. Multiple `ORDER BY` keys
were rejected too, and that restriction was documented nowhere.

## B4 — Examples execute

Every example in a description, help text, or docstring is a test. Extract and run them in CI.

**Seen in the wild [independent]:** a docstring recommended `ORDER BY CASE … LIKE`; the engine's
grammar accepts neither `CASE` nor `LIKE` in that position. Confident, prominent, never run.

**Lesson:** examples are the highest-trust element in documentation, so a broken one costs more
than a missing one.

## B5 — Result-set honesty

Does the response tell the caller what it actually did?

- Does an empty result echo the resolved inputs it filtered on?
- Does every cap emit `truncated` / `has_more` and a continuation cursor?
- Is anything advertised as "complete" or "full" that is actually capped?
- **Is the ordering stated where the caller reads `limit`, not only in the response?** Part 1
  §5 sets the requirement; this is where it is checked. A capped result under undisclosed
  ordering is an arbitrary sample presented as the top matches — and a full result set from a
  filter that silently matched everything (Part 1 §7) is the same defect inverted.

**Seen in the wild [independent]:** after strict validation closed the typo path, a
*valid-yet-wrong* project name still returned empty — and the response omitted the resolved
project entirely, so "searched the wrong project" and "searched the right one, found nothing"
were indistinguishable. Fixed by echoing the resolved project; no wording change substitutes.

Separately, property discovery was capped at 50 keys per label with no truncation reported while
documented as "full" discovery. That project's own premortem named the consequence: response
caps silently clip the relevant result and agents draw false "not found" conclusions.

**Lesson:** validation removes typos, not wrong-but-plausible values. A bounded result described
as complete is a false claim, not a rounding detail.

## B6 — Cross-surface parity

Trigger the same mistake through the CLI, the schema, and the library binding. They need not be
byte-identical, but must name the same parameter, the same bound, and the same corrective action.
Divergence teaches callers the surfaces behave differently.

## B7 — Unknown-name handling

Check **every** dispatch path — a boundary guard and a fallback arm are two messages that drift
apart. JSON-RPC separates `-32601 "Method not found"` from `-32602 "Invalid params"`; the two
paths are distinct by specification, so check both.

- **Unknown tool/command:** list served names, derived from the registry the dispatcher reads.
- **Unknown parameter:** offer the nearest accepted name by edit distance when one is close
  (threshold `len/3`, clamped to 1..3), else list accepted names sorted.
- **Out-of-range value:** append the parameter's own description so the caller learns what the
  accepted values select.
- **Aliases:** every accepted alias appears in the schema, or it does not exist.

**Seen in the wild:** a fix landed at the request-boundary guard only; the dispatcher's fallback
arm produced a second, untouched message — the one most callers actually hit. An existing test
caught it. Derive the shared text from one function so the two cannot drift.

**Before optimizing this,** see Part 3 — the cost is linear in supplied keys and was measured
well below the complexity of a cache.

## B8 — Protocol contract completeness

Everything above asks whether the *inputs* are well described. A caller also has to predict
what comes back and whether calling is safe. Four declarations answer that, on any surface:

| Declaration | What its absence costs | Where it lives |
|---|---|---|
| **Strict input** — unknown fields rejected | a misspelled argument succeeds silently | `additionalProperties: false`; `deny_unknown_fields`; `extra="forbid"` |
| **Output shape** — the response contract | callers parse defensively or guess | `outputSchema`; OpenAPI `responses`; a return type |
| **Effect class** — read-only, destructive, idempotent | every call is treated as dangerous | MCP `annotations`; HTTP method semantics |
| **Display name** distinct from the identifier | cosmetic only | `title` |

**None of this is new, and the prior art is the argument.** HTTP settled the same two problems
decades earlier: `GET` is *safe*, `PUT` and `DELETE` are *idempotent*, `POST` is neither — the
effect class is carried by the method itself. And `4xx` versus `5xx` is exactly the split below.
A surface that omits these is not choosing simplicity; it is discarding a distinction its
protocol already offers.

**Effect class is the safety-relevant one, and defaults decide who pays.** Where a protocol
supplies defaults, they usually assume the dangerous case — MCP defaults `destructiveHint` to
**true**, so omitting the block *asserts* the tool is destructive rather than leaving it
unknown. The cost then lands on the read-only majority, which a conforming client must treat
like a delete. Check the defaults for your surface before assuming absence means "unspecified".

**Use the right error channel.** Every protocol separates *you called it wrong* from *the call
was valid and failed* — JSON-RPC `-32602` versus a result flagged as an error; HTTP `4xx`
versus `5xx`; an exception type versus a returned error value. Conflating them denies the
caller the one fact that decides whether retrying can help.

Trigger four cases and check which channel each takes: unknown tool or method, unknown
parameter, out-of-range value, and a valid call whose operation fails.

For MCP's exact spellings, hint defaults, and a live-server audit recipe:
`references/mcp-specifics.md`.

---

## Group A — Is the text right?

## A1 — Availability

Does every parameter named inside a description or error exist on *that* entry point, settable
by *that* caller? Highest-yield text pass: helpful cross-references get written from memory of
the whole API, but each entry point exposes a subset.

```bash
rg -o '`[a-z_]{3,}`' path/to/schema.rs
```

For each hit, find the enclosing tool, list its real parameters, confirm membership. A shared
validation helper may only name parameters common to every caller it serves.

**Seen in the wild:** a shared paging validator named three parameters that exist on none of the
four query types it served — they belonged to *methods* and to a different tool.

```
- limit must be 0 or greater, got -5; pass a positive count, 0 for every match,
  or use lines_per_message, transcript_lines, or summary_items, which take negatives
+ limit must be 0 or greater, got -5; pass a positive count, or 0 for every match
```

Shorter *and* correct. The redirect was kept only in the one schema where both parameters
genuinely coexist.

**Beware false positives.** In one audit this pass flagged 11 candidates and 10 were English
words colliding with parameter names: "explicit **offset**" (a timezone), "**limit** each
returned message" (verb), "with **context**" (noun), "auto-generated **summary** messages"
(adjective). Verify each; never bulk-edit grep output.

**Do not invert that into skipping verification.** A 10-out-of-11 false-positive rate is a
fact about short parameter names being common English words, not a licence to dismiss the
batch — the single real finding in that audit was the most consequential defect in the
release. Cheap to check, expensive to miss: check all of them.

## A2 — Semantic duplication

Do two parameters mean the same thing under different names, or does one name mean different
things on different tools?

List every parameter across every tool and group by meaning rather than by spelling. Duplicates
hide behind synonyms: `limit`/`max_results`/`count`, `path`/`directory`/`root`,
`verbose`/`debug`, `filter`/`query`/`match`. Each pair forces callers to learn which is which,
and the two drift as one gains features the other lacks.

```bash
# Group declared parameters by name to find one concept spelled several ways,
# and one spelling used for several concepts.
rg -o '"[a-z_]{3,}"\s*:\s*\{' schema.json | sort | uniq -c | sort -rn
```

**Fixes, in order of preference:** extend the existing parameter; alias the new name to it and
declare the alias; or, if both must exist, state in each description how it differs from its
near-twin. Shipping two undifferentiated names is the option to avoid.

The mirror defect is one name meaning different things on different tools — worse, because a
caller who learned it once is now confidently wrong. A shared name with a different enum per
tool is the mechanically detectable form. **It is also a candidate, not a finding**, and the
worked example below is why.

Measured on one server, `mode` carried seven distinct value spaces:

| Tool | `mode` accepts | What `mode` selects |
|---|---|---|
| `search_graph` | `full`, `summary` | detail level |
| `search_code` | `compact`, `full`, `files` | detail level |
| `get_code` | `full`, `signature`, `head_tail` | detail level |
| `get_code_snippet` | `full`, `signature`, `head_tail` | detail level |
| `index_repository` | `full`, `moderate`, `fast`, `cross-repo-intelligence` | detail level |
| `trace_path` | `calls`, `data_flow`, `cross_service` | **which edges to follow** |
| `manage_adr` | `get`, `update`, `sections` | **which action to perform** |

**The first reading was wrong.** Reading the descriptions shows `full` means *the maximal,
least-reduced variant* in all five: individual not aggregated, snippets included not
deduplicated, whole source not signature, all files not filtered. "full = give me everything"
transfers correctly every time. Flagging it would have been a false positive.

**The real defect is a level up:** `mode` selects three *kinds* of thing — detail level (five
tools), edge selection, action verb. Only the last two break the convention, and the action
verb is the hazard: `manage_adr` with `mode="update"` **writes**, so a mutation hides behind a
name promising a view setting, on a tool with no `destructiveHint` (B8).

**The test to apply, before calling any shared name overloaded:** do the shared *values* carry
a consistent meaning? If yes, the convention holds and the differing enums are fine. If no,
rename the outliers — `traversal_edges`, `action` — and leave the convention alone.

The same applies to descriptions: one name described differently across tools is a candidate.
Sometimes it is legitimate per-tool tailoring, sometimes two concepts wearing one name. Read
them before deciding.

### Check the whole function too, not only its parameters

Two tools that do the same job are the same defect one level up, and more expensive: callers
must choose before they can call, and the two diverge in capability over time. Ask what task
each tool completes, in one sentence, and look for sentences that match.

**The tell is a parameter whose value selects behavior another tool already provides** — the
seam where two overlapping tools were welded together. Observed: `search_code` accepted
`search_in: "graph"`, the job `search_graph` exists to do, and resolved the overlap by silently
running a source search. The duplication and the silent substitution (B1) were one defect.

Signals worth a closer look, each of which may be legitimate — decide, then record why:

- A tool that is another tool plus a mode flag.
- Several tools enumerating the same collection through different filters.
- A `type`, `mode`, `kind`, or `search_in` parameter whose branches have little code in common.
- Two tools whose descriptions differ only in adjectives.

**Fixes:** merge and keep the mode parameter; or keep both and have each description name the
other and say when to prefer it. What must not survive is an overlap resolved by silently
picking one, and the choice going unstated where the caller reads.

## A3 — Grammatical attachment

In "a, b, and c, which take negatives" — does the clause attach to `c` or to all three? Any list
followed by a relative clause, participle, or trailing qualifier is a candidate. Rewrite or
split. English will not disambiguate this and readers split roughly evenly.

```bash
rg ', which |, that |, taking |, accepting ' path/to/
```

## A4 — Negation

Ban `no negative`, `not negative`, `non-negative`, `never`, `cannot be`, `must not`,
`don't pass`. Each states what is forbidden and leaves the accepted set implicit; double
negatives invert under paraphrase.

```bash
rg -i "no negative|not negative|non-negative|never |cannot |must not|don't pass" path/to/
```

Restate as accepted values: `"0 or greater"`, `"one of: user, assistant, system"`. A genuine
mutual exclusion may say so, but must still name the way forward — `"--seq selects one message
by sequence, so it takes only --context; drop --role and --limit, or omit --seq to filter a
range"`.

## A5 — Value-space completeness

Run the Disambiguation Checklist in `SKILL.md` against every parameter; Part 1 holds the
reasoning behind its questions. The minimum for a signed integer is four facts: positive,
negative, zero, omitted. Report as a fraction.

**Seen in the wild:** guidance claimed `-5` "probably meant lots of results". Three sibling
parameters already established positive = first N, negative = last N, `0` = all — so `-5` means
the *last five* and `0` means "lots". Check siblings before inventing semantics.

## A6 — Default drift

Does every declared default appear in the human-readable text? A default living only in a schema
`default` key or a function signature is invisible to a caller reading prose.

```bash
rg '"default"|= None|unwrap_or' path/to/
```

State it in words: "defaults to none", "defaults to false".

## A7 — Vague qualitative words

```bash
rg -i "reasonable|appropriate|large|fast|efficient|as needed|properly"
```

Each is guidance shaped like a fact. **Seen in the wild [independent]:** a user rejected
"reasonable computational cost" in a schema and required "effective and computationally
efficient". Replace with a threshold, a measurement, or delete.

## A8 — Brevity, measured

Compute median and maximum description length; justify each outlier against the required facts.
A median near 150 characters is a healthy shape.

**The median is a diagnostic, not a target** — it says "look at the outliers", not "make
everything 150 characters". Padding a short description that already carries its facts, or
trimming a long one that needs them, turns a measurement into damage. The standard is
two-sided: every fact present is required, and every required fact is present.

Cut: restatements of the type, filler, repetition of the parameter's own name. Never cut: an
accepted value, a special-value meaning, a default, a unit, or an interaction.

**Order matters more than length.** Readers and context-compactors truncate tails, so the
distinguishing fact goes first.

**Restatement is not automatically duplication.** It earns its place whenever the reader is in
a different context and cannot see the original:

- An **error message** restates the description's facts, because the caller is not reading the
  schema at the moment it fires.
- An **always-loaded checklist** restates the reference, because the reference may never be
  opened.
- A **presentation parameter** restates what it does *not* affect (Part 1 §4), because the
  reader's default assumption is wrong.

The test is not "do these words appear twice?" but "will the reader have the other copy in
front of them?" Cut only when the answer is yes — deleting a restatement because it exists
elsewhere is how an always-loaded document loses facts a reader needed. Made and reverted here.

**Seen in the wild:** a proposal to shorten several bounds to "natural numbers" was rejected —
whether `0 ∈ ℕ` is disputed, and `0` was load-bearing in every affected parameter. Brevity that
trades away precision on the special case is a regression.

An outlier legitimately survives when it carries irreducible facts: one 504-character
description (3.4× the median) stated three sign cases, what the parameter does *not* affect,
its ordering relative to another parameter, and a contrast against a sibling tool.

## A9 — Unstated assumptions

Does the text presuppose anything the reader does not have *at the point they read it*?

Read each description as a newcomer who has seen nothing else — no other parameter, no source,
no conversation, no prior call. Everything the text leans on must either be present or be
reachable from what it names.

Five forms, in rough order of frequency:

| Form | Example | Why it breaks |
|---|---|---|
| **Forward reference** | "duplicates at both levels" before the levels are named | The reader has no model to attach it to |
| **Undefined term** | "the catalogue", "the seam", "streamlined mode" | Project vocabulary that reads as ordinary English |
| **Dangling deixis** | "this", "the other one", "as above", "the same way" | No antecedent survives out of context |
| **Assumed reading order** | "unlike the previous parameter", "see the `query` description" | Declarations reach the caller individually, in no fixed order |
| **Assumed access** | "check the dashboard", "see the config" | The reader may hold neither |

**Assumed reading order is broken by construction wherever declarations are surfaced one at a
time** — a tool description in a schema, a hover tooltip over one parameter, a `--help` entry, a
man-page flag, a docstring rendered for a single function. There is no "previous" parameter and
no "above". Any comparison must name its target in full — `get_session transcript_lines`, not
"the windowing parameter mentioned earlier".

```bash
# Deixis and ordering assumptions inside description strings.
rg -o '"description"[^"]*"[^"]*\b(as above|as described|see below|the previous|the other|likewise|similarly|this one)\b' schema.json
```

Grep finds the ordering forms. Forward references and undefined terms need a cold reader,
because they look fine to anyone who already knows the answer: **you cannot detect an unstated
assumption using the knowledge that makes it invisible.** Hand the text to someone without
project context and ask what they cannot answer.

Found while writing this document, by running this pass on it: "duplicates at both levels"
(levels never named), "Group B outranks Group A" in an introduction that never says what the
groups are, and a bare "see A8" pointing into a file the reader had not been told to open.

---

## Process Failures to Expect

Not about the parameters — about reviewing them. All observed.

| Failure | Correction |
|---|---|
| **Test hit a different guard** than intended, passing while proving nothing. **[independent]** — hit twice in one week on unrelated codebases. | Confirm the red test fails *for the expected reason*. |
| **Assumed red meant the code was wrong.** A boundary test failed because the assertion miscounted, not because the implementation misbehaved. "Fixing" the code would have widened a threshold that was already correct. | On red, decide *which* is wrong before changing either. Recompute the expected value by hand. |
| **Suppressed stderr, then read the empty output as a result.** `cmd 2>/dev/null` turned `error: unexpected argument` into zero rows, and the corpus was briefly declared empty. Nearly filed as a product bug. | Never discard stderr while investigating. An empty result and a swallowed error are indistinguishable — this document's own pass B5 applied to your shell. |
| **Reviewed a stale artifact.** The installed binary predated the branch by four days, so findings described a schema already fixed. **[independent]** — the same week, another agent noted "the optimized executable predates the current validation change". | Check the artifact's build date against your last commit before acting on any finding. |
| **Tested at a layer that skips validation** — direct dispatch bypasses the schema entirely. | Test at the layer that actually validates. |
| **Fixed one of two code paths.** | Grep every site producing that error class; derive shared text from one function. |
| **Hardcoded a list that must stay in sync** — "unknown tool" listed served tools as a literal. | Build from the registry the dispatcher reads. |
| **Claimed two files diverged without diffing** — the entire delta was an uncommitted local edit. | Diff against committed state before reporting drift. |
| **Blamed the tool before re-reading the invocation** — a flag placed after `--` instead of before it. | Reproduce against the documented form first. |
| **Deferred a fix as "disproportionate at release time"** with nothing yet published. | Before first publish there is no compatibility surface to protect. |
| **Treated grep hits as findings.** | Verify each candidate individually. |
| **Promoted a mechanical candidate without testing its premise.** Seven differing `mode` enums were called a defect; reading the descriptions showed the shared value `full` meant the same thing in all five tools that used it. The real defect was one level up and much narrower. | A structural signal answers "are these different?" — not "does the difference harm the caller?" Answer the second before reporting. |

---

## Reporting

Report open findings rather than dropping them. A pass with an unresolved finding is a result;
silently omitting it is not.

**A filled template is not evidence that the passes ran.** Every number must come from a
command you executed or a file you read. An unmeasured count is a fabrication, and worse than
no report because it ends the review. `not run` is a legitimate entry, and the only honest one
for a skipped pass.

**The grep commands are starting points, not the check.** Each assumes a convention: backticked
parameters, descriptions in one file, a matching language. Zero hits from a command written for
another codebase means the command did not fit — adapt the pattern and re-run.

```
B1 Silent ignore         62 params traced, 62 reach behavior
B2 Schema drift           0 implemented-undeclared, 0 declared-unimplemented
B3 Composition            3 pairs checked, 3 restrictions documented
B4 Examples               9 extracted, 9 executed in CI
B5 Result-set honesty     echo: NO (open)   truncation: yes   ordering stated: NO (open)
B6 Cross-surface          parity confirmed via CLI, MCP schema, Python binding
B7 Unknown names          2 dispatch paths checked
B8 Protocol contract      additionalProperties 7/7, outputSchema 4/7, annotations 0/7

A1 Availability          11 candidates → 1 real
A2 Semantic duplication   1 concept spelled 5 ways, 1 name reused for 3 concepts
A3 Attachment             1 candidate  → 1 real
A4 Negation               3 → all fixed
A5 Value-space            4/4 signed params complete
A6 Default drift          2 → both fixed
A7 Vague words            0
A8 Brevity                median 148 chars, max 504 (lines_per_message), justified
A9 Unstated assumptions   3 forward refs, 0 undefined terms, 0 ordering assumptions
```

## Regression-lock shape

Assert the *absence* of banned phrasing, not only the presence of the good phrasing.

```python
@pytest.mark.parametrize("query_type", [SessionQuery, MessageQuery, AnalysisQuery, FileQuery])
def test_negative_limit_names_what_to_pass_instead(query_type):
    with pytest.raises(ValueError) as excinfo:
        query_type(limit=-5)
    message = str(excinfo.value)

    assert "limit" in message                    # names the parameter
    assert "0 or greater" in message             # states the bound as accepted values
    assert "-5" in message                       # quotes the offending value
    assert "0 for every match" in message        # says what 0 selects

    assert "no negative" not in message          # regression lock: double negative
    assert "not negative" not in message
    assert "lines_per_message" not in message    # regression lock: unavailable parameter
    assert "transcript_lines" not in message
```

The four `assert ... not in` lines are the ones that keep the fix fixed.

---

# Part 3 — Solving It For The Caller

## The Remediation Ladder

Ordered from most to least helpful. Climb as high as is *safe*, not as high as is possible.

| Rung | Response | When it is right |
|---|---|---|
| 1 | **Accept and proceed silently** — treat the input as the intended one | Only when the mapping is unambiguous, documented, and harmless if wrong. A declared compatibility alias qualifies. A guess never does. |
| 2 | **Accept, and say what was assumed** | Same as rung 1 but the mapping is newly introduced, deprecated, or lossy. Echo the effective value. |
| 3 | **Reject, naming the single likeliest fix** | The default. One candidate is clearly closest. |
| 4 | **Reject, listing accepted values** | Nothing is close enough to single out, or several tie. |
| 5 | **Reject bare** | Never. |

**Rung 1 is the trap.** Silently correcting an input the caller did not write is pass B1's
silent-substitution defect wearing a friendly face. The test: could the caller reasonably have
meant something else? If yes, drop to rung 3.

**Rungs 3 and 4 are one message, not two.** Lead with the suggestion, then still list the
catalogue — a caller whose guess was wrong recovers from the same response instead of making a
second discovery call.

```
unknown tool: search_message — did you mean "search_messages"? this server
provides "search_sessions", "get_session", "list_sessions", …
```

**Never invent a suggestion to fill the slot.** A confidently wrong pointer is worse than none:
it converts one retry into a detour. Test the far-miss case explicitly, asserting no suggestion
appears.

---

## Choosing an Algorithm

| Algorithm | Good for | Watch out |
|---|---|---|
| **Levenshtein** (insert/delete/substitute) | The safe default for identifiers | Counts a transposition as 2 edits |
| **Damerau-Levenshtein** (adds transposition) | Typos specifically — `limti`→`limit` costs 1, not 2 | Slightly more code; worth it for hand-typed input |
| **Jaro-Winkler** (weights a common prefix) | Human names, free-text fields | Ranks correctly but compresses the gap on namespaced APIs — see below |
| **Trigram / n-gram** (`pg_trgm`, `difflib` ratio) | Longer strings, fuzzy search over descriptions | Weak on short identifiers |
| **Soundex / Metaphone** (phonetic) | Spoken input, name lookup | Meaningless for identifiers — avoid |

**The Jaro-Winkler trap.** It boosts scores for shared prefixes — exactly what a namespaced API
has everywhere. It still *ranks* correctly; the problem is that it compresses the gap, so any
threshold loose enough to catch typos admits the wrong candidate too. Measured for the typo
`search_message`:

| Candidate | Levenshtein | Jaro-Winkler |
|---|---|---|
| `search_messages` (correct) | **1** | 0.987 |
| `search_sessions` (wrong) | **5** | 0.856 |

Levenshtein separates 1 from 5, so a threshold of 3 admits one and rejects the other. Both
Jaro-Winkler scores sit above any usual cutoff. Python's `difflib.get_close_matches`, which is
ratio-based, returns *both* for this input. For prefix-clustered names — `get_*`, `list_*`,
`search_*` — prefer plain Levenshtein precisely because it does not reward the shared part.

### Threshold, tie-breaks, and cost

- **Scale the threshold with length, then clamp.** `len/3` clamped to `1..3` works well: short
  names do not collide with unrelated short names, long names tolerate the extra slip a long
  word invites, and the clamp stops a 30-character name from matching almost anything.
- **Break ties deterministically** — by distance, then shortest candidate, then lexically. One
  typo must never produce different suggestions across runs.
- **Do not optimize before measuring.** Validation is `Ω(k)` in supplied keys and cannot be
  sublinear. A full typo-rejection round trip — JSON parse, schema parse, validation, error
  serialization, pipe I/O — measured 49.8–61.7 µs median over 900 calls. A linear scan over a
  few hundred candidates is not the bottleneck. Reach for a **BK-tree** only with a large
  dictionary (thousands of terms) where you have measured the scan mattering.

---

## Libraries by Language

Verify the exact API against current docs before use — versions and function names move.

| Language | Distance library | Framework support |
|---|---|---|
| **Rust** | `strsim` (Levenshtein, Damerau, Jaro-Winkler, normalized variants) | `clap` suggests near-miss flags automatically |
| **Python** | `difflib.get_close_matches` (stdlib, ratio-based); `rapidfuzz` (fast, C++ backed) | CPython itself suggests for `NameError`/`AttributeError`; `click` via `click-didyoumean` |
| **Go** | `agext/levenshtein`; `lithammer/fuzzysearch` | `cobra` has built-in command suggestions with a configurable minimum distance |
| **JS/TS** | `fastest-levenshtein`, `leven`, `didyoumean2` | `commander` exposes suggestion-after-error |
| **Ruby** | `did_you_mean` — in the standard library | Wired into `NoMethodError`/`NameError` by default |
| **Java** | Apache Commons Text `LevenshteinDistance` | `picocli` suggests near-miss options |
| **C / C++** | Hand-rolled is ~25 lines; no dependency needed | — |
| **SQL** | Postgres `pg_trgm` (`similarity`, `%` operator), `fuzzystrmatch` | — |

**Prefer the framework's built-in.** If the CLI parser already suggests, its behavior is tested,
localized, and consistent with the ecosystem. Add a hand-rolled pass only for the surfaces the
framework does not cover — typically MCP tool names, JSON keys, and config file keys.

**Cover every surface once you have the helper.** Extract one `nearest_name(name, candidates)`
and call it from each surface. Two copies of a threshold drift; one cannot.

---

## Step Zero: You Cannot Suggest What You Never See

Before any of the above can run, the system has to *notice* the unknown input. Most
serialization defaults do not — they discard it silently, so the suggestion code is never
reached and the caller gets a successful-looking call that ignored their argument.

| Layer | Default for an unknown key | Make it reject |
|---|---|---|
| JSON Schema / MCP | **Allows** (`additionalProperties` defaults to true) | `"additionalProperties": false` |
| Rust `serde` | **Ignores** | `#[serde(deny_unknown_fields)]` |
| Python `pydantic` v2 | **Ignores** (`extra="ignore"`) | `extra="forbid"` |
| Go `encoding/json` | **Ignores** | `Decoder.DisallowUnknownFields()` |
| Java `Jackson` | Rejects | already strict — leave it |
| CLI parsers (`clap`, `argparse`, `commander`) | Rejects | already strict |
| Env vars | **Ignores**, universally | nothing built in — see below |
| YAML/TOML config | **Ignores** in most loaders | loader-specific strict mode |

The pattern: **argument parsers are strict, serializers are permissive.** So a CLI catches a
typo while the same product's JSON, config, and env surfaces silently drop it — one concept,
two behaviors, four surfaces, one codebase.

Closing this is prerequisite work, and it pays a second dividend: tightening the policy
surfaces every parameter that was implemented but undeclared. Expect failures, and read them
before rolling back — one project's strictness change produced seven, all pre-existing drift.

## What The Language Lets You Do At All

Where the mistake is caught determines whether you can help, and the type system matters less
than the boundary.

| Where the name is bound | Who reports the mistake | Your leverage |
|---|---|---|
| Compile time (Rust struct field, Go field, typed kwargs) | The compiler or IDE | **None at runtime** — the name itself is your entire guidance |
| Runtime, in-process (Python `**kwargs`, JS options object, Ruby hash) | You, if you check | Full ladder available |
| Across a boundary (CLI, JSON, MCP, HTTP, config, env) | You, always | Full ladder — **and mandatory** |

The boundary row surprises people: **a statically typed program is dynamically typed at its
edges.** A Rust MCP server has compile-checked structs internally and untyped JSON arriving from
the client, so the type system helps with neither detection nor message — which is why such a
server still needs hand-rolled suggestions.

### Where suggestions are structurally impossible

- **Positional parameters** — there is no name to misspell, so nothing to suggest. A caller who
  swaps two same-typed arguments gets silence from every layer. The only defenses are keeping
  arity low, ordering by likely-to-differ types, and naming the *function* so the order is
  implied. Applies to C, Go, and anything called positionally.
- **Languages without keyword arguments** (C, Go, Java, most JS call sites) — options arrive as
  a struct or object. Static languages catch a misspelled field at compile time, so no runtime
  message is possible or needed; JS silently yields `undefined`, so it needs a runtime check.
- **Environment variables** — no ecosystem validates these by default. `MYAPP_TIMEOOUT` is
  ignored everywhere, always. If you read env, enumerate the recognized names at startup, warn
  on unrecognized ones sharing your prefix, and offer a `--print-config` that shows every
  effective value with its source. That is the only recovery path this surface has.
- **Shell and template expansion** — no introspection is available. Validate after expansion.

When suggestion is impossible, the effort moves entirely to naming, arity, and startup
validation. Those are the rungs you have left.

## Where Suggestions Are Not The Answer

Edit distance bridges typos, not vocabulary. It will never map a CLI's `--regex` to an MCP
`match_mode` — different words for one concept, at a distance no threshold should accept.

When two surfaces deliberately spell a concept differently, the fix is a small explicit alias
table consulted *before* the distance fallback. Build it only from observed misuse — a
speculative alias table is a second vocabulary to maintain.

The same limit applies to structural mistakes: a caller who nests a key one level too deep, or
passes an array where an object belongs, needs a type-shaped message (`expected an object with
keys a, b; got an array`), not a nearest-name hint.

# MCP Specifics

Everything protocol-particular, kept out of the main reference so the passes stay
surface-agnostic. Pass B8 states the general contract; this file is how MCP spells it.

Verified against
[`schema/2025-06-18/schema.ts`](https://raw.githubusercontent.com/modelcontextprotocol/modelcontextprotocol/main/schema/2025-06-18/schema.ts)
and the [tools specification](https://modelcontextprotocol.io/specification/2025-06-18/server/tools),
2026-07-20.

---

## The four declarations a tool should carry

| Declaration | Default when absent | What that costs |
|---|---|---|
| `inputSchema.additionalProperties` | `true` — unknown keys accepted and dropped | a misspelled argument succeeds silently |
| `outputSchema` | absent | the response shape is unguessable before calling |
| `annotations` | absent → see the hint defaults below | every tool is treated as destructive |
| `title` | falls back to `name` | cosmetic |

`outputSchema` is optional, but once declared the server **MUST** return conforming
`structuredContent`, and clients **SHOULD** validate it. Declaring it is a promise, not a hint.

## ToolAnnotations, and why the defaults matter

```typescript
export interface ToolAnnotations {
  title?: string;
  readOnlyHint?: boolean;     // Default: false
  destructiveHint?: boolean;  // Default: true
  idempotentHint?: boolean;   // Default: false
  openWorldHint?: boolean;    // Default: true
}
```

Omitting the block does not leave a caller guessing. It **asserts** the tool is destructive,
non-idempotent, and open-world. The cost therefore lands on the read-only majority: a
conforming client must treat `list_projects` exactly like `delete_project`, so it confirms
every call with the user or trusts none.

Two conditionals the schema states and a caller cannot infer:

1. `destructiveHint` and `idempotentHint` are meaningful **only when `readOnlyHint == false`**.
2. Display precedence is `title`, then `annotations.title`, then `name`.

Clients **MUST** treat annotations as untrusted unless the server is trusted — they are hints
for planning, not a security boundary.

Do not confuse `ToolAnnotations` with `Annotations` (`audience`, `priority`, `lastModified`),
which annotates **content**, not tools.

## Two error channels, and which to use

| Channel | Means | Example |
|---|---|---|
| JSON-RPC error `-32601` | method not found | unknown tool name |
| JSON-RPC error `-32602` | invalid params | unknown parameter, out-of-range value |
| result with `isError: true` | the call was valid, the operation failed | API rate limit, missing file |

Reporting a bad argument as `isError` denies the caller the distinction that decides whether
retrying can help. Trigger all four cases and check which channel each takes: unknown tool,
unknown parameter, out-of-range value, and a valid call whose operation fails.

## Auditing a live server

Any MCP server, whatever its implementation language, because this reads protocol output:

```bash
printf '%s\n' \
  '{"jsonrpc":"2.0","id":1,"method":"initialize","params":{"protocolVersion":"2024-11-05","capabilities":{},"clientInfo":{"name":"audit","version":"1"}}}' \
  '{"jsonrpc":"2.0","method":"notifications/initialized"}' \
  '{"jsonrpc":"2.0","id":2,"method":"tools/list"}' \
  | your-mcp-server > /tmp/tools.jsonl

python3 scripts/check-schema-descriptions.py /tmp/tools.jsonl
```

If the server hides tools behind a reveal call, invoke that first — a `tools/list` before it
audits only the visible subset.

## Measured across three servers

| | ai-session-search | codebase-memory-mcp | GitKraken |
|---|---|---|---|
| `additionalProperties: false` | **7/7** | 0/17 | 0/31 |
| `outputSchema` | 4/7 | **17/17** | 0/31 |
| `annotations` | 0/7 | 0/17 | **31/31** |
| `title` | 0/7 | **17/17** | 0/31 |

Each server is strong on exactly one declaration and weak on the rest, and no two agree on
which. Three independent teams each solved a different quarter of the same contract, which is
the evidence that B8 checks something real rather than something invented.

# Sources

Checked: 2026-07-20

## Requirements as stated

The maintainer's instructions, verbatim and in order, with what each produced. Kept because
several rules here exist only because a specific correction was made, and paraphrasing them
would lose the reason.

1. *"make a skill for parameter naming and the full iterative process and gotchas and
   assumptions and what to avoid and look for and strategies like the checklist make it global
   across this machine (and use the process on the naming of the sections file etc)"*
   - Created the skill; the naming-decision table below applies its own rules to its filenames.
2. *"you're not accounting for the idiosyncracies of properly complex params and failure modes
   … e.g. -5 is also a valid index to the end, where 5 might be the first 5 entries -5 might be
   the last 5 entries, and 0 unlimited"*
   - Part 1 §1, the signed-integer collision. The single most-cited shape in the skill.
3. *"searching 'ambiguous' and 'docstring' and 'param' alongside other relevant words might be
   useful"*
   - Cross-project corroboration via ai-session-search; the `[independent]` markers.
4. *"there is also a skill writing skill you should use (also the eval script … contains some
   bugs like hardcoded strings it insists be present which is a false assumption)"*
   - Applied `ai-skill-builder`; fixed its path handling and warning strings (`a6ec825`).
5. *"you are proliferating a lot of files you need to properly consolidate this skill"*
   - Six files to four; `mistakes-and-fixes.md` folded into the passes it justified.
6. *"shouldnt the checklist and the guides be consolidated in the main skill file too also does
   somewhere say to explore the codebase to ensure conventions are followed and avoid semantic
   duplication"*
   - Checklist promoted to SKILL.md; process step 1 and pass A2 added.
7. *"be careful if you have a checklist it needs to be complete i'm concerned you are causing
   some regressions"*
   - Caught: the checklist had silently dropped 6 questions. Restored to 20.
8. *"be careful about cutting you are often wordy and remember that wordiness reduction without
   losing facts process"*
   - The fact-sheet-then-cut-then-verify loop; pass A8's "never cut" list.
9. *"again don't make it too easy to pass either capture the intent"*
   - Audit warnings must name a substitute before self-crediting.
10. *"ai can be literal anticipate and prevent common errors it will make, e.g. oh we have
    numbers or some really high level object -> pass (thus concept missed entirely) make
    explicit the concepts behind the concepts"*
    - "Generalizing Beyond the Examples" and the composite-parameter recursion rule.
11. *"maybe your parameter naming skill can detect problems too? might get too complex though"*
    - `check-schema-descriptions.py`, deliberately scoped to the mechanical checks only.
12. *"remember tdd"* / *"make sure the tdd is actually robust and covers edge cases too"*
    - Red-first for every code change; the edge-case suite around `nearest_name`.
13. *"hey no ableism! i just saw blind!!! you need to amend that!!!"*
    - Renamed the checker's tier 2 from "CALLER-BLIND" to "UNDECLARED"; commit amended.
14. *"i didn't consent to new commits soft reset that"*
    - Commits are made only when asked.
15. *"are function names mentioned too? should this actually all be called the function
    signature naming skill"* / *"if it can cover both make sure you use both words"*
    - Renamed to `function-signature-and-parameter-guidance` at v3.0.0.
16. *"read the whole thing block by block for all files"* / *"use your context and or actually
    reread the files not just programming tricks"*
    - A full read found nine defects greps had missed, including B8 filed under Group A.
17. *"shouldn't you just do direct real file edits like you usually do"*
    - Scripted string surgery was failing silently; switched to the Edit tool.
18. *"use numbered lists also … looks like you just cut a lot of facts we discussed keeping"*
    - Restored the checker's 11 check names and the CLI frameworks it cannot read.
19. *"would a maintainer accept it as is?"* / *"keep working until it is maintainer ready and
    you need to harshly assess and refine not assume maintainer ready"*
    - Found the blocking gap: skill-builder Step 4 testing had never been run. Added the blind
      triggering test and `tests/` (20 tests).
20. *"do you have other mcp servers active you could try the system and skill on?"*
    - GitKraken audit; closed the selection-bias gap.
21. *"are you sure you are covering real mcp doc based criteria is destructiveHint
    standardized?"*
    - Verified against `schema.ts`. Corrected the annotation-defaults claim: absent annotations
      *assert* destructive rather than leaving it unknown.
22. *"be careful not to get too mcp focused maybe an mcp specifics resource md file shoudl be
    separate"*
    - Split `mcp-specifics.md`; B8 restated generically, surfacing the HTTP prior art.
23. *"you need to make this applicable to c and rust and python and mcp and jsonrps etc etc so
    be careful withthe phrasing and framing and assumptions at the core"*
    - B2 renamed to "Declaration versus implementation drift"; A9 degeneralized from schemas.
24. *"remember how we discussed ai looks at examples like that's the whole universe rather than
    the general principle, words must be explicitly used to help pure examples are not enough"*
    - The generalizing rule now governs writing, not only reading: state the principle in words
      *and* give the example.
25. *"the languages i use the most are typescript javascript rust python c/c++ also be careful
    you are not adding fluff or unwarranted length"*
    - Added `indexOf` returning `-1`; cut `strcmp` and two framing sentences. Net shorter.

## Lessons from building this skill

Process findings, distinct from the guidance itself. Each cost real rework.

1. **The passes kept finding defects in the document that defines them.** The checker shipped
   B5 (reported "no findings" after parsing zero parameters) and B1 (silently skipped every
   nested parameter under a type union). SKILL.md and this file each carried A9 unstated
   assumptions. A9 was written, then immediately caught three instances in its own section.
   If a rule is real, it applies to its own statement — run each new pass on the skill first.
2. **Grep yields candidates; only reading yields findings.** Three separate false results:
   A1 flagged 11 cross-references of which 10 were English words; an ableism scan matched
   "B**lame**d" via `lame`; a self-check reported "Group A/B not self-defined" because a line
   wrap split the phrase. Every one looked authoritative.
3. **Renumbering breaks cross-references silently.** Inserting pass A2 shifted A4→A5 through
   A7→A8, breaking the checklist's mapping column *and* the script's printed codes, with
   nothing failing. The fix generalizes: the script now reports by check *name*, because names
   are stable and numbers are positional.
4. **Consolidate on pointer count, not file count.** Three files that constantly referenced
   each other were one file; the proof was a `§10` pointer that outlived the section it named.
5. **Reformatting is not reducing.** Converting the References prose to a table saved exactly
   zero words — markdown pipe syntax cost back everything the consolidation gained.
6. **Compression and fact-deletion look identical in a diff.** Cutting words that restate an
   adjacent word's work is compression; cutting the checker's 11 check names is deletion, and
   a reader cannot recover those from context. The usable test is *will the reader have the
   other copy in front of them?*, which is why pass A8 states it.
7. **Verify before asserting, including about specifications.** Three corrections, all caught
   by someone asking rather than by a check: `destructiveHint` defaults to **true**, inverting
   the claim that missing annotations leave the effect class unknown; Jaro-Winkler was called
   "actively wrong" when it ranks correctly and merely compresses the gap; a seven-way `mode`
   enum divergence was a false positive because the shared value `full` meant the same thing
   in all five tools using it.
8. **Deriving a checker from the codebases that inspired it hides its blind spots.** Running
   it against an unrelated third-party server confirmed the checks and, as importantly, showed
   it staying silent where they did not apply.
9. **Good content is not a usable skill.** The guidance was sound long before the skill was
   dependable. A blind triggering test — an agent given only the description and sixteen
   requests — found four overtriggering phrases, one self-contradiction, and five uncovered
   cases that no amount of content review would have surfaced.
10. **Section order is a correctness property, not styling.** B8 sat under the "Group A"
    heading, and "Generalizing Beyond the Examples" opened with "every check names an instance"
    while positioned above any check.
11. **A bare example is read as the rule's full scope, by humans and models alike.** This
    shaped how the passes are worded: each states its principle in words *and* names an
    instance, and cited cases are labelled as cases (*an instance*, *seen in the wild*). Left
    to an example alone, "reject negative `limit`" gets applied to `limit` and nothing else.
    The reader-facing half of this — state the accepted range rather than a sample value —
    belongs in SKILL.md and is there; this entry is the authoring rule.
12. **Scripted edits fail silently; the Edit tool fails loudly.** `if old in t` skips a
    non-matching replacement without complaint, which produced vanished edits, orphan line
    fragments, and a word hyphenated across a line break. This is the skill's own silent-ignore
    defect committed by the tool chosen to fix it.

## Version history

**3.0.0** — Renamed from `parameter-naming-and-guidance`; the old name undersold the scope,
since 5 of 17 passes operate on the callable and SKILL.md never said "function" or "tool name".
Added pass B8 (protocol contract completeness: `additionalProperties`, `outputSchema`,
`annotations`, error channel) and pass A9 (unstated assumptions). Broadened B5 from zero-result
honesty to result-set honesty, closing a hole where Part 1 required ordering disclosure and no
pass verified it. Moved B8 out from under the Group A heading. Added `tests/`. Description
rewritten after a blind triggering test found four overtriggering phrases, one
self-contradiction, and five uncovered use cases. SKILL.md 2,134 → ~1,850 words with all facts
retained; sections reordered so nothing is invoked before it is introduced.

**2.x** — Split Group A (is the text right) from Group B (does the behavior match) and put B
first. Consolidated six files to four: `mistakes-and-fixes.md` folded into the passes it
justified, and three workflow files merged into one document with three Parts.

**1.0.0** — Initial: the Four Facts, naming rules, and the first review passes, drawn from
defects found preparing ai-session-search 1.0.0-rc.1.

This skill is grounded in two evidence classes: **specifications** (external, citable) and
**direct observation** (defects found in real codebases this week). Observation entries name
the artifact so a maintainer can re-verify or discard them as the code changes.

---

## Primary — specifications, verified this session

- [JSON-RPC 2.0 Specification](https://www.jsonrpc.org/specification)
  — confirmed `-32602 "Invalid params"` is the reserved code for invalid method parameters,
  distinct from `-32601 "Method not found"`. Basis for pass B7 treating unknown *tool* and
  unknown *parameter* as separate dispatch paths that must both be checked.
- [Python 3 — Common Sequence Operations](https://docs.python.org/3/library/stdtypes.html)
  — confirmed `s[-5]` returns exactly one item (`len(s) + i` substitution) and raises
  `IndexError` on short input, while `s[-5:]` returns *up to* five items and clamps silently.
  Basis for the Part 1 §1 claim that "negative counts from the end" is insufficient without
  stating arity.

- **pydantic 2.12.3** — `BaseModel(a=2, unknown_key="x")` accepted and silently dropped the
  unknown key. Confirms the Step Zero claim that `extra="ignore"` is the default.
- **Levenshtein vs Jaro-Winkler on prefix-clustered names** — for the typo `search_message`:
  Levenshtein 1 (`search_messages`) vs 5 (`search_sessions`); Jaro-Winkler 0.987 vs 0.856.
  `difflib.get_close_matches` returns both. Corrects an earlier overstatement here:
  Jaro-Winkler ranks correctly but compresses the gap, so a typo-tolerant threshold admits the
  wrong candidate. It does not mis-rank.
- **`difflib.get_close_matches`** — present in the Python 3 standard library, as claimed in the
  library table.
- [MCP `schema.ts`, 2025-06-18](https://raw.githubusercontent.com/modelcontextprotocol/modelcontextprotocol/main/schema/2025-06-18/schema.ts)
  — `ToolAnnotations` declares `title`, `readOnlyHint` (default `false`), `destructiveHint`
  (default **`true`**), `idempotentHint` (default `false`), `openWorldHint` (default `true`),
  and `annotations` is optional on `Tool`. Corrects an earlier claim in pass B8 that missing
  annotations leave a caller unable to tell whether a tool mutates: the spec's defaults mean an
  absent block *asserts* the tool is destructive, non-idempotent, and open-world. The cost is
  therefore borne by read-only tools, which a conforming client must treat as destructive.
  The schema also states `destructiveHint` and `idempotentHint` are meaningful only when
  `readOnlyHint == false`, and that display precedence is `title`, `annotations.title`, `name`.
  Note that `Annotations` (audience, priority, lastModified) is a *content* type and unrelated.
- **GitKraken MCP, 31 tools / 126 parameters**, audited 2026-07-20 — third-party commercial
  server, used to test the checker outside the two codebases it was derived from. Confirmed by
  hand: `git_branch.action` and `git_worktree.action` declare enums (`create`/`list`,
  `list`/`add`) that their descriptions never name; `app_tool_box.directory` has no description.
  Annotations 31/31, outputSchema 0/31, additionalProperties 0/31.

## Primary — specifications, cited from model knowledge, NOT fetched this session

Flagged explicitly because this skill's own rules forbid presenting unverified claims as
verified. Remaining rows in the Step Zero and library tables were not executed either — verify
before relying on any of them for a consequential decision.

- ISO 80000-2 (Quantities and units — Mathematics) defines ℕ as including `0`, while much
  mathematical literature excludes it. Basis for naming rule 6 ("never use math jargon for a
  bound"). The claim needed for the rule is only that the convention is *disputed*, which is
  uncontroversial; the specific standard number is the unverified part. Standard is paywalled.
- [JSON Schema Validation 2020-12](https://json-schema.org/draft/2020-12/json-schema-validation)
  — `minimum` as the keyword whose presence signals whether negatives are accepted. Used as
  background, not as a load-bearing claim.

## Primary — direct observation (the tool's own behavior is ground truth)

- **ai-session-search**, branch `feat/ai-session-search-rust-migration`, commits `1581150`,
  `728852e`, `87b00fe`, `7722279` — source of the unmarked "Seen in the wild" entries in
  Part 2 of the reference, each naming the wrong text and the committed correction.
- **ai-session-search MCP response shape**, observed live 2026-07-20 — an empty
  `search_messages` result echoes `match_mode`, `limit`, and `offset`, but omits `query`,
  `session_id`, `path_prefix`, `provider`, and the seq bounds. Concrete instance of the
  zero-result ambiguity in pass B5. Open at time of writing.

## Secondary — cross-project corroboration

Entries marked `[independent]` in Part 2 of the reference come from a Codex session on an
unrelated codebase during the same week. Recurrence across unrelated projects is the evidence
that these failure modes are structural. Retrieved via `ai-session-search`.

- Session `codex:019f5f0a-8bad-7e41-935a-c446d448ecc4` (repo `codebase-memory-mcp`):
  - seq 33692 — closing the input policy exposed four implemented-but-undeclared parameters
    (`verbose`, `project_name`, `repo_path`, `auto_dep_limit`). Basis for pass B2.
  - seq 33896 — `search_code` + `search_in:"graph"` silently performed a source search.
    Basis for pass B1.
  - seq 34319 — valid-but-wrong project yields an empty result indistinguishable from a
    genuine miss; fixed by echoing the resolved project. Basis for pass B5.
  - seq 24733, 25388, 25417 — advertised `WITH` + `OPTIONAL MATCH` failing to compose; a
    docstring example (`ORDER BY CASE … LIKE`) the grammar cannot execute; an unprojected
    `ORDER BY` key silently ignored. Basis for passes B1, B3, and B4.
  - seq 22947 — `detect_changes.depth` clamped, echoed, and never used to traverse. Basis
    for pass B1.
  - seq 24168, 22197 — property discovery capped at 50 keys with no truncation reported while
    documented as "full"; project premortem naming silent clipping as a cause of false
    "not found" conclusions. Basis for pass B5.
  - seq 33559, 33791 — argument validation is `Ω(k)` in supplied keys; a full typo-rejection
    round trip measured 49.8–61.7 µs median, 70.2–82.2 µs p95 over 900 calls, so a shared
    cache was rejected. Basis for the "measure before optimizing validation" rule.
  - seq 22548, 35478 — a user rejected "reasonable computational cost" as vague (basis for
    pass A6) and questioned `mcp_schema_project_snapshot` as implying database duplication
    (basis for naming rule 7).

## Method

- [Anthropic: The Complete Guide to Building Skills for Claude](https://resources.anthropic.com/hubfs/The-Complete-Guide-to-Building-Skill-for-Claude.pdf)
  (January 2026), via the local `ai-skill-builder` skill — progressive disclosure levels,
  trigger-phrase description format, required `references/sources.md` with per-source
  "what it confirmed" notes.

## Discrepancies

`ai-skill-builder/scripts/audit-skill.sh` produced three warnings on this skill. All three
were traced to the checker rather than the skill, and the script was **fixed in place** —
path handling repaired, and each warning amended to state its intent and permit self-credit
when a differently-shaped skill meets it.

- **Relative paths failed.** `audit-skill.sh .` reported 1 FAIL and 79%; the absolute path
  reported 0 FAIL and 91% on identical content. Cause: `basename "."` returns `.`, failing the
  kebab-case name check. Fixed by resolving with `cd … && pwd -P` before `basename`. Verified
  across `.`, `name`, `name/`, and `./name`.
- **`## How It Works` was grepped literally,** so the check could not see a descriptive
  heading such as `## The Process` — penalizing the naming discipline it should reward.
  Warning rejected for this skill; string amended to name the intent.
- **"No examples found"** looks only in `## Examples` or `references/examples/`. This skill's
  worked examples live inside Part 2 of the reference, beside the passes they illustrate.
  Consolidation was chosen over satisfying the grep; splitting them back out would recreate
  the duplication that merging removed. Warning amended to accept inline examples.
- **The same script warns when SKILL.md lacks "quantitative outcomes"**, grepping for
  `faster|reduction|improvement|save.*time|NN%` (line 314). This contradicts
  `ai-skill-builder/references/best-practices.md:62-91`, which states outcome-focused
  language belongs in a GitHub README and is *wrong* for SKILL.md. Warning rejected as
  internally inconsistent; no invented metrics were added. The real measurements this skill
  does carry (`Ω(k)`, 49.8–61.7 µs, median 148-character descriptions) are facts, not
  positioning, and simply do not match the grep.
- **`best-practices.md:406` recommends a 15–30 word description**, while
  `best-practices.md:48` and the Anthropic guide set a 1024-character hard limit and the
  guide's own Format B example exceeds 30 words. Followed the character limit; this skill's
  description is 790 characters, since trigger-phrase coverage matters more than brevity
  for activation.

## Naming decisions for this skill's own files

The rules apply to filenames and headings too:

| Candidate | Verdict |
|---|---|
| `parameter-naming-and-guidance` | **Renamed away** at v3.0.0. Undersold the scope: 5 of 17 passes (A2, B2, B3, B7, B8) operate on the callable, and SKILL.md mentioned "function" and "tool name" zero times. |
| `designing-and-reviewing-signatures` | Rejected. A signature is name, parameters, and return type — it excludes descriptions and error messages, over half the content. |
| `hard-to-misuse-apis` | Rejected. Names the outcome and covers all five areas, but "APIs" is a stretch for config keys and env vars. |
| `function-signature-and-parameter-guidance` | **Chosen.** `function-signature` rather than bare `signature`, which a cold reader parses as crypto or email (pass A9). `guidance` carries the descriptions and error messages a signature omits. |
| `param-surface-audit` | Rejected. "Surface" is invented jargon; `param` is an abbreviation nobody searches for. |
| `parameter-review` | Rejected. "Review" is a vague verb — it says neither what gets reviewed nor what is produced. |
| `references/gotchas.md` | Rejected. Names a feeling, not contents. |
| `references/designing-and-reviewing-signatures-and-parameters.md` | **Chosen.** Names the two activities it serves. Long, and worth it — a shorter `handbook.md` or `guide.md` would say nothing. |

Two consolidation rounds shaped that last file, both naming problems pointing at design
problems: `mistakes-and-fixes.md` held 21 defects that each restated the pass they justified,
so each moved under its pass as evidence; and three files covering three phases of one
workflow cross-referenced each other constantly, until a `§10` pointer outlived the section it
named. **A file that needs constant pointers into another file is usually one file.**

Section headings follow the same rule: "The Four Facts" and "What to Avoid" say what follows;
"Best Practices" and "Considerations" would not.

#!/usr/bin/env python3
"""Check the mechanically decidable passes against a JSON Schema.

Reads an MCP `tools/list` response, a single JSON Schema, or a list of either, and
reports on the five passes that are string or structure facts. It deliberately does
NOT attempt the passes that need semantics — see the coverage banner it prints.

SCOPE — read before relying on this.

Handles: JSON Schema and MCP `tools/list`, any project exposing either. Because it
reads protocol output rather than source, the server's implementation language is
irrelevant — verified against a C-implemented MCP server (61 parameters, findings
confirmed by hand against the served schema). OpenAPI parameter schemas work too,
being JSON Schema.

Does NOT handle: clap / argparse / click / cobra definitions, `--help` output,
YAML or TOML config schemas, docstrings, function signatures. For those surfaces
use the grep recipes in the passes themselves — the reference gives one per check.

The word lists below are English-only and deliberately short: they favour precision
over recall, so a clean run means "none of these exact patterns", not "no problems".
Three of the five checks are the reference's grep recipes with JSON walking added;
the two that are genuinely easier here are the length statistics and the
`minimum`-absence heuristic.

Usage:
    aise mcp serve <<< '{"jsonrpc":"2.0","id":1,"method":"tools/list"}' \\
        | python3 check-schema-descriptions.py -
    python3 check-schema-descriptions.py schema.json

Pin the artifact. An installed binary usually predates the branch you are reviewing, so
findings may describe a schema you already fixed. Check the build date against your last
commit before acting on any result, or run the freshly built binary.
"""

import json
import re
import statistics
import sys

# Bounds stated as absences. Double negatives invert under paraphrase.
NEGATION = re.compile(
    r"\b(no negative|not negative|non-?negative|must not|cannot be|don't pass)\b", re.I
)
# Guidance shaped like a fact, carrying no threshold.
VAGUE = re.compile(r"\b(reasonable|appropriate|as needed|properly|sensible)\b", re.I)
# A signed-looking integer whose text never says what negatives do.
SIGN_WORDS = re.compile(r"\bnegative\b", re.I)


def walk(schema, tool, path=()):
    """Yield (tool, param_path, name, spec) for every leaf property, recursing into
    nested objects — a container is a namespace, so its members are parameters too."""
    for name, spec in (schema.get("properties") or {}).items():
        here = path + (name,)
        yield tool, ".".join(here), name, spec
        if not isinstance(spec, dict):
            continue
        # Recurse on anything carrying nested properties. Checking `type == "object"` alone
        # misses union types like ["object", "null"] and schemas that declare properties
        # without a type — both common, and both would silently skip every nested parameter.
        declared = spec.get("type")
        types = declared if isinstance(declared, list) else [declared]
        if "object" in types or "properties" in spec:
            yield from walk(spec, tool, here)


def parse(raw):
    """Parse one JSON document, or find the tools/list reply inside JSON-RPC stdio
    output. A stdio server emits one object per line, so a whole-input parse fails
    with 'Extra data' — scan lines and take the one carrying tool definitions."""
    try:
        return json.loads(raw)
    except json.JSONDecodeError:
        pass
    for line in raw.splitlines():
        line = line.strip()
        if not line.startswith(("{", "[")) or '"tools"' not in line:
            continue
        try:
            doc = json.loads(line)
        except json.JSONDecodeError:
            continue
        # `initialize` also carries a "tools" key — capabilities.tools is an object,
        # while tools/list returns a non-empty array. Require the array.
        found = doc.get("result", doc) if isinstance(doc, dict) else doc
        if isinstance(found, dict) and isinstance(found.get("tools"), list):
            return doc
    sys.exit(
        "No JSON schema found. Pass a JSON Schema file, a tools/list response, or pipe\n"
        "stdio server output containing a line with a \"tools\" array."
    )


def load(raw):
    """Accept a tools/list response, a bare tool array, or one schema."""
    doc = parse(raw)
    if isinstance(doc, dict) and "result" in doc:
        doc = doc["result"]
    if isinstance(doc, dict) and "tools" in doc:
        return [(t.get("name", "?"), t.get("inputSchema") or {}) for t in doc["tools"]]
    if isinstance(doc, list):
        return [(t.get("name", "?"), t.get("inputSchema") or t) for t in doc]
    return [(doc.get("title", "schema"), doc)]


def tool_level_findings(raw):
    """Checks that need the whole tool list, not one parameter: protocol contract
    completeness, and one name carrying different meanings across tools."""
    out = []
    doc = parse(raw)
    tools = (doc.get("result", doc) if isinstance(doc, dict) else {}).get("tools")
    if not isinstance(tools, list):
        return out  # a bare schema has no tool-level contract to check

    enums, descs = {}, {}
    for t in tools:
        name = t.get("name", "?")
        schema = t.get("inputSchema") or {}
        # Unknown keys are silently dropped unless the schema forbids them.
        if schema.get("additionalProperties") is not False:
            out.append(("permissive-schema", name,
                        "additionalProperties is not false: a misspelled argument is "
                        "accepted and ignored"))
        # Without an output schema the caller cannot predict the response shape.
        if "outputSchema" not in t:
            out.append(("no-output-schema", name,
                        "no outputSchema: the response shape is unguessable before calling"))
        # Absent annotations do not leave the effect class unknown — the MCP defaults
        # assert it. destructiveHint defaults true and openWorldHint defaults true, so
        # omitting the block declares the tool destructive, non-idempotent, and
        # open-world. A read-only tool pays for that silence.
        annotations = t.get("annotations")
        if not isinstance(annotations, dict):
            out.append(("no-annotations", name,
                        "no annotations: the defaults assert destructive, non-idempotent, "
                        "and open-world. A read-only tool must declare readOnlyHint or a "
                        "conforming client treats it like a delete"))
        elif annotations.get("readOnlyHint") is True:
            # destructiveHint and idempotentHint are meaningful only when readOnlyHint
            # is false, so declaring them alongside it states a contradiction.
            contradictory = [k for k in ("destructiveHint", "idempotentHint")
                             if annotations.get(k) is True]
            if contradictory:
                out.append(("contradictory-annotations", name,
                            f"readOnlyHint is true, so {' and '.join(contradictory)} "
                            f"cannot apply — the schema scopes them to readOnlyHint == false"))
        for pname, spec in (schema.get("properties") or {}).items():
            if isinstance(spec, dict):
                if "enum" in spec:
                    enums.setdefault(pname, {})[name] = tuple(spec["enum"])
                descs.setdefault(pname, {})[name] = spec.get("description", "")

    for pname, per_tool in sorted(enums.items()):
        if len(set(per_tool.values())) > 1:
            shown = "; ".join(f"{k}={list(v)}" for k, v in per_tool.items())
            out.append(("overloaded-name", pname,
                        f"same name, different value space per tool: {shown}"))
    for pname, per_tool in sorted(descs.items()):
        texts = {d for d in per_tool.values() if d}
        if len(per_tool) > 1 and len(texts) > 1:
            out.append(("divergent-description", pname,
                        f"described differently on {len(per_tool)} tools "
                        f"({len(texts)} distinct texts) — verify they mean the same thing"))
    return out


def main():
    source = sys.argv[1] if len(sys.argv) > 1 else "-"
    raw = sys.stdin.read() if source == "-" else open(source).read()

    findings, lengths = [], []
    total = 0
    findings.extend(tool_level_findings(raw))

    for tool, schema in load(raw):
        for tool, path, name, spec in walk(schema, tool):
            if not isinstance(spec, dict):
                continue
            total += 1
            text = spec.get("description", "")
            where = f"{tool}.{path}"

            if not text:
                findings.append(("no-description", where, "no description"))
                continue
            lengths.append(len(text))

            if m := NEGATION.search(text):
                findings.append(
                    ("negation", where, f"bound stated as an absence: {m.group(0)!r}")
                )
            if m := VAGUE.search(text):
                findings.append(
                    ("vague-word", where, f"vague qualitative word: {m.group(0)!r}")
                )
            if "default" in spec and "default" not in text.lower():
                findings.append(
                    ("undocumented-default", where, f"declared default {spec['default']!r} absent from text")
                )
            if "enum" in spec:
                low = text.lower()
                missing = [v for v in spec["enum"] if str(v).lower() not in low]
                if missing:
                    findings.append(
                        ("undocumented-enum-value", where,
                         f"accepted values absent from the description: {missing}")
                    )
            # Integer with no `minimum` accepts negatives; the text must say what they do.
            if spec.get("type") == "integer" and "minimum" not in spec:
                if not SIGN_WORDS.search(text):
                    findings.append(
                        ("unstated-sign", where, "accepts negatives, text does not say what they select")
                    )

    # Zero parameters means the input did not parse as expected, NOT that the schema is
    # clean. Reporting "no findings" here would be this skill's own failure #19: an empty
    # result indistinguishable from a genuine pass.
    if total == 0:
        sys.exit(
            "Found 0 parameters — the input parsed but exposed no `properties`.\n"
            "This is an input problem, not a clean result. Check that the JSON is a\n"
            "tools/list reply, a tool array, or a schema with a `properties` object."
        )

    # Severity order matches this skill's thesis: silent-success defects first, then what
    # the caller cannot know, then confirmed text defects, then candidates needing judgment.
    TIERS = [
        ("1. SILENT — accepted and ignored, no error reaches the caller",
         {"permissive-schema"}),
        ("2. UNDECLARED — the caller cannot know this before calling",
         {"no-output-schema", "no-annotations"}),
        ("3. TEXT DEFECT — confirmed, the description omits a stated fact",
         {"negation", "undocumented-default", "undocumented-enum-value",
          "unstated-sign", "no-description", "contradictory-annotations"}),
        ("4. CANDIDATE — needs judgment, may be a working convention",
         {"overloaded-name", "divergent-description", "vague-word"}),
    ]
    print(f"Checked {total} parameters across the mechanically decidable checks.\n")
    confirmed = 0
    for title, codes in TIERS:
        rows = [f for f in findings if f[0] in codes]
        if not rows:
            continue
        print(f"{title}")
        for code, where, why in rows:
            print(f"  {code}  {where}\n        {why}")
        print()
        if not title.startswith("4."):
            confirmed += len(rows)

    if any(f[0] in TIERS[3][1] for f in findings):
        print("Tier 4 is a candidate list, not a defect list. A shared name whose values carry a\n"
              "consistent meaning across tools is a working convention — read the descriptions\n"
              "before changing anything. Tier 4 does not affect the exit code.\n")
    if not findings:
        print("  No findings from any mechanical check.")

    if lengths:
        print(
            f"\nlength  median {int(statistics.median(lengths))} chars, "
            f"max {max(lengths)} — justify each outlier by hand."
        )

    print(
        "\nNOT CHECKED — these need judgment and are not attempted here:\n"
        "  availability, semantic duplication, grammatical attachment, full sign\n"
        "  completeness, silent ignore, schema drift, composition, examples-execute,\n"
        "  zero-result honesty, cross-surface parity, unknown-name handling,\n"
        "  unstated assumptions\n"
        "A clean run here is NOT a clean review: the unchecked passes catch the defects that\nreturn success, which are the ones that matter most."
    )
    return 1 if confirmed else 0


if __name__ == "__main__":
    sys.exit(main())

#!/usr/bin/env python3
"""Tests for scripts/check-schema-descriptions.py.

Every bug this script has shipped is pinned here as a regression. Two of the three
were the skill's own defects living in the skill's own checker — a false clean bill
on zero input (pass B5) and a value silently skipped (pass B1) — which is why they
are tested rather than merely fixed.

Run: python3 tests/test_check_schema_descriptions.py
"""

import json
import pathlib
import subprocess
import sys
import unittest

SCRIPT = pathlib.Path(__file__).resolve().parent.parent / "scripts" / "check-schema-descriptions.py"


def run(payload):
    """Feed the checker on stdin; return (exit_code, stdout+stderr)."""
    text = payload if isinstance(payload, str) else json.dumps(payload)
    proc = subprocess.run(
        [sys.executable, str(SCRIPT), "-"],
        input=text, capture_output=True, text=True,
    )
    return proc.returncode, proc.stdout + proc.stderr


def tool(name="t", props=None, strict=False, **extra):
    """Build a tool definition. `strict` sets additionalProperties: false, which is
    needed whenever a test wants no tier-1 finding to fire."""
    schema = {"type": "object", "properties": props or {}}
    if strict:
        schema["additionalProperties"] = False
    return {"name": name, "inputSchema": schema, **extra}


def clean_tool(name, props):
    """A tool with nothing for the tool-level checks to report."""
    return tool(name, props, strict=True, outputSchema={}, annotations={})


class Regressions(unittest.TestCase):
    """One test per bug the script has actually shipped."""

    def test_multiline_jsonrpc_stdio_does_not_crash(self):
        """Bug 1: a whole-input json.loads raised 'Extra data' on stdio output.

        A stdio MCP server emits one JSON object per line. The checker must find the
        tools/list reply among them rather than fail to parse the stream.
        """
        stream = "\n".join([
            json.dumps({"jsonrpc": "2.0", "id": 1,
                        "result": {"capabilities": {"tools": {}}}}),
            json.dumps({"jsonrpc": "2.0", "method": "notifications/initialized"}),
            json.dumps({"jsonrpc": "2.0", "id": 2,
                        "result": {"tools": [tool(props={"a": {"type": "string",
                                                              "description": "x"}})]}}),
        ])
        code, out = run(stream)
        self.assertIn("Checked 1 parameters", out, out)

    def test_initialize_reply_is_not_mistaken_for_tools_list(self):
        """Bug 1b: `initialize` also carries a "tools" key, but as an object.

        Selecting the first line containing "tools" picked capabilities.tools = {},
        yielding zero parameters and a false clean bill.
        """
        stream = "\n".join([
            json.dumps({"jsonrpc": "2.0", "id": 1,
                        "result": {"capabilities": {"tools": {"listChanged": True}}}}),
            json.dumps({"jsonrpc": "2.0", "id": 2,
                        "result": {"tools": [tool(props={"a": {"type": "string",
                                                              "description": "x"}})]}}),
        ])
        code, out = run(stream)
        self.assertIn("Checked 1 parameters", out, out)

    def test_zero_parameters_is_an_error_not_a_clean_result(self):
        """Bug 2: reported "No findings" when it had parsed nothing.

        This is pass B5 applied to the checker: an empty result that looks identical
        to a genuine pass. It must exit non-zero and name the likely cause.
        """
        code, out = run({"tools": []})
        self.assertNotEqual(code, 0, "zero parameters must not exit clean")
        self.assertIn("0 parameters", out)
        self.assertNotIn("No findings", out)

    def test_nested_properties_under_a_type_union_are_walked(self):
        """Bug 3: recursion required type == "object" exactly.

        A nullable object declares type ["object", "null"], so every nested parameter
        was skipped in silence — pass B1 (accepted but never reaching the behavior).
        """
        code, out = run({"tools": [tool(props={
            "opts": {
                "type": ["object", "null"],
                "description": "options",
                "properties": {"inner": {"type": "integer",
                                         "description": "must not be negative"}},
            }
        })]})
        self.assertIn("t.opts.inner", out, "nested parameter under a type union was skipped")
        self.assertIn("negation", out)

    def test_nested_properties_without_a_declared_type_are_walked(self):
        """Bug 3b: schemas may declare `properties` and omit `type` entirely."""
        code, out = run({"tools": [tool(props={
            "opts": {"description": "options",
                     "properties": {"inner": {"type": "string",
                                              "description": "reasonable size"}}}
        })]})
        self.assertIn("t.opts.inner", out)
        self.assertIn("vague-word", out)

    def test_enum_documentation_check_is_case_insensitive(self):
        """A case-sensitive check flagged `toon` while the description said TOON.

        Found as a false positive in the checker's own output during an audit.
        """
        code, out = run({"tools": [tool(props={
            "format": {"type": "string", "enum": ["toon", "json"],
                       "description": "Compact TOON tables by default; json returns objects."}
        })]})
        self.assertNotIn("undocumented-enum-value", out, out)


class Severity(unittest.TestCase):
    """Tier assignment decides what fails CI, so it is pinned."""

    def test_candidates_alone_do_not_fail_ci(self):
        """A shared name with differing enums may be a working convention.

        The `mode`/`full` case proved this: seven differing enums, one consistent
        meaning. Candidates report but must not break a build.
        """
        code, out = run({"tools": [
            clean_tool("a", {"mode": {"type": "string", "enum": ["x"],
                                      "description": "mode x"}}),
            clean_tool("b", {"mode": {"type": "string", "enum": ["y"],
                                      "description": "mode y"}}),
        ]})
        self.assertIn("overloaded-name", out)
        self.assertIn("CANDIDATE", out)
        self.assertEqual(code, 0, "a candidate-only run must exit clean")

    def test_confirmed_defects_fail_ci(self):
        code, out = run({"tools": [tool(props={
            "a": {"type": "string", "description": "must not be empty"}
        })]})
        self.assertIn("negation", out)
        self.assertNotEqual(code, 0)

    def test_silent_tier_reported_before_text_tier(self):
        """Group B outranks Group A, so tier 1 must print above tier 3."""
        code, out = run({"tools": [tool(props={
            "a": {"type": "string", "description": "must not be empty"}
        })]})
        self.assertLess(out.index("1. SILENT"), out.index("3. TEXT DEFECT"), out)

    def test_output_names_the_passes_it_did_not_run(self):
        """A clean run must never read as a clean review."""
        code, out = run({"tools": [clean_tool("t", {"a": {"type": "string",
                                                          "description": "x"}})]})
        self.assertIn("NOT CHECKED", out)
        self.assertIn("clean run here is NOT a clean review", out)


class ToolLevelChecks(unittest.TestCase):
    def test_permissive_schema_is_flagged_as_silent(self):
        code, out = run({"tools": [tool(props={"a": {"type": "string", "description": "x"}})]})
        self.assertIn("permissive-schema", out)
        self.assertLess(out.index("1. SILENT"), out.index("permissive-schema") + 1)

    def test_strict_schema_is_not_flagged(self):
        t = tool(props={"a": {"type": "string", "description": "x"}})
        t["inputSchema"]["additionalProperties"] = False
        code, out = run({"tools": [t]})
        self.assertNotIn("permissive-schema", out)

    def test_readonly_tool_declaring_destructive_is_flagged(self):
        """The schema scopes destructiveHint and idempotentHint to readOnlyHint == false,
        so declaring both states a contradiction the caller cannot resolve."""
        t = clean_tool("t", {"a": {"type": "string", "description": "x"}})
        t["annotations"] = {"readOnlyHint": True, "destructiveHint": True}
        code, out = run({"tools": [t]})
        self.assertIn("contradictory-annotations", out)
        self.assertNotEqual(code, 0, "a stated contradiction is a confirmed defect")

    def test_consistent_annotations_are_not_flagged(self):
        t = clean_tool("t", {"a": {"type": "string", "description": "x"}})
        t["annotations"] = {"readOnlyHint": True}
        code, out = run({"tools": [t]})
        self.assertNotIn("contradictory-annotations", out)
        self.assertNotIn("no-annotations", out)

    def test_missing_annotations_message_names_the_default_not_uncertainty(self):
        """Absent annotations assert destructive rather than leaving it unknown."""
        code, out = run({"tools": [tool(props={"a": {"type": "string", "description": "x"}})]})
        self.assertIn("no-annotations", out)
        self.assertIn("assert destructive", out)

    def test_missing_annotations_and_output_schema_are_flagged(self):
        code, out = run({"tools": [tool(props={"a": {"type": "string", "description": "x"}})]})
        self.assertIn("no-annotations", out)
        self.assertIn("no-output-schema", out)

    def test_bare_schema_input_skips_tool_level_checks(self):
        """A plain JSON Schema has no tool contract to check; it must not be flagged."""
        code, out = run({"properties": {"a": {"type": "string", "description": "x"}}})
        self.assertNotIn("no-annotations", out)
        self.assertIn("Checked 1 parameters", out)


class ParameterChecks(unittest.TestCase):
    def test_integer_without_minimum_is_flagged_unless_sign_is_documented(self):
        code, out = run({"tools": [tool(props={
            "n": {"type": "integer", "description": "a count"}
        })]})
        self.assertIn("unstated-sign", out)

    def test_integer_documenting_negatives_is_not_flagged(self):
        code, out = run({"tools": [tool(props={
            "n": {"type": "integer",
                  "description": "positive keeps the first N, negative keeps the last N, 0 all"}
        })]})
        self.assertNotIn("unstated-sign", out)

    def test_declared_default_absent_from_description_is_flagged(self):
        code, out = run({"tools": [tool(props={
            "n": {"type": "string", "default": "x", "description": "a name"}
        })]})
        self.assertIn("undocumented-default", out)


if __name__ == "__main__":
    unittest.main(verbosity=2)