Skip to content

Add JS/Python profiler parity tests - #126

Merged
yakew7 merged 2 commits into
yakew7:mainfrom
ahmdkaml:js-profiler-parity-ci
Aug 4, 2026
Merged

Add JS/Python profiler parity tests#126
yakew7 merged 2 commits into
yakew7:mainfrom
ahmdkaml:js-profiler-parity-ci

Conversation

@ahmdkaml

@ahmdkaml ahmdkaml commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

What changed

  • Added a Node.js harness (scripts/profile-js.js) to execute the JavaScript profiler from CI.
  • Added parity tests covering multiple fixture datasets.
  • Fixed JS profiler behavior to match the Python implementation:
    • Preserve "none" as a categorical value instead of treating it as missing.
    • Emit small_group reliability flags.
  • Updated the profiler CI workflow to provision Node.js so the parity tests run automatically.

This automates the Python↔JavaScript parity check described in SPEC.md and helps prevent future implementation drift.
Closes #110

@vercel

vercel Bot commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

@ahmdkaml is attempting to deploy a commit to the yashkewlani2020-gmailcom's projects Team on Vercel.

A member of the Team first needs to authorize it.

@ahmdkaml
ahmdkaml marked this pull request as ready for review August 4, 2026 14:23
Copilot AI lite review requested due to automatic review settings August 4, 2026 14:23

@yakew7 yakew7 left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Looks fine Merging the PR

@yakew7
yakew7 merged commit 599f4ef into yakew7:main Aug 4, 2026
10 of 11 checks passed

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Adds automated Python↔JavaScript profiler parity checks (per faircode/SPEC.md / issue #110) by introducing a Node.js harness, fixture-backed pytest parity tests, and CI provisioning for Node so parity runs on every PR.

Changes:

  • Add scripts/profile-js.js Node harness to execute the JS profiler engine and emit JSON.
  • Add pytest parity coverage across multiple fixture CSVs (including new small.csv and credit_customers.csv).
  • Update JS engine behavior to better align with the Python profiler ("none" handling and small_group reliability flags), and update CI (audits.yml) to install Node 20 for profiler tests.

Reviewed changes

Copilot reviewed 5 out of 9 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
tests/test_js_parity.py Adds Python↔JS parity test that runs Node harness and compares profiler JSON results.
tests/fixtures/small.csv New minimal fixture dataset for parity testing.
tests/fixtures/credit_customers.csv New larger categorical-heavy fixture dataset for parity testing.
scripts/profile-js.js New Node.js harness to run the JS profiler engine from CI/tests and print JSON.
assets/profiler-engine.js Adjusts missing-token handling and adds small_group flag messages in JS to match Python behavior.
.github/workflows/audits.yml Provisions Node.js in the profiler job so parity tests can run in CI.
Suppressed comments (2)

tests/test_js_parity.py:36

  • subprocess.run(["node", "scripts/profile-js.js", ...]) will fail if the test is executed from a non-repo-root working directory. Using the resolved harness path (and the discovered NODE executable) makes this robust and matches how the CSV path is handled.
    completed = subprocess.run(
        ["node", "scripts/profile-js.js", str(csv)],
        capture_output=True,
        text=True,
        encoding="utf-8",

tests/test_js_parity.py:46

  • The parity assertion currently drops flags entirely. flags are part of the profiler contract (and are tested on the Python side in tests/test_profiler.py), so excluding them means JS flagging drift could slip through even if the structured metrics still match. If exact string formatting is too brittle across languages, consider comparing a normalized form of the flags (e.g., stripping numeric literals) in addition to the full structured JSON.
    # Flags are human-readable messages. They duplicate information already
    # present in the structured output and may differ because Python and
    # JavaScript format floating-point values differently (e.g. 6.25 -> 6.2
    # vs 6.3). Compare the structured data instead.
    python_result = dict(python_result)

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread tests/test_js_parity.py
Comment on lines +3 to +13
from pathlib import Path
import json
import subprocess

import pandas as pd
import pytest

from faircode import profile

FIXTURES = Path(__file__).resolve().parent / "fixtures"

yakew7 added a commit that referenced this pull request Aug 4, 2026
The parity test suite added in #126 copied 25MB of dataset CSVs into
tests/fixtures/ that are byte-identical to files already tracked in
their audit folders. Point the tests at the existing datasets instead.
yakew7 added a commit that referenced this pull request Aug 4, 2026
The parity test suite added in #126 copied 25MB of dataset CSVs into
tests/fixtures/ that are byte-identical to files already tracked in
their audit folders. Point the tests at the existing datasets instead.
yakew7 added a commit that referenced this pull request Aug 4, 2026
…rity work

Documents the FAQPage JSON-LD + sitemap lastmod, llms-full.txt, the
homepage FAQ/Contact/Legal section, PR #126's JS/Python profiler
parity tests, and the fixture-dedup fix. README's Connect section
gained the same contact email and FAQ link for consistency.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

CI: automated Python↔JS profiler parity test

3 participants