mirror of
https://github.com/mongodb/agent-skills.git
synced 2026-09-18 21:15:11 +08:00
refactor(lint-item-echo): make the echo lint a true port of analysis/echo.py
The two implementations shared constants via echo-thresholds.json but not a tokenizer: the lint split on non-alphanumerics and kept domain vocabulary, echo.py kept hyphens/sigils and stripped it — so the shared 0.1 floor was applied to differently-computed quantities, and the lint's verbatim-span rule used a consecutive-n-gram-run approximation that could stitch a 'span' from matches at different corpus locations. Now a true port, verified numerically (containment/span outputs identical on shared fixtures): same regex tokenizer with the same sigil/hyphen normalisation, fenced-code blocks stripped on BOTH sides (an item restating a code example is legitimate reuse, not echo), the stopword list promoted into echo-thresholds.json as part of the shared contract, the exact binary-search longest-common-run algorithm for the span, and corpus construction aligned (sorted recursive references/**). The one deliberate difference is reporting granularity: the lint scores prompt and answer fields separately, the Python module pools them. Recalibration on the corpus: the one prior finding (mcp-setup case 2, 11% containment) goes away — it was carried by domain vocabulary, the false- positive class the domain stopwords exist to suppress. The lint is restructured to validate-evals.mjs's pattern (exports + main guard) so lint-item-echo.test.mjs can pin the port; its fixtures and expected values are identical to test_echo.py's — the two repos' CIs can't import each other, so both pin the same numbers.
This commit is contained in:
Cory Bullinger
@@ -14,6 +14,7 @@ on:
|
||||
- "testing/validate-evals.mjs"
|
||||
- "testing/validate-evals.test.mjs"
|
||||
- "testing/lint-item-echo.mjs"
|
||||
- "testing/lint-item-echo.test.mjs"
|
||||
- "testing/echo-thresholds.json"
|
||||
- "testing/package.json"
|
||||
# The lockfile too: `npm ci` installs from it, so a lockfile-only change (a dependency
|
||||
@@ -41,8 +42,13 @@ jobs:
|
||||
run: npm ci --prefix testing --no-audit --no-fund
|
||||
- name: Validate eval cases against schema, check referenced assets exist
|
||||
run: node testing/validate-evals.mjs
|
||||
- name: Pin the files-containment rule (cross-skill answer-key handover guard)
|
||||
run: node --test testing/validate-evals.test.mjs
|
||||
- name: Pin the files-containment rule and the echo-lint's Python parity
|
||||
# validate-evals.test.mjs pins the cross-skill answer-key handover guard;
|
||||
# lint-item-echo.test.mjs pins the tokenizer/span behaviour that keeps this lint a
|
||||
# true port of agent-skills-evals's analysis/echo.py (same fixtures and expected
|
||||
# values as test_echo.py — the two repos' CIs can't import each other, so both pin
|
||||
# the same numbers).
|
||||
run: node --test testing/validate-evals.test.mjs testing/lint-item-echo.test.mjs
|
||||
- name: Lint for items that echo their own skill's guidance text (advisory)
|
||||
# --base enables the co-movement check: an edit to SKILL.md that RAISES an eval
|
||||
# item's overlap with SKILL.md is teaching to the test, and it is detectable from
|
||||
|
||||
@@ -1,5 +1,124 @@
|
||||
{
|
||||
"_comment": "Answer-echo detection constants, declared once here because two implementations consume them: testing/lint-item-echo.mjs (this repo, runs on every PR, no model calls, no private checkout) and agent-skills-evals/inspect/analysis/echo.py (the offline analysis layer). The duplication of the ALGORITHM across a public JS port and a private Python module is deliberate -- the lint has to run on a public PR without cloning a private repo -- but duplicated CONSTANTS would mean the two disagree about what counts as echoing, and only one of them would be the one anybody reads.",
|
||||
"_comment": "Answer-echo detection constants AND tokenizer definition, declared once here because two implementations consume them: testing/lint-item-echo.mjs (this repo, runs on every PR, no model calls, no private checkout) and agent-skills-evals/inspect/analysis/echo.py (the offline analysis layer). The ALGORITHM is implemented twice because the lint has to run on a public PR without cloning a private repo -- but it is a TRUE PORT: both sides tokenize identically (same regex, same fenced-code stripping, same stopword list from this file) and compute the same verbatim span, so the constants below mean the same quantity on both sides. The only deliberate difference is reporting granularity: the lint scores prompt and answer fields separately (leakage vs legitimate restatement read differently in review); the analysis layer pools them.",
|
||||
"domainStopwords": [
|
||||
"a",
|
||||
"an",
|
||||
"and",
|
||||
"are",
|
||||
"as",
|
||||
"at",
|
||||
"be",
|
||||
"by",
|
||||
"for",
|
||||
"from",
|
||||
"has",
|
||||
"have",
|
||||
"in",
|
||||
"into",
|
||||
"is",
|
||||
"it",
|
||||
"its",
|
||||
"of",
|
||||
"on",
|
||||
"or",
|
||||
"that",
|
||||
"the",
|
||||
"to",
|
||||
"with",
|
||||
"you",
|
||||
"your",
|
||||
"this",
|
||||
"these",
|
||||
"those",
|
||||
"if",
|
||||
"then",
|
||||
"when",
|
||||
"use",
|
||||
"using",
|
||||
"used",
|
||||
"should",
|
||||
"must",
|
||||
"can",
|
||||
"will",
|
||||
"would",
|
||||
"may",
|
||||
"mongodb",
|
||||
"mongo",
|
||||
"db",
|
||||
"database",
|
||||
"databases",
|
||||
"collection",
|
||||
"collections",
|
||||
"document",
|
||||
"documents",
|
||||
"field",
|
||||
"fields",
|
||||
"index",
|
||||
"indexes",
|
||||
"indices",
|
||||
"query",
|
||||
"queries",
|
||||
"aggregate",
|
||||
"aggregation",
|
||||
"pipeline",
|
||||
"stage",
|
||||
"stages",
|
||||
"find",
|
||||
"filter",
|
||||
"sort",
|
||||
"limit",
|
||||
"skip",
|
||||
"project",
|
||||
"group",
|
||||
"match",
|
||||
"lookup",
|
||||
"unwind",
|
||||
"count",
|
||||
"explain",
|
||||
"executionstats",
|
||||
"keypattern",
|
||||
"ixscan",
|
||||
"collscan",
|
||||
"compound",
|
||||
"single",
|
||||
"equality",
|
||||
"range",
|
||||
"connection",
|
||||
"connect",
|
||||
"client",
|
||||
"pool",
|
||||
"poolsize",
|
||||
"timeout",
|
||||
"uri",
|
||||
"driver",
|
||||
"search",
|
||||
"vector",
|
||||
"embedding",
|
||||
"atlas",
|
||||
"cluster",
|
||||
"mcp",
|
||||
"server",
|
||||
"tool",
|
||||
"tools",
|
||||
"skill",
|
||||
"agent",
|
||||
"schema",
|
||||
"design",
|
||||
"pattern",
|
||||
"embed",
|
||||
"reference",
|
||||
"bucket",
|
||||
"create",
|
||||
"created",
|
||||
"creates",
|
||||
"make",
|
||||
"made",
|
||||
"get",
|
||||
"set",
|
||||
"add",
|
||||
"added",
|
||||
"remove"
|
||||
],
|
||||
"n": 8,
|
||||
"minAbsoluteContainment": 0.1,
|
||||
"maxVerbatimSpan": 20,
|
||||
|
||||
+196
-170
@@ -24,16 +24,22 @@
|
||||
* would mostly measure the second and call it the first.
|
||||
*
|
||||
* 2. **Verbatim span.** A shared run of ≥ maxVerbatimSpan tokens is a quotation whatever
|
||||
* the containment score says.
|
||||
* the containment score says. Computed as the true longest common token run (the rule
|
||||
* that must never false-positive), over RAW tokens — a quotation includes the ordinary
|
||||
* words, and stripping them would fragment the run and understate the copy.
|
||||
*
|
||||
* 3. **Co-movement** (`--base <ref>`, the check that needs no held-out set). If a PR edits
|
||||
* a skill's guidance AND that edit raises an item's containment, that is teaching to the
|
||||
* test — detectable from the diff alone, no curator and no golden corpus required. This
|
||||
* is the highest-value check here and it is pure string processing; it mirrors
|
||||
* agent-skills-evals/inspect/analysis/echo.py::comovement.
|
||||
* is the highest-value check here and it is pure string processing.
|
||||
*
|
||||
* Thresholds come from testing/echo-thresholds.json, shared with the Python analysis
|
||||
* module so the two implementations cannot disagree about what counts as echoing.
|
||||
* echo-thresholds.json is the complete shared contract with the Python analysis layer
|
||||
* (agent-skills-evals/inspect/analysis/echo.py): the constants AND the tokenizer
|
||||
* definition (regex, fenced-code stripping, stopword list). This file is a TRUE PORT —
|
||||
* both sides compute the same quantities, so the shared constants mean the same thing.
|
||||
* The one deliberate difference is reporting granularity: the lint scores `prompt` and
|
||||
* answer fields separately (leakage vs legitimate restatement read differently in
|
||||
* review); the Python module pools them.
|
||||
*
|
||||
* Advisory today (prints, exits 0) unless --strict is passed.
|
||||
* TODO: flip validate-eval-cases.yml to --strict once the thresholds have been checked
|
||||
@@ -41,9 +47,9 @@
|
||||
* answer.
|
||||
*/
|
||||
import { execFileSync } from "node:child_process";
|
||||
import { readFileSync, readdirSync } from "node:fs";
|
||||
import { readFileSync } from "node:fs";
|
||||
import { dirname, join, relative } from "node:path";
|
||||
import { fileURLToPath } from "node:url";
|
||||
import { fileURLToPath, pathToFileURL } from "node:url";
|
||||
import { globSync } from "glob";
|
||||
|
||||
const here = dirname(fileURLToPath(import.meta.url));
|
||||
@@ -65,11 +71,6 @@ const MIN_CONTAINMENT = T.minAbsoluteContainment;
|
||||
const MAX_SPAN = T.maxVerbatimSpan;
|
||||
const CO_MOVEMENT_MIN_DELTA = T.coMovementMinDelta;
|
||||
|
||||
const args = process.argv.slice(2);
|
||||
const STRICT = args.includes("--strict");
|
||||
const baseIdx = args.indexOf("--base");
|
||||
const BASE_REF = baseIdx !== -1 ? args[baseIdx + 1] : null;
|
||||
|
||||
// In GitHub Actions, surface findings as PR annotations (file-attached ::warning) so they
|
||||
// appear in the Files Changed review view even while the lint is advisory (exit 0). A stdout
|
||||
// line in a green workflow log is invisible to a reviewer; an annotation is not. The lint
|
||||
@@ -95,21 +96,32 @@ function reportFinding(file, message) {
|
||||
console.log(`⚠ ${rel}: ${message}`);
|
||||
}
|
||||
|
||||
const STOPWORDS = new Set(
|
||||
"a an the of to in on for and or is are was were be been being with as at by from this " +
|
||||
"that it its into if then else not do does did you your".split(" "),
|
||||
);
|
||||
// The stopword list is part of the shared contract (echo-thresholds.json), not a local
|
||||
// choice: two implementations with different tokenizers compute different containment
|
||||
// values for the SAME shared floor, which is precisely the disagreement the file exists
|
||||
// to prevent. Domain vocabulary (index, collection, schema, …) is stripped — those are
|
||||
// the words an item MUST use to describe its task, so leaving them in makes legitimate
|
||||
// items look like copies.
|
||||
const STOPWORDS = new Set(T.domainStopwords ?? []);
|
||||
|
||||
function tokenize(text) {
|
||||
return (text ?? "")
|
||||
.toLowerCase()
|
||||
.replace(/```[\s\S]*?```/g, " ") // fenced code blocks are not prose to compare
|
||||
.replace(/[^a-z0-9\s]/g, " ")
|
||||
.split(/\s+/)
|
||||
.filter((w) => w && !STOPWORDS.has(w));
|
||||
/**
|
||||
* True port of agent-skills-evals/inspect/analysis/echo.py::tokenize — same fenced-code
|
||||
* stripping, same regex, same sigil/hyphen normalisation, same stopword list. Fenced code
|
||||
* blocks are stripped first: code examples in a skill are not prose to quote, and an item
|
||||
* restating a code block is legitimate reuse, not echo. Operator-ish tokens keep their
|
||||
* sigil through the split so they can be matched against the stopword list without it,
|
||||
* which is why the normalisation is a second pass.
|
||||
*/
|
||||
export function tokenize(text, { stripDomain = true } = {}) {
|
||||
const noFences = (text ?? "").toLowerCase().replace(/```[\s\S]*?```/g, " ");
|
||||
const raw = noFences.match(/[a-z_$][a-z0-9_$-]*/g) ?? [];
|
||||
const tokens = raw
|
||||
.map((t) => t.replace(/^\$+/, "").replace(/^[-_]+|[-_]+$/g, ""))
|
||||
.filter(Boolean);
|
||||
return stripDomain ? tokens.filter((t) => !STOPWORDS.has(t)) : tokens;
|
||||
}
|
||||
|
||||
function ngrams(tokens, n) {
|
||||
export function ngrams(tokens, n) {
|
||||
const grams = new Set();
|
||||
for (let i = 0; i + n <= tokens.length; i++) {
|
||||
grams.add(tokens.slice(i, i + n).join(" "));
|
||||
@@ -118,45 +130,48 @@ function ngrams(tokens, n) {
|
||||
}
|
||||
|
||||
// containment = fraction of the ITEM's n-grams that also appear in the corpus
|
||||
function containment(itemGrams, corpusGrams) {
|
||||
export function containment(itemGrams, corpusGrams) {
|
||||
if (itemGrams.size === 0) return 0;
|
||||
let hits = 0;
|
||||
for (const g of itemGrams) if (corpusGrams.has(g)) hits++;
|
||||
return hits / itemGrams.size;
|
||||
}
|
||||
|
||||
// Longest run of consecutive item n-grams that each independently appear in the corpus —
|
||||
// an approximation of "verbatim shared span," cheap because item text is always short
|
||||
// (one prompt/expected_output, not the whole corpus).
|
||||
function longestSharedSpan(tokens, corpusGrams) {
|
||||
let best = 0;
|
||||
let run = 0;
|
||||
for (let i = 0; i + N <= tokens.length; i++) {
|
||||
const g = tokens.slice(i, i + N).join(" ");
|
||||
if (corpusGrams.has(g)) {
|
||||
run++;
|
||||
best = Math.max(best, run);
|
||||
} else {
|
||||
run = 0;
|
||||
/**
|
||||
* True longest run of consecutive tokens present in both texts — the verbatim-span rule
|
||||
* is the one that must never false-positive, so this is the exact algorithm echo.py's
|
||||
* longest_verbatim_span uses (binary search over run length over rolling k-gram sets),
|
||||
* NOT a consecutive-n-gram-run approximation, which can stitch a "span" from matches at
|
||||
* different corpus locations.
|
||||
*/
|
||||
export function longestVerbatimSpan(a, b) {
|
||||
if (a.length === 0 || b.length === 0) return 0;
|
||||
const sharesRun = (k) => {
|
||||
if (k <= 0 || a.length < k || b.length < k) return false;
|
||||
const bRuns = new Set();
|
||||
for (let i = 0; i + k <= b.length; i++) bRuns.add(b.slice(i, i + k).join(" "));
|
||||
for (let i = 0; i + k <= a.length; i++) {
|
||||
if (bRuns.has(a.slice(i, i + k).join(" "))) return true;
|
||||
}
|
||||
return false;
|
||||
};
|
||||
if (!sharesRun(1)) return 0;
|
||||
let lo = 1;
|
||||
let hi = Math.min(a.length, b.length);
|
||||
while (lo < hi) {
|
||||
const mid = Math.floor((lo + hi + 1) / 2);
|
||||
if (sharesRun(mid)) lo = mid;
|
||||
else hi = mid - 1;
|
||||
}
|
||||
return best === 0 ? 0 : best + N - 1; // consecutive overlapping n-grams -> word span length
|
||||
return lo;
|
||||
}
|
||||
|
||||
/** Every markdown file that makes up a skill's guidance surface, as repo-relative paths. */
|
||||
function skillCorpusFiles(skillName) {
|
||||
const dir = join(repoRoot, "skills", skillName);
|
||||
const files = [];
|
||||
files.push(join(dir, "SKILL.md"));
|
||||
const refsDir = join(dir, "references");
|
||||
try {
|
||||
for (const f of readdirSync(refsDir)) {
|
||||
if (f.endsWith(".md")) files.push(join(refsDir, f));
|
||||
}
|
||||
} catch {
|
||||
/* no references/ dir is fine */
|
||||
}
|
||||
return files;
|
||||
// Sorted recursive glob, matching the Python side's sorted(rglob("*.md")) — file order
|
||||
// shifts boundary n-grams, so it is part of the port, not a nicety.
|
||||
return [join(dir, "SKILL.md"), ...globSync(join(dir, "references", "**", "*.md")).sort()];
|
||||
}
|
||||
|
||||
function readCorpus(files) {
|
||||
@@ -213,134 +228,145 @@ function itemFields(ev) {
|
||||
return { prompt, answer };
|
||||
}
|
||||
|
||||
const evalFiles = globSync(join(here, "*/evals/evals.json")).sort();
|
||||
const bySkill = evalFiles.map((file) => {
|
||||
const doc = readJson(file);
|
||||
const skillName = doc.skill_name;
|
||||
const corpusFiles = skillCorpusFiles(skillName);
|
||||
const corpusText = readCorpus(corpusFiles);
|
||||
// An empty corpus silently makes every containment 0 and every item "clean" -- the lint
|
||||
// would report a green result precisely when it is measuring nothing. The usual cause is
|
||||
// `skill_name` not matching a directory under skills/, which is a config error worth
|
||||
// failing on rather than passing quietly.
|
||||
if (corpusText.trim() === "") {
|
||||
console.error(
|
||||
`✗ ${file}: skill_name '${skillName}' has no readable guidance text under ` +
|
||||
`skills/${skillName}/ (SKILL.md + references/*.md). Nothing could be compared, so ` +
|
||||
`a "no echoing items" result here would be meaningless.`,
|
||||
);
|
||||
process.exit(1);
|
||||
}
|
||||
return {
|
||||
file,
|
||||
skillName,
|
||||
doc,
|
||||
corpusFiles,
|
||||
corpusGrams: ngrams(tokenize(corpusText), N),
|
||||
};
|
||||
});
|
||||
function main() {
|
||||
const args = process.argv.slice(2);
|
||||
const STRICT = args.includes("--strict");
|
||||
const baseIdx = args.indexOf("--base");
|
||||
const BASE_REF = baseIdx !== -1 ? args[baseIdx + 1] : null;
|
||||
|
||||
const perItem = [];
|
||||
|
||||
for (const { file, skillName, doc, corpusGrams: ownCorpus } of bySkill) {
|
||||
for (const ev of doc.evals ?? []) {
|
||||
const fields = itemFields(ev);
|
||||
const entry = { skillName, file, id: ev.id, fields: {} };
|
||||
for (const [role, text] of Object.entries(fields)) {
|
||||
const tokens = tokenize(text);
|
||||
if (tokens.length < N) continue; // too short to judge
|
||||
const itemGrams = ngrams(tokens, N);
|
||||
entry.fields[role] = {
|
||||
tokens,
|
||||
itemGrams,
|
||||
own: containment(itemGrams, ownCorpus),
|
||||
};
|
||||
}
|
||||
if (Object.keys(entry.fields).length > 0) perItem.push(entry);
|
||||
}
|
||||
}
|
||||
|
||||
console.log(
|
||||
`Flagging needs containment >= ${MIN_CONTAINMENT} against the item's own skill, ` +
|
||||
`or a verbatim span >= ${MAX_SPAN} words.`,
|
||||
);
|
||||
|
||||
// A prompt that quotes the guidance is leakage; an expected_output that does is often just
|
||||
// a correctly-stated answer. Same numbers, different verdict, so they are reported apart.
|
||||
const ROLE_NOTE = {
|
||||
prompt: "LEAKAGE: the question carries its own answer",
|
||||
answer: "expected answer restates the guidance (often legitimate — judge in review)",
|
||||
};
|
||||
|
||||
let flagged = 0;
|
||||
for (const item of perItem) {
|
||||
const own = bySkill.find((s) => s.skillName === item.skillName);
|
||||
for (const [role, m] of Object.entries(item.fields)) {
|
||||
const span = longestSharedSpan(m.tokens, own.corpusGrams);
|
||||
const bySpan = span >= MAX_SPAN;
|
||||
const byContainment = m.own >= MIN_CONTAINMENT;
|
||||
if (!bySpan && !byContainment) continue;
|
||||
flagged++;
|
||||
const why = bySpan
|
||||
? `verbatim span of ~${span} words shared with its own skill`
|
||||
: `${(m.own * 100).toFixed(0)}% containment against ${item.skillName}`;
|
||||
reportFinding(item.file, `case ${item.id} [${role}]: ${why} — ${ROLE_NOTE[role]}`);
|
||||
}
|
||||
}
|
||||
|
||||
// Co-movement: did THIS PR's guidance edit raise an item's overlap with the guidance?
|
||||
let coMoved = 0;
|
||||
if (BASE_REF) {
|
||||
for (const { file, skillName, doc, corpusFiles } of bySkill) {
|
||||
const base = readCorpusAtRef(corpusFiles, BASE_REF);
|
||||
const afterText = readCorpus(corpusFiles);
|
||||
if (!base.anyPresent) {
|
||||
console.log(
|
||||
`Co-movement: skipping ${skillName} — no guidance at base, so this PR adds the ` +
|
||||
`skill. Nothing moved; the static check above covers its items.`,
|
||||
const evalFiles = globSync(join(here, "*/evals/evals.json")).sort();
|
||||
const bySkill = evalFiles.map((file) => {
|
||||
const doc = readJson(file);
|
||||
const skillName = doc.skill_name;
|
||||
const corpusFiles = skillCorpusFiles(skillName);
|
||||
const corpusText = readCorpus(corpusFiles);
|
||||
// An empty corpus silently makes every containment 0 and every item "clean" -- the lint
|
||||
// would report a green result precisely when it is measuring nothing. The usual cause is
|
||||
// `skill_name` not matching a directory under skills/, which is a config error worth
|
||||
// failing on rather than passing quietly.
|
||||
if (corpusText.trim() === "") {
|
||||
console.error(
|
||||
`✗ ${file}: skill_name '${skillName}' has no readable guidance text under ` +
|
||||
`skills/${skillName}/ (SKILL.md + references/*.md). Nothing could be compared, so ` +
|
||||
`a "no echoing items" result here would be meaningless.`,
|
||||
);
|
||||
continue;
|
||||
process.exit(1);
|
||||
}
|
||||
const beforeText = base.text;
|
||||
if (beforeText === afterText) continue; // guidance untouched in this PR
|
||||
const beforeGrams = ngrams(tokenize(beforeText), N);
|
||||
const afterGrams = ngrams(tokenize(afterText), N);
|
||||
return {
|
||||
file,
|
||||
skillName,
|
||||
doc,
|
||||
corpusFiles,
|
||||
corpusGrams: ngrams(tokenize(corpusText), N),
|
||||
corpusTokensRaw: tokenize(corpusText, { stripDomain: false }),
|
||||
};
|
||||
});
|
||||
|
||||
const perItem = [];
|
||||
|
||||
for (const { file, skillName, doc, corpusGrams: ownCorpus } of bySkill) {
|
||||
for (const ev of doc.evals ?? []) {
|
||||
for (const [role, text] of Object.entries(itemFields(ev))) {
|
||||
const fields = itemFields(ev);
|
||||
const entry = { skillName, file, id: ev.id, fields: {} };
|
||||
for (const [role, text] of Object.entries(fields)) {
|
||||
// No minimum-length skip: containment of an item too short to form an n-gram is 0
|
||||
// (not a fabricated score), and the verbatim-span rule must still see short items —
|
||||
// a copied run long enough to trip maxVerbatimSpan on raw tokens can strip below N.
|
||||
const tokens = tokenize(text);
|
||||
if (tokens.length < N) continue;
|
||||
const grams = ngrams(tokens, N);
|
||||
const before = containment(grams, beforeGrams);
|
||||
const after = containment(grams, afterGrams);
|
||||
const delta = after - before;
|
||||
if (delta < CO_MOVEMENT_MIN_DELTA) continue;
|
||||
coMoved++;
|
||||
reportFinding(
|
||||
file,
|
||||
`CO-MOVEMENT case ${ev.id} [${role}]: this PR's edit to ${skillName}'s ` +
|
||||
`guidance raised containment ${before.toFixed(2)} → ${after.toFixed(2)} ` +
|
||||
`(+${delta.toFixed(2)}). Added guidance text that overlaps an eval item is ` +
|
||||
`teaching to the test, whatever the absolute number is.`,
|
||||
entry.fields[role] = {
|
||||
rawTokens: tokenize(text, { stripDomain: false }),
|
||||
own: containment(ngrams(tokens, N), ownCorpus),
|
||||
};
|
||||
}
|
||||
perItem.push(entry);
|
||||
}
|
||||
}
|
||||
|
||||
console.log(
|
||||
`Flagging needs containment >= ${MIN_CONTAINMENT} against the item's own skill, ` +
|
||||
`or a verbatim span >= ${MAX_SPAN} words.`,
|
||||
);
|
||||
|
||||
// A prompt that quotes the guidance is leakage; an expected_output that does is often just
|
||||
// a correctly-stated answer. Same numbers, different verdict, so they are reported apart.
|
||||
const ROLE_NOTE = {
|
||||
prompt: "LEAKAGE: the question carries its own answer",
|
||||
answer: "expected answer restates the guidance (often legitimate — judge in review)",
|
||||
};
|
||||
|
||||
let flagged = 0;
|
||||
for (const item of perItem) {
|
||||
const own = bySkill.find((s) => s.skillName === item.skillName);
|
||||
for (const [role, m] of Object.entries(item.fields)) {
|
||||
const span = longestVerbatimSpan(m.rawTokens, own.corpusTokensRaw);
|
||||
const bySpan = span >= MAX_SPAN;
|
||||
const byContainment = m.own >= MIN_CONTAINMENT;
|
||||
if (!bySpan && !byContainment) continue;
|
||||
flagged++;
|
||||
const why = bySpan
|
||||
? `verbatim span of ${span} words shared with its own skill`
|
||||
: `${(m.own * 100).toFixed(0)}% containment against ${item.skillName}`;
|
||||
reportFinding(item.file, `case ${item.id} [${role}]: ${why} — ${ROLE_NOTE[role]}`);
|
||||
}
|
||||
}
|
||||
|
||||
// Co-movement: did THIS PR's guidance edit raise an item's overlap with the guidance?
|
||||
let coMoved = 0;
|
||||
if (BASE_REF) {
|
||||
for (const { file, skillName, doc, corpusFiles } of bySkill) {
|
||||
const base = readCorpusAtRef(corpusFiles, BASE_REF);
|
||||
const afterText = readCorpus(corpusFiles);
|
||||
if (!base.anyPresent) {
|
||||
console.log(
|
||||
`Co-movement: skipping ${skillName} — no guidance at base, so this PR adds the ` +
|
||||
`skill. Nothing moved; the static check above covers its items.`,
|
||||
);
|
||||
continue;
|
||||
}
|
||||
const beforeText = base.text;
|
||||
if (beforeText === afterText) continue; // guidance untouched in this PR
|
||||
const beforeGrams = ngrams(tokenize(beforeText), N);
|
||||
const afterGrams = ngrams(tokenize(afterText), N);
|
||||
for (const ev of doc.evals ?? []) {
|
||||
for (const [role, text] of Object.entries(itemFields(ev))) {
|
||||
// No minimum-length skip: containment of an item too short to form an n-gram is
|
||||
// 0, so its delta is 0 — same outcome, one less special case.
|
||||
const grams = ngrams(tokenize(text), N);
|
||||
const before = containment(grams, beforeGrams);
|
||||
const after = containment(grams, afterGrams);
|
||||
const delta = after - before;
|
||||
if (delta < CO_MOVEMENT_MIN_DELTA) continue;
|
||||
coMoved++;
|
||||
reportFinding(
|
||||
file,
|
||||
`CO-MOVEMENT case ${ev.id} [${role}]: this PR's edit to ${skillName}'s ` +
|
||||
`guidance raised containment ${before.toFixed(2)} → ${after.toFixed(2)} ` +
|
||||
`(+${delta.toFixed(2)}). Added guidance text that overlaps an eval item is ` +
|
||||
`teaching to the test, whatever the absolute number is.`,
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
} else {
|
||||
console.log(
|
||||
"Co-movement check skipped (no --base <ref>). It is the check that needs no held-out " +
|
||||
"set, so pass the PR base SHA in CI.",
|
||||
);
|
||||
}
|
||||
} else {
|
||||
console.log(
|
||||
"Co-movement check skipped (no --base <ref>). It is the check that needs no held-out " +
|
||||
"set, so pass the PR base SHA in CI.",
|
||||
);
|
||||
|
||||
if (flagged === 0 && coMoved === 0) {
|
||||
console.log("No echoing items found.");
|
||||
} else {
|
||||
console.log(
|
||||
`\n${flagged} static finding(s), ${coMoved} co-movement finding(s). Not automatically ` +
|
||||
"wrong — some overlap is expected for MongoDB vocabulary — but worth a second look: " +
|
||||
"does the item test whether the agent APPLIES the guidance, or just whether it can " +
|
||||
"quote it?",
|
||||
);
|
||||
}
|
||||
|
||||
process.exit(STRICT && flagged + coMoved > 0 ? 1 : 0);
|
||||
}
|
||||
|
||||
if (flagged === 0 && coMoved === 0) {
|
||||
console.log("No echoing items found.");
|
||||
} else {
|
||||
console.log(
|
||||
`\n${flagged} static finding(s), ${coMoved} co-movement finding(s). Not automatically ` +
|
||||
"wrong — some overlap is expected for MongoDB vocabulary — but worth a second look: " +
|
||||
"does the item test whether the agent APPLIES the guidance, or just whether it can " +
|
||||
"quote it?",
|
||||
);
|
||||
}
|
||||
|
||||
process.exit(STRICT && flagged + coMoved > 0 ? 1 : 0);
|
||||
// Run as a CLI only when invoked directly, not when imported by the test.
|
||||
if (import.meta.url === pathToFileURL(process.argv[1] ?? "").href) main();
|
||||
|
||||
@@ -0,0 +1,108 @@
|
||||
// Pins the tokenizer/span behaviour that makes lint-item-echo.mjs a true port of
|
||||
// agent-skills-evals/inspect/analysis/echo.py. The fixtures and expected values here are
|
||||
// IDENTICAL to test_echo.py's — that is the cross-repo parity mechanism: neither repo's
|
||||
// CI can import the other, so both pin the same numbers.
|
||||
//
|
||||
// Uses node's built-in test runner — no new dependency on the testing/ toolchain.
|
||||
import { test } from "node:test";
|
||||
import assert from "node:assert/strict";
|
||||
|
||||
import { containment, longestVerbatimSpan, ngrams, tokenize } from "./lint-item-echo.mjs";
|
||||
|
||||
// Same fixtures as test_echo.py.
|
||||
const SKILL = `
|
||||
Follow the ESR rule when building a compound index: equality fields first, then sort
|
||||
fields, then range fields. A filter on an exact value with a descending sort should use
|
||||
a compound index whose leading key is the equality field.
|
||||
`;
|
||||
const HONEST_ITEM = `
|
||||
Queries against the events collection that filter by an exact type and sort by
|
||||
timestamp descending are slow. Diagnose the cause and create an appropriate index.
|
||||
`;
|
||||
const COPIED_ITEM = `
|
||||
Follow the ESR rule when building a compound index: equality fields first, then sort
|
||||
fields, then range fields. A filter on an exact value with a descending sort should use
|
||||
a compound index whose leading key is the equality field.
|
||||
`;
|
||||
const OTHER_SKILL_A = `
|
||||
Configure maxPoolSize to match expected concurrency. A serverless function should keep
|
||||
the pool small and reuse the client across invocations rather than reconnecting.
|
||||
`;
|
||||
|
||||
const N = 8; // echo-thresholds.json's n
|
||||
|
||||
function containmentOf(itemText, skillText) {
|
||||
return containment(ngrams(tokenize(itemText), N), ngrams(tokenize(skillText), N));
|
||||
}
|
||||
|
||||
test("tokenize: lowercases and splits", () => {
|
||||
assert.ok(tokenize("ESR rule", { stripDomain: false }).includes("esr"));
|
||||
});
|
||||
|
||||
test("tokenize: strips domain vocabulary by default", () => {
|
||||
const toks = tokenize("create a compound index on the collection");
|
||||
assert.ok(!toks.includes("index") && !toks.includes("collection"));
|
||||
});
|
||||
|
||||
test("tokenize: retains domain words when asked", () => {
|
||||
assert.ok(tokenize("create a compound index", { stripDomain: false }).includes("index"));
|
||||
});
|
||||
|
||||
test("tokenize: operator sigils normalised", () => {
|
||||
assert.ok(tokenize("$match stage", { stripDomain: false }).includes("match"));
|
||||
});
|
||||
|
||||
test("tokenize: hyphenated words stay one token", () => {
|
||||
assert.deepEqual(tokenize("outer-join", { stripDomain: false }), ["outer-join"]);
|
||||
});
|
||||
|
||||
test("tokenize: fenced code blocks are stripped", () => {
|
||||
// code examples are not prose to quote; an item restating one is legitimate reuse
|
||||
assert.deepEqual(tokenize("intro text\n```python\ndb.things.find({})\n```\nafter"), [
|
||||
"intro",
|
||||
"text",
|
||||
"after",
|
||||
]);
|
||||
});
|
||||
|
||||
test("tokenize: empty text", () => {
|
||||
assert.deepEqual(tokenize(""), []);
|
||||
});
|
||||
|
||||
test("containment: identical text is fully contained", () => {
|
||||
assert.equal(containmentOf(COPIED_ITEM, SKILL), 1.0);
|
||||
});
|
||||
|
||||
test("containment: unrelated text is zero", () => {
|
||||
assert.equal(containmentOf(HONEST_ITEM, OTHER_SKILL_A), 0.0);
|
||||
});
|
||||
|
||||
test("containment: honest item scores below a copy", () => {
|
||||
assert.ok(containmentOf(HONEST_ITEM, SKILL) < containmentOf(COPIED_ITEM, SKILL));
|
||||
});
|
||||
|
||||
test("containment: short item cannot be judged", () => {
|
||||
assert.equal(containmentOf("create an index", SKILL), 0.0);
|
||||
});
|
||||
|
||||
test("span: finds a long quotation", () => {
|
||||
assert.ok(
|
||||
longestVerbatimSpan(
|
||||
tokenize(COPIED_ITEM, { stripDomain: false }),
|
||||
tokenize(SKILL, { stripDomain: false }),
|
||||
) > 20,
|
||||
);
|
||||
});
|
||||
|
||||
test("span: short for unrelated text", () => {
|
||||
assert.ok(
|
||||
longestVerbatimSpan(
|
||||
tokenize(HONEST_ITEM, { stripDomain: false }),
|
||||
tokenize(OTHER_SKILL_A, { stripDomain: false }),
|
||||
) < 5,
|
||||
);
|
||||
});
|
||||
|
||||
test("span: zero on empty", () => {
|
||||
assert.equal(longestVerbatimSpan([], tokenize(SKILL, { stripDomain: false })), 0);
|
||||
});
|
||||
Reference in New Issue
Block a user