-
Notifications
You must be signed in to change notification settings - Fork 10
fix(parity): log per-file reasons for native orchestrator drops (#1011) #1024
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,72 @@ | ||
| import { describe, expect, it } from 'vitest'; | ||
| import { classifyNativeDrops, NATIVE_SUPPORTED_EXTENSIONS } from '../../src/domain/parser.js'; | ||
|
|
||
| describe('classifyNativeDrops', () => { | ||
| it('groups WASM-only languages under unsupported-by-native', () => { | ||
| const { byReason, totals } = classifyNativeDrops([ | ||
| 'src/a.fs', | ||
| 'src/b.gleam', | ||
| 'src/c.clj', | ||
| 'src/d.jl', | ||
| 'src/e.R', | ||
| 'src/f.erl', | ||
| 'src/g.sol', | ||
| 'src/h.cu', | ||
| 'src/i.groovy', | ||
| 'src/j.v', | ||
| 'src/k.m', | ||
| ]); | ||
| expect(totals['unsupported-by-native']).toBe(11); | ||
| expect(totals['native-extractor-failure']).toBe(0); | ||
| expect(byReason['unsupported-by-native'].get('.fs')).toEqual(['src/a.fs']); | ||
| expect(byReason['unsupported-by-native'].get('.gleam')).toEqual(['src/b.gleam']); | ||
| expect(byReason['unsupported-by-native'].get('.r')).toEqual(['src/e.R']); | ||
| }); | ||
|
|
||
| it('flags natively-supported extensions as native-extractor-failure', () => { | ||
| const { byReason, totals } = classifyNativeDrops([ | ||
| 'src/a.ts', | ||
| 'src/b.py', | ||
| 'src/c.go', | ||
| 'src/d.rs', | ||
| ]); | ||
| expect(totals['native-extractor-failure']).toBe(4); | ||
| expect(totals['unsupported-by-native']).toBe(0); | ||
| expect(byReason['native-extractor-failure'].get('.ts')).toEqual(['src/a.ts']); | ||
| expect(byReason['native-extractor-failure'].get('.py')).toEqual(['src/b.py']); | ||
| }); | ||
|
|
||
| it('handles a mix of supported and unsupported extensions', () => { | ||
| const { byReason, totals } = classifyNativeDrops([ | ||
| 'src/a.ts', | ||
| 'src/b.fs', | ||
| 'src/c.fs', | ||
| 'src/d.gleam', | ||
| ]); | ||
| expect(totals['native-extractor-failure']).toBe(1); | ||
| expect(totals['unsupported-by-native']).toBe(3); | ||
| expect(byReason['unsupported-by-native'].get('.fs')).toEqual(['src/b.fs', 'src/c.fs']); | ||
| expect(byReason['unsupported-by-native'].get('.gleam')).toEqual(['src/d.gleam']); | ||
| }); | ||
|
|
||
| it('lowercases extensions so .R and .r share a bucket', () => { | ||
| const { byReason, totals } = classifyNativeDrops(['scripts/a.R', 'scripts/b.r']); | ||
| expect(totals['unsupported-by-native']).toBe(2); | ||
| expect(byReason['unsupported-by-native'].get('.r')).toEqual(['scripts/a.R', 'scripts/b.r']); | ||
| }); | ||
|
|
||
| it('returns empty buckets when no files are passed', () => { | ||
| const { byReason, totals } = classifyNativeDrops([]); | ||
| expect(totals['native-extractor-failure']).toBe(0); | ||
| expect(totals['unsupported-by-native']).toBe(0); | ||
| expect(byReason['native-extractor-failure'].size).toBe(0); | ||
| expect(byReason['unsupported-by-native'].size).toBe(0); | ||
| }); | ||
|
|
||
| it('exposes the native-supported extension set for callers', () => { | ||
| expect(NATIVE_SUPPORTED_EXTENSIONS.has('.ts')).toBe(true); | ||
| expect(NATIVE_SUPPORTED_EXTENSIONS.has('.py')).toBe(true); | ||
| expect(NATIVE_SUPPORTED_EXTENSIONS.has('.fs')).toBe(false); | ||
| expect(NATIVE_SUPPORTED_EXTENSIONS.has('.gleam')).toBe(false); | ||
| }); | ||
| }); | ||
|
Comment on lines
+4
to
+83
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
The non-trivial formatting function (
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed in cbbc9ae — moved |
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
NATIVE_SUPPORTED_EXTENSIONSis keyed to one specific snapshot ofLanguageKind::from_extension. If the Rust addon gains a new language (or drops one) between addon releases without a matching JS update, drops will be silently mis-classified: a real native failure shows up asunsupported-by-native(info, quiet) instead ofnative-extractor-failure(warn, loud). The inverse case — removed support — would spam falsenative-extractor-failurewarnings. There's no runtime assertion that the two lists agree, so the drift won't be caught until a user notices wrong log levels. Consider adding a CI step or a startup assertion that cross-checks the set against the native addon's own exported metadata if the addon exposes it; if it doesn't, at minimum add an integration test that verifies the current addon version is the one this set was generated from.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in cbbc9ae — added a drift guard test that parses
crates/codegraph-core/src/parser_registry.rsand assertsNATIVE_SUPPORTED_EXTENSIONSagrees with the RustLanguageKind::from_extensionarms. The native addon doesn't expose its own metadata, so source-level cross-check at CI time is the cheapest way to catch drift before users see mis-classified log levels. Ifparser_registry.rsadds or removes an extension, the test fails loudly with a list of mismatches.