Skip to content

fix(test): restore strict parity assertions and add CI parity job #916

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.

Sign up for GitHub

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

Merged
carlos-alm merged 3 commits into from
Apr 13, 2026
Merged

fix(test): restore strict parity assertions and add CI parity job #916

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
1f6ebab
fix(test): restore strict parity assertions and add CI parity job
carlos-alm Apr 11, 2026
2f9f450
fix: align native addon verification with musl detection and prevent …
carlos-alm Apr 12, 2026
f6172ef
fix(test): apply CODEGRAPH_PARITY guard to tests/engines/ parity file…
carlos-alm Apr 13, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
72 changes: 71 additions & 1 deletion .github/workflows/ci.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,76 @@ jobs:
STRIP_FLAG=$(node -e "const [M]=process.versions.node.split('.').map(Number); console.log(M>=23?'--strip-types':'--experimental-strip-types')")
node $STRIP_FLAG scripts/verify-imports.ts

parity:
strategy:
fail-fast: false
matrix:
os: [ubuntu-latest, macos-latest, windows-latest]

runs-on: ${{ matrix.os }}
name: Engine parity (${{ matrix.os }})

steps:
- uses: actions/checkout@v6

- name: Setup Node.js
uses: actions/setup-node@v6
with:
node-version: 22

- name: Install dependencies
shell: bash
run: |
for attempt in 1 2 3; do
npm install && break
if [ "$attempt" -lt 3 ]; then
echo "::warning::npm install attempt $attempt failed, retrying in 15s..."
sleep 15
else
echo "::error::npm install failed after 3 attempts"
exit 1
fi
done

- name: Verify native addon is available
shell: bash
run: |
node -e "
const { createRequire } = require('node:module');
const r = createRequire(require.resolve('./package.json'));
const os = require('os');
const fs = require('fs');
const plat = os.platform();
const arch = os.arch();
let libc = '';
if (plat === 'linux') {
try {
const files = fs.readdirSync('/lib');
libc = files.some(f => f.startsWith('ld-musl-') && f.endsWith('.so.1')) ? 'musl' : 'gnu';
} catch { libc = 'gnu'; }
}
const pkgs = {
'linux-x64-gnu': '@optave/codegraph-linux-x64-gnu',
'linux-x64-musl': '@optave/codegraph-linux-x64-musl',
'linux-arm64-gnu': '@optave/codegraph-linux-arm64-gnu',
'linux-arm64-musl': '@optave/codegraph-linux-arm64-musl',
'darwin-arm64': '@optave/codegraph-darwin-arm64',
'darwin-x64': '@optave/codegraph-darwin-x64',
'win32-x64': '@optave/codegraph-win32-x64-msvc',
};
const key = libc ? plat + '-' + arch + '-' + libc : plat + '-' + arch;
const pkg = pkgs[key];
if (!pkg) { console.error('No native package for ' + key); process.exit(1); }
try { r(pkg); console.log('Native addon loaded: ' + pkg); }
catch (e) { console.error('Failed to load ' + pkg + ': ' + e.message); process.exit(1); }
Comment on lines +185 to +198
Copy link
Copy Markdown
Contributor

@greptile-apps greptile-apps bot Apr 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Linux musl platform not handled — diverges from isNativeAvailable()

The verification step hardcodes linux-x64 → @optave/codegraph-linux-x64-gnu, but native.ts's resolvePlatformPackage() calls detectLibc() and resolves to linux-x64-musl on a musl host. On such a runner the step would succeed (gnu package loads), then isNativeAvailable() would try the musl package (not installed), return false, and the tests would silently skip — giving false confidence.

The fix is to mirror the musl detection in the verification script, or simply invoke isNativeAvailable() directly via a small Node script that imports src/infrastructure/native.js:

// Verify by calling the same path the test will use
node --experimental-strip-types -e "
  import { isNativeAvailable } from './src/infrastructure/native.js';
  if (!isNativeAvailable()) { process.stderr.write('Native addon unavailable\n'); process.exit(1); }
  console.log('Native addon verified via isNativeAvailable()');
"

(Adjust the strip-types flag the same way verify-imports does.)

Copy link
Copy Markdown
Contributor Author

@carlos-alm carlos-alm Apr 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 2f9f450 — the verification script now mirrors the detectLibc() logic from native.ts: on Linux, it reads /lib for ld-musl-* entries and resolves to the correct -gnu or -musl package. Also added linux-arm64-gnu and linux-arm64-musl entries for completeness.

