Skip to content

Commit cd9b136

Browse files
authored
refactor(signals): consolidate isTestFile/isCodeFile into gittensory-engine (#4144)
isTestFile/isCodeFile were hand-ported three times (canonical in src/signals/test-evidence.ts + path-matchers.ts, plus independent copies in packages/gittensory-mcp/lib/local-branch.js and packages/gittensory-mcp/scripts/gittensor-score-preview.mjs) -- commit history has multiple "re-sync isTestFile with the server" fixes for exactly this drift. A fourth, already-half-done copy was sitting unused in packages/gittensory-engine/src/signals/test-evidence.ts (orphaned from #3882, stale relative to the root file, never wired into the package's exports or imported anywhere). Makes the engine copy the actual up-to-date canonical source (adding the extended isCodeFile alongside isTestPath/isSourcePath), and turns src/signals/test-evidence.ts into a thin re-export shim -- same pattern already used for scoring/preview.ts and focus-manifest.ts. path-matchers.ts now delegates isCodeFile to the same source instead of composing its own copy. Every existing call site keeps importing from the same paths. The two packages/gittensory-mcp hand-ports aren't touched yet -- they can't depend on @jsonbored/gittensory-engine until it's actually published to npm (it currently only resolves via the workspace symlink), which needs a one-time manual bootstrap outside CI. Follow-up once that's done.
1 parent 97f087b commit cd9b136

4 files changed

Lines changed: 170 additions & 185 deletions

File tree

packages/gittensory-engine/package.json

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,10 @@
4646
"./scoring/pending-pr-scenarios": {
4747
"types": "./dist/scoring/pending-pr-scenarios.d.ts",
4848
"default": "./dist/scoring/pending-pr-scenarios.js"
49+
},
50+
"./signals/test-evidence": {
51+
"types": "./dist/signals/test-evidence.d.ts",
52+
"default": "./dist/signals/test-evidence.js"
4953
}
5054
},
5155
"files": [

packages/gittensory-engine/src/signals/test-evidence.ts

Lines changed: 152 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,26 +2,95 @@ export function isTestPath(file: string): boolean {
22
return (
33
/(^|\/)(test|tests|spec|__tests__)\//i.test(file) ||
44
/(^|\/)src\/test\//i.test(file) ||
5-
/(^|\/)[^/]+_test\.(go|py|rb|dart)$/i.test(file) ||
6-
/(^|\/)test_[^/]*\.py$/i.test(file) ||
5+
/(^|\/)[^/]+_test\.(go|py|rb|dart)$/i.test(file) || // Dart/Flutter `foo_test.dart` co-located with source
6+
/(^|\/)test_[^/]*\.py$/i.test(file) || // pytest's default `test_*.py` prefix convention (the suffix rule above only catches `*_test.py`)
77
/(^|\/)[^/]+_spec\.rb$/i.test(file) ||
88
/\.(test|spec)\.(ts|tsx|mts|cts|js|jsx|mjs|cjs|py|rb|rs)$/i.test(file) ||
99
/(^|\/)[^/]+\.(cy|e2e)\.(ts|tsx|mts|cts|js|jsx|mjs|cjs)$/i.test(file) ||
10+
// JVM / C# / Swift / PHP `SomethingTest(s)`/`SomethingSpec` class-suffix convention
11+
// (JUnit, Kotlin/ScalaTest, Spock, xUnit/NUnit, XCTest, PHPUnit/PHPSpec). Case-sensitive on the
12+
// PascalCase suffix so it can't false-positive on words that merely end in
13+
// "test"/"spec" (Latest.java, Contest.cs, manifest.scala, Latest.php).
1014
/(^|\/)\w*(Tests?|Spec)\.(java|kt|kts|scala|cs|swift|groovy|php)$/.test(file) ||
1115
/(^|\/)__snapshots__\//i.test(file)
1216
);
1317
}
1418

19+
// Canonical hand-authored-source extensions — the SOURCE-side sibling of isTestPath's class-suffix rule.
20+
// The two matchers MUST stay symmetric: isTestPath recognizes java/kt/kts/scala/cs/swift/groovy test files,
21+
// so this set lists those same languages. Otherwise a C#/Swift/Groovy/Kotlin-script SOURCE change is classified
22+
// as neither code nor test and silently escapes both the missing-tests gate signals and token scoring.
23+
const SOURCE_FILE_EXTENSION = /\.(ts|tsx|mts|cts|js|jsx|mjs|cjs|py|rb|rs|kt|kts|scala|java|cs|swift|groovy|go|sql)$/i;
24+
25+
/** True iff `file` is a hand-authored program-source file: a recognized source extension that is not itself a
26+
* test file. The single source of truth for every `isCodeFile` in the signals layer, so the source/test
27+
* classifiers can never drift (the same way isCodeFile's `isTestFile` wrappers all delegate to isTestPath). */
28+
export function isSourcePath(file: string): boolean {
29+
return SOURCE_FILE_EXTENSION.test(file) && !isTestPath(file);
30+
}
31+
32+
// Extensions recognized as code outside isSourcePath's core set (php, native, front-end frameworks, Dart).
33+
// isSourcePath owns the JVM/.NET/Swift/Groovy/Kotlin-script set symmetric with isTestPath.
34+
const EXTENDED_SOURCE_EXTENSION = /\.(php|cpp|cc|c|h|hpp|m|vue|svelte|astro|dart)$/i;
35+
36+
/** cs/swift/groovy/kts plus php, C/C++/Objective-C, vue/svelte/astro, and dart — see isSourcePath for the
37+
* canonical JVM/.NET/Swift/Groovy/Kotlin-script matcher kept symmetric with isTestPath. Generated Dart part
38+
* files (.g.dart/.freezed.dart/.gr.dart) stay non-code (#3724). The single source of truth both the Worker
39+
* (src/signals/path-matchers.ts) and the published @jsonbored/gittensory-mcp/gittensory-miner CLIs delegate
40+
* to, so the three previously-independent hand-ports can't silently drift from each other again. */
41+
export function isCodeFile(file: string): boolean {
42+
if (isSourcePath(file)) return true;
43+
return EXTENDED_SOURCE_EXTENSION.test(file) && !isTestPath(file) && !/\.(g|freezed|gr)\.dart$/i.test(file);
44+
}
45+
46+
export function hasLocalTestEvidence(input: { tests?: string[] | undefined; testFiles?: string[] | undefined }): boolean {
47+
return (input.tests ?? []).length > 0 || (input.testFiles ?? []).some((file) => isTestPath(file));
48+
}
49+
50+
// A body can mention testing without having actually done it ("No tests run", "Tests not run", "Not
51+
// tested locally", "did not run any tests") -- the affirmative keyword match below would otherwise treat
52+
// that as passing evidence and let a configured manifest test expectation silently disappear. Rather than
53+
// enumerate ever more literal phrase templates (which a previous version of this function tried, and which
54+
// still missed "Not tested" because its test-noun list didn't include the verb form "tested"), detect
55+
// negation by PROXIMITY: a negation word within a few words of a test/validation stem, in either order,
56+
// with a shared stem definition so the "is this a test/validation mention at all" question is answered
57+
// exactly once. The filler between the negation word and the stem may not cross a clause/sentence boundary
58+
// (a comma, period, exclamation mark, or question mark), so an unrelated "not" earlier in the body (e.g.
59+
// "This is not a breaking change. Tested with npm run test:ci.") cannot suppress a later, unrelated
60+
// affirmative note. A colon, semicolon, or dash is deliberately NOT a hard boundary here -- see
61+
// LABEL_SEPARATOR_GAP below.
1562
const TEST_STEM = "(?:test(?:ed|s|ing)?|validat(?:ion|ed)|verif(?:y|ied|ying)|manual check|smoke(?:\\s+tests?)?)";
1663
const NEGATION_WORD = "(?:no|not|never|without|skip(?:ped)?|didn't|doesn't|isn't|wasn't|weren't|haven't|hasn't)";
1764
const NEGATION_CONTINUATION = "(?:not|never|failed|failing|skipped|incomplete)";
1865
const SAME_SENTENCE_FILLER_WORD = "[^\\s.,!?;]+";
66+
// A label-style status line often glues its separator directly onto the negation word or stem with no
67+
// surrounding whitespace ("Tests: not run.", "Validation; skipped.", "Tests - not run."). The plain
68+
// `\s+` gap below would never match across that punctuation, so the negation went undetected and the
69+
// bare "Tests"/"Validation" keyword fell through to the affirmative check instead (#3304, round 4).
70+
// Allow ONE label separator (colon, semicolon, or a hyphen/en-dash/em-dash) with any trailing
71+
// whitespace to stand in for the mandatory whitespace, but only at the junction touching the negation
72+
// word or stem itself -- every other gap between filler words stays pure whitespace, so a label
73+
// separator elsewhere in the sentence still cannot let a negation reach across unrelated content (the
74+
// filler-word bound below already exists for exactly this reason).
1975
const LABEL_SEPARATOR_GAP = "(?:\\s+|[:;\\-\\u2013\\u2014]\\s*)";
76+
2077
const NEGATES_BEFORE_TEST_STEM = new RegExp(`\\b${NEGATION_WORD}\\b${LABEL_SEPARATOR_GAP}(?:${SAME_SENTENCE_FILLER_WORD}\\s+){0,3}${TEST_STEM}\\b`, "i");
2178
const NEGATES_AFTER_TEST_STEM = new RegExp(`\\b${TEST_STEM}\\b${LABEL_SEPARATOR_GAP}(?:${SAME_SENTENCE_FILLER_WORD}\\s+){0,2}${NEGATION_CONTINUATION}\\b`, "i");
79+
// A compound negated adjective with no separating whitespace at all ("untested", "unvalidated", "unverified").
2280
const NEGATES_TEST_STEM_PREFIX = /\bun(?:tested|validated|verified)\b/i;
81+
2382
const AFFIRMATIVE_TEST_MENTION = /\b(test(?:ed|s|ing)?|validation|validated|verified|manual check|smoke|pytest|vitest|npm test|pnpm test|cargo test|go test)\b/i;
2483

84+
// A body can contain BOTH a genuine negated clause ("No tests run locally.") and a separate, later clause
85+
// with real affirmative evidence ("Validated with npm run test:ci.") -- evaluating the negation checks
86+
// against the WHOLE body would let the first clause veto the second, discarding real evidence the manifest
87+
// gate is specifically trying to detect (#3304, round 3). Split on the same clause-boundary punctuation the
88+
// proximity checks already treat as a hard stop -- colon/semicolon/dash are excluded here on purpose
89+
// (#3304, round 4): they are typically a label separator glued directly onto the word on either side
90+
// ("Tests: not run."), and splitting on them would sever the stem from its own negation before the
91+
// proximity checks ever run, the same way the round-3 bug worked one level up. Require at least one
92+
// clause to be an affirmative, non-negated mention -- so an earlier honest "no tests" disclosure can no
93+
// longer suppress later evidence.
2594
export function hasValidationNote(value: string): boolean {
2695
return value
2796
.split(/[.,!?]+/)
@@ -33,3 +102,84 @@ export function hasValidationNote(value: string): boolean {
33102
AFFIRMATIVE_TEST_MENTION.test(clause),
34103
);
35104
}
105+
106+
/**
107+
* Coarse classification of how much test coverage accompanies a set of changed paths.
108+
* Used by slop signals to weight diffs that touch source but include no tests differently
109+
* from those with proportionally strong test changes.
110+
*/
111+
export type TestCoverageClassification = "strong" | "adequate" | "weak" | "absent";
112+
113+
export function classifyTestCoverage(changedPaths: string[]): TestCoverageClassification {
114+
if (changedPaths.length === 0) return "absent";
115+
const testCount = changedPaths.filter(isTestPath).length;
116+
if (testCount === 0) return "absent";
117+
const ratio = testCount / changedPaths.length;
118+
if (ratio >= 0.4) return "strong";
119+
if (ratio >= 0.2) return "adequate";
120+
return "weak";
121+
}
122+
123+
// #2187 (foundational slice of #1972 — boundary-safe test generation): a small, precise framework list, each
124+
// tied to an unambiguous marker file/pattern and an existing isTestPath naming family. Deliberately narrow —
125+
// a longer list of guessable-but-ambiguous frameworks would make detectTestConvention's output less trustworthy
126+
// as a test-gen input than returning null (see the "unknown => null" fail-safe below).
127+
export const TEST_FRAMEWORKS = ["vitest", "jest", "pytest", "go-test", "rspec", "cargo-test"] as const;
128+
export type TestFramework = (typeof TEST_FRAMEWORKS)[number];
129+
130+
/** Deterministic detection result: which framework, where tests live, and the file-naming convention to
131+
* follow when scaffolding a new one. `testDir` is `null` for a co-located convention (e.g. Go/Rust/Dart's
132+
* `_test`/`#[cfg(test)]` siblings), matching how those ecosystems actually lay out tests. */
133+
export type TestConvention = {
134+
framework: TestFramework;
135+
testDir: string | null;
136+
namingPattern: string;
137+
};
138+
139+
// One marker file per framework, checked against the basename of each changed/known path. Ordered by
140+
// specificity where two frameworks could share an ecosystem (vitest before jest: a repo migrating from Jest to
141+
// Vitest keeps `jest.config.js` around far more often than the reverse, so vitest's own config marker — when
142+
// present — must win).
143+
const FRAMEWORK_MARKERS: ReadonlyArray<{ framework: TestFramework; pattern: RegExp; testDir: string | null; namingPattern: string }> = [
144+
{ framework: "vitest", pattern: /(^|\/)vitest\.config\.(ts|mts|cts|js|mjs|cjs)$/i, testDir: "test/", namingPattern: "*.test.ts" },
145+
{ framework: "vitest", pattern: /(^|\/)vitest\.workspace\.(ts|mts|cts|js|mjs|cjs)$/i, testDir: "test/", namingPattern: "*.test.ts" },
146+
{ framework: "jest", pattern: /(^|\/)jest\.config\.(ts|js|mjs|cjs|json)$/i, testDir: "__tests__/", namingPattern: "*.test.js" },
147+
{ framework: "pytest", pattern: /(^|\/)pytest\.ini$/i, testDir: null, namingPattern: "test_*.py" },
148+
{ framework: "pytest", pattern: /(^|\/)pyproject\.toml$/i, testDir: null, namingPattern: "test_*.py" },
149+
{ framework: "go-test", pattern: /(^|\/)go\.mod$/i, testDir: null, namingPattern: "*_test.go" },
150+
{ framework: "rspec", pattern: /(^|\/)\.rspec$/i, testDir: "spec/", namingPattern: "*_spec.rb" },
151+
{ framework: "cargo-test", pattern: /(^|\/)Cargo\.toml$/i, testDir: null, namingPattern: "#[cfg(test)] mod tests" },
152+
];
153+
154+
// Fallback inference from an EXISTING test file's own naming, when no marker is present (e.g. a marker file
155+
// wasn't part of the changed/known set passed in, but the repo already has real test files to imitate).
156+
const CONVENTION_FROM_EXISTING_TEST: ReadonlyArray<{ framework: TestFramework; pattern: RegExp; testDir: string | null; namingPattern: string }> = [
157+
{ framework: "vitest", pattern: /\.(test|spec)\.(ts|tsx|mts|cts)$/i, testDir: "test/", namingPattern: "*.test.ts" },
158+
{ framework: "jest", pattern: /\.(test|spec)\.(js|jsx|mjs|cjs)$/i, testDir: "__tests__/", namingPattern: "*.test.js" },
159+
{ framework: "pytest", pattern: /(^|\/)test_[^/]*\.py$|[^/]+_test\.py$/i, testDir: null, namingPattern: "test_*.py" },
160+
{ framework: "go-test", pattern: /[^/]+_test\.go$/i, testDir: null, namingPattern: "*_test.go" },
161+
{ framework: "rspec", pattern: /[^/]+_spec\.rb$/i, testDir: "spec/", namingPattern: "*_spec.rb" },
162+
];
163+
164+
/**
165+
* Detect a repo's test framework + convention from a bounded set of changed paths and known marker filenames
166+
* (e.g. `package.json`, `pyproject.toml`, `go.mod` — paths the caller already has, never fetched by this
167+
* function). Deterministic and pure: markers win over inferring from existing test-file naming (a config file
168+
* is a stronger, unambiguous signal than a naming guess), and the marker list is checked in a fixed order so a
169+
* repo with more than one marker present always resolves to the same framework. Returns `null` when nothing in
170+
* `paths`/`markers` matches any known convention — an unrecognized layout is left alone (fail-safe) rather than
171+
* guessing, since a wrong framework guess would make the downstream test-gen spec actively misleading.
172+
*/
173+
export function detectTestConvention(paths: string[], markers: string[]): TestConvention | null {
174+
for (const marker of FRAMEWORK_MARKERS) {
175+
if (markers.some((path) => marker.pattern.test(path)) || paths.some((path) => marker.pattern.test(path))) {
176+
return { framework: marker.framework, testDir: marker.testDir, namingPattern: marker.namingPattern };
177+
}
178+
}
179+
for (const convention of CONVENTION_FROM_EXISTING_TEST) {
180+
if (paths.some((path) => isTestPath(path) && convention.pattern.test(path))) {
181+
return { framework: convention.framework, testDir: convention.testDir, namingPattern: convention.namingPattern };
182+
}
183+
}
184+
return null;
185+
}

src/signals/path-matchers.ts

Lines changed: 5 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { isSourcePath, isTestPath } from "./test-evidence";
1+
import { isCodeFile, isTestPath } from "./test-evidence";
22

33
// Pure, deterministic path matchers for slop classification (#561). Siblings to `isTestFile` /
44
// `isTestPath`: they identify changed files that are NOT genuine hand-authored effort — machine-
@@ -15,17 +15,10 @@ export function isTestFile(file: string): boolean {
1515
return isTestPath(file);
1616
}
1717

18-
// Extensions recognized as code outside test-evidence's isSourcePath core set (php, native, front-end
19-
// frameworks, Dart). isSourcePath owns the JVM/.NET/Swift/Groovy/Kotlin-script set symmetric with isTestPath.
20-
const EXTENDED_SOURCE_EXTENSION = /\.(php|cpp|cc|c|h|hpp|m|vue|svelte|astro|dart)$/i;
21-
22-
/** cs/swift/groovy/kts plus php, C/C++/Objective-C, vue/svelte/astro, and dart — see isSourcePath for the
23-
* canonical JVM/.NET/Swift/Groovy/Kotlin-script matcher kept symmetric with isTestPath. Generated Dart part
24-
* files (.g.dart/.freezed.dart/.gr.dart) stay non-code (#3724). */
25-
export function isCodeFile(file: string): boolean {
26-
if (isSourcePath(file)) return true;
27-
return EXTENDED_SOURCE_EXTENSION.test(file) && !isTestFile(file) && !/\.(g|freezed|gr)\.dart$/i.test(file);
28-
}
18+
// isCodeFile is the single source of truth the published gittensory-mcp/gittensory-miner CLIs also
19+
// depend on (via @jsonbored/gittensory-engine) — defined once in test-evidence.ts alongside the
20+
// isSourcePath/isTestPath pair it composes, re-exported here so this file's existing callers don't change.
21+
export { isCodeFile };
2922

3023
function normalize(path: string): string {
3124
return String(path ?? "")

0 commit comments

Comments
 (0)