test(parsers): gate LANGUAGE_REGISTRY ↔ NATIVE_SUPPORTED_EXTENSIONS parity#1154
test(parsers): gate LANGUAGE_REGISTRY ↔ NATIVE_SUPPORTED_EXTENSIONS parity#1154carlos-alm wants to merge 3 commits into
Conversation
…arity Closes #1121. The drift guard between NATIVE_SUPPORTED_EXTENSIONS and parser_registry.rs covered link 2 ↔ 3, but link 1 ↔ 2 (LANGUAGE_REGISTRY ↔ NATIVE_SUPPORTED_EXTENSIONS) had no test. A WASM-only language added to the registry would silently degrade the native engine without flagging. Adds three tests with an explicit WASM_ONLY_ALLOWLIST (currently empty) so the allowlist itself can't rot — entries must reference a real registry extension and must not duplicate a language that's already been ported.
|
Claude finished @carlos-alm's task —— View job PR Review - Final AssessmentReviewing PR #1154: test(parsers): gate LANGUAGE_REGISTRY ↔ NATIVE_SUPPORTED_EXTENSIONS parity Todo List
Phase 1: Root Cause Analysis ✅Problem Being Solved: Real issue from #1121 - there was a gap in drift detection between the JS Why This Approach: Justified architectural fix. The existing drift guard only covered Risk Assessment: Low risk - pure additive testing with well-defined allowlist mechanism for intentional exceptions. Phase 2: Configuration Change Detection ✅HIGH-RISK Changes: None detected. This PR only adds tests - no linting rules disabled, no timeouts increased, no build configuration modified, no TypeScript strict mode changes. Assessment: Clean - purely additive test coverage with no configuration relaxation. Phase 3: Test Integrity ✅RED FLAGS: None detected. The tests are properly structured and strict: ✅ Strong assertions: Tests use Test Quality Assessment:
All tests are properly isolated and test exactly what they claim to test. Phase 4: Code Quality ✅Implementation Quality: Excellent ✅ Clean TypeScript: Proper typing with Minor Code Quality Notes:
Security/Performance: No concerns - pure test code with no external dependencies or side effects. Phase 5: Backlog Compliance ✅Zero-dep: ✓ — No new runtime dependencies added. Uses existing test infrastructure (vitest, existing imports) Foundation-aligned: ✓ — Fully aligned. Strengthens the dual-engine architecture (P1: always-current) by ensuring native engine coverage matches the JS registry. No cloud APIs, external DBs, or agent behavior. Problem-fit: 5/5 — Directly prevents AI agent confusion from inconsistent engine results. When the native engine silently degrades due to missing extractors, agents receive inconsistent dependency graphs depending on engine selection, leading to hallucinated dependencies and incorrect blast radius analysis. This gate prevents exactly that scenario. Breaking: No — Purely additive test coverage with no behavior changes to public APIs, CLI output, or database schema. Tier: 1 (Zero-dep ✓ + Foundation-aligned ✓) Critical ConcernsNone. This is a high-quality defensive improvement that strengthens the core architecture. Final RecommendationRoot Cause AnalysisProblem Being Solved: CI gap allowing silent native engine degradation via WASM-only language additions Backlog Compliance
Critical ConcernsNone Final Recommendation
|
Greptile SummaryAdds a new
Confidence Score: 5/5Test-only change adding three new CI gates; no production code is modified and all three tests pass today with an empty allowlist. The change is purely additive test code that closes a documented gap. The new gates correctly catch forward drift and the normalisation fixes from the prior round are in place for tests 2 and 3. One minor asymmetry remains in test 1's allowlist lookup, but it would only produce a false positive rather than silently missing a regression. No files require special attention; the single changed file is low-risk test code. Important Files Changed
Reviews (4): Last reviewed commit: "Merge branch 'main' into feat/1121-langu..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| it('WASM_ONLY_ALLOWLIST does not list extensions that already have a Rust extractor', () => { | ||
| // Catches stale allowlist entries: once a language is ported to Rust the | ||
| // allowlist line should be deleted, not left behind as dead config. | ||
| const stale = [...WASM_ONLY_ALLOWLIST].filter((ext) => NATIVE_SUPPORTED_EXTENSIONS.has(ext)); |
There was a problem hiding this comment.
registryExts (used in test 1 and test 3) and NATIVE_SUPPORTED_EXTENSIONS are all lowercase. If a future contributor adds a mixed-case entry such as .TS to the allowlist, test 2 would silently miss it as stale (NATIVE_SUPPORTED_EXTENSIONS.has('.TS') → false) and test 3 would wrongly report it as an orphan (registryExts.has('.TS') → false because registryExts was built with .toLowerCase()). Normalising allowlist entries to lowercase in these two filters keeps all three tests consistent.
| const stale = [...WASM_ONLY_ALLOWLIST].filter((ext) => NATIVE_SUPPORTED_EXTENSIONS.has(ext)); | |
| const stale = [...WASM_ONLY_ALLOWLIST] | |
| .map((ext) => ext.toLowerCase()) | |
| .filter((ext) => NATIVE_SUPPORTED_EXTENSIONS.has(ext)); |
There was a problem hiding this comment.
Fixed in 57eb769 — added .map((ext) => ext.toLowerCase()) before the NATIVE_SUPPORTED_EXTENSIONS filter so a mixed-case allowlist entry can't slip past the staleness check.
| registryExts.add(ext.toLowerCase()); | ||
| } | ||
| } | ||
| const orphans = [...WASM_ONLY_ALLOWLIST].filter((ext) => !registryExts.has(ext)); |
There was a problem hiding this comment.
.toLowerCase() here, a capitalised allowlist entry would be falsely reported as an orphan even though a matching (lowercase) extension exists in registryExts.
| const orphans = [...WASM_ONLY_ALLOWLIST].filter((ext) => !registryExts.has(ext)); | |
| const orphans = [...WASM_ONLY_ALLOWLIST] | |
| .map((ext) => ext.toLowerCase()) | |
| .filter((ext) => !registryExts.has(ext)); |
There was a problem hiding this comment.
Fixed in 57eb769 — added .map((ext) => ext.toLowerCase()) before the registryExts filter so the orphan check matches the lowercase form actually stored in registryExts.
Lowercase allowlist entries before comparing against `registryExts` and `NATIVE_SUPPORTED_EXTENSIONS` so a mixed-case future entry can't slip past the staleness and orphan checks. Both reference sets are already lowercased, so without this normalization a capitalized allowlist entry would be silently skipped by test 2 and falsely flagged by test 3.
1 participant
Summary
Closes #1121.
The acceptance criterion from #1071 called for a CI gate that fires when a JS
LANGUAGE_REGISTRYentry has no corresponding Rust extractor (or sits on an explicit allowlist). The existing drift guard intests/parsers/native-drop-classification.test.tscoversNATIVE_SUPPORTED_EXTENSIONS ↔ parser_registry.rs(link 2 ↔ 3), but the link fromLANGUAGE_REGISTRYtoNATIVE_SUPPORTED_EXTENSIONS(link 1 ↔ 2) had no test. A WASM-only language added to the registry would silently be classified asunsupported-by-nativeand only the JS backfill would parse it — exactly the regression class #1071 was meant to prevent.Adds a new
describeblock (LANGUAGE_REGISTRY ↔ NATIVE_SUPPORTED_EXTENSIONS parity) with three tests:LANGUAGE_REGISTRYextension has a Rust extractor (or appears on the documentedWASM_ONLY_ALLOWLIST).LANGUAGE_REGISTRY(catches typos and dead config).WASM_ONLY_ALLOWLISTis an emptySet<string>today, documented in-code so future contributors know when to add to it (intentional WASM-only languages with a tracked Rust port) vs. when to port directly.Test plan
npx vitest run tests/parsers/native-drop-classification.test.ts— 16/16 passing (13 existing + 3 new)..toyto a registry copy and confirmed the detector flagged it) before removing the throwaway test.npx biome check tests/parsers/native-drop-classification.test.ts— clean.