The key-construction now uses plat-arch-libc on Linux (matching resolvePlatformPackage()) and plat-arch on other platforms.

"

- name: Run parity tests
shell: bash
env:
CODEGRAPH_PARITY: '1'
run: npx vitest run tests/engines/ tests/integration/build-parity.test.ts --reporter=verbose

rust-check:
runs-on: ubuntu-latest
name: Rust compile check
Expand All @@ -154,7 +224,7 @@ jobs:

ci-pipeline:
if: always()
needs: [lint, test, typecheck, audit, verify-imports, rust-check]
needs: [lint, test, typecheck, audit, verify-imports, rust-check, parity]
Copy link
Copy Markdown
Contributor

@greptile-apps greptile-apps bot Apr 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Known-failing job blocks the entire CI gate

The PR description explicitly states: "The parity job will fail until a new native binary is published from current source." Adding this job to ci-pipeline's needs array means the required gate will fail until the binary ships, blocking every subsequent PR from showing green CI.

Either coordinate the binary publish to happen before this PR merges, or use continue-on-error: true on the parity job temporarily (and remove it once the binary is live):

  parity:
    strategy:
      fail-fast: false
      matrix:
        os: [ubuntu-latest, macos-latest, windows-latest]
    continue-on-error: true   # remove once updated binary is published
    runs-on: ${{ matrix.os }}

Copy link
Copy Markdown
Contributor Author

@carlos-alm carlos-alm Apr 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The parity job is now passing on all three platforms (ubuntu, macos, windows) — the native binary was published before CI ran. The continue-on-error workaround is not needed since the gate is green.

That said, if this becomes an issue in the future (e.g. binary lags behind a source change), the coordinated approach is to publish the binary first. Adding continue-on-error would defeat the purpose of making parity a hard gate.

runs-on: ubuntu-latest
name: CI Testing Pipeline
steps:
Expand Down
7 changes: 6 additions & 1 deletion tests/engines/ast-parity.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,12 @@ function processItems(items: string[]): void {
}
`;

describe('AST node parity (native vs WASM)', () => {
// In the dedicated parity CI job (CODEGRAPH_PARITY=1), never silently skip —
// fail hard so a missing native addon is immediately visible.
const requireParity = !!process.env.CODEGRAPH_PARITY;
const describeOrSkip = requireParity || isNativeAvailable() ? describe : describe.skip;

describeOrSkip('AST node parity (native vs WASM)', () => {
beforeAll(async () => {
if (!isNativeAvailable()) return;
native = getNative();
Expand Down
5 changes: 4 additions & 1 deletion tests/engines/dataflow-parity.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,10 @@ function detectNativeDataflow() {
return !!r?.dataflow;
}

const describeOrSkip = hasNative ? describe : describe.skip;
// In the dedicated parity CI job (CODEGRAPH_PARITY=1), never silently skip —
// fail hard so a missing native addon is immediately visible.
const requireParity = !!process.env.CODEGRAPH_PARITY;
const describeOrSkip = requireParity || hasNative ? describe : describe.skip;

describeOrSkip('Cross-engine dataflow parity', () => {
beforeAll(async () => {
Expand Down
6 changes: 4 additions & 2 deletions tests/engines/parity.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -110,8 +110,10 @@ function normalize(symbols) {
}

const hasNative = isNativeAvailable();

const describeOrSkip = hasNative ? describe : describe.skip;
// In the dedicated parity CI job (CODEGRAPH_PARITY=1), never silently skip —
// fail hard so a missing native addon is immediately visible.
const requireParity = !!process.env.CODEGRAPH_PARITY;
const describeOrSkip = requireParity || hasNative ? describe : describe.skip;

describeOrSkip('Cross-engine parity', () => {
beforeAll(async () => {
Expand Down
66 changes: 7 additions & 59 deletions tests/integration/build-parity.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,10 @@ import { isNativeAvailable } from '../../src/infrastructure/native.js';
const FIXTURE_DIR = path.join(import.meta.dirname, '..', 'fixtures', 'sample-project');

const hasNative = isNativeAvailable();
const describeOrSkip = hasNative ? describe : describe.skip;
// In the dedicated parity CI job (CODEGRAPH_PARITY=1), never silently skip —
// fail hard so a missing native addon is immediately visible.
const requireParity = !!process.env.CODEGRAPH_PARITY;
const describeOrSkip = requireParity || hasNative ? describe : describe.skip;

function copyDirSync(src, dest) {
fs.mkdirSync(dest, { recursive: true });
Expand Down Expand Up @@ -102,73 +105,18 @@ describeOrSkip('Build parity: native vs WASM', () => {
it('produces identical edges', () => {
const wasmGraph = readGraph(path.join(wasmDir, '.codegraph', 'graph.db'));
const nativeGraph = readGraph(path.join(nativeDir, '.codegraph', 'graph.db'));

// Transitional: the published native binary (v3.9.0) does not yet extract
// new_expression as a call site. The Rust code is fixed in this PR but the
// binary used by CI is the npm-published one. If the native engine is missing
// the new_expression calls edge, compare after filtering it from WASM output.
// Remove this filter once the next native binary is published.
type Edge = { source_name: string; target_name: string; kind: string };
const nativeHasNewExprEdge = (nativeGraph.edges as Edge[]).some(
(e) => e.kind === 'calls' && e.target_name === 'Calculator' && e.source_name === 'main',
);
if (nativeHasNewExprEdge) {
// Native binary supports new_expression — compare directly
expect(nativeGraph.edges).toEqual(wasmGraph.edges);
} else {
// Filter the new_expression calls edge from WASM output for comparison
const wasmFiltered = (wasmGraph.edges as Edge[]).filter(
(e) => !(e.kind === 'calls' && e.target_name === 'Calculator' && e.source_name === 'main'),
);
expect(nativeGraph.edges).toEqual(wasmFiltered);
}
expect(nativeGraph.edges).toEqual(wasmGraph.edges);
});

it('produces identical roles', () => {
const wasmGraph = readGraph(path.join(wasmDir, '.codegraph', 'graph.db'));
const nativeGraph = readGraph(path.join(nativeDir, '.codegraph', 'graph.db'));

// Transitional: without the new_expression calls edge, the native engine
// classifies Calculator as dead-unresolved instead of core. Filter this
// known divergence when the installed native binary is older.
// Remove this filter once the next native binary is published.
type Role = { name: string; role: string };
const nativeCalcRole = (nativeGraph.roles as Role[]).find((r) => r.name === 'Calculator');
const wasmCalcRole = (wasmGraph.roles as Role[]).find((r) => r.name === 'Calculator');
if (nativeCalcRole?.role === wasmCalcRole?.role) {
expect(nativeGraph.roles).toEqual(wasmGraph.roles);
} else {
// Normalize the Calculator role divergence for comparison
const normalizeRoles = (roles: Role[], targetRole: string) =>
roles.map((r) => (r.name === 'Calculator' ? { ...r, role: targetRole } : r));
expect(normalizeRoles(nativeGraph.roles as Role[], 'core')).toEqual(
normalizeRoles(wasmGraph.roles as Role[], 'core'),
);
}
expect(nativeGraph.roles).toEqual(wasmGraph.roles);
});

it('produces identical ast_nodes', () => {
const wasmGraph = readGraph(path.join(wasmDir, '.codegraph', 'graph.db'));
const nativeGraph = readGraph(path.join(nativeDir, '.codegraph', 'graph.db'));
// Filter out 'call' kind transitionally: the WASM side no longer emits call AST
// nodes, but the published native binary (v3.7.0) still does. Once the next native
// binary is published with call removal, this filter can be dropped.
const filterCall = (nodes: unknown[]) =>
(nodes as { kind: string }[]).filter((n) => n.kind !== 'call');
const wasmFiltered = filterCall(wasmGraph.astNodes);
const nativeFiltered = filterCall(nativeGraph.astNodes);
// Diagnostic: log counts to help debug CI-only parity failures
if (nativeFiltered.length !== wasmFiltered.length) {
console.error(
`[parity-diag] native astNodes: ${nativeFiltered.length}, wasm astNodes: ${wasmFiltered.length}`,
);
console.error(
`[parity-diag] native kinds: ${JSON.stringify([...new Set(nativeFiltered.map((n) => n.kind))])}`,
);
console.error(
`[parity-diag] wasm kinds: ${JSON.stringify([...new Set(wasmFiltered.map((n) => n.kind))])}`,
);
}
expect(nativeFiltered).toEqual(wasmFiltered);
expect(nativeGraph.astNodes).toEqual(wasmGraph.astNodes);
});
});
Loading