Skip to content

fix(scoring): parse underscores in the fractional part of upstream constants - #993

Closed
philluiz2323 wants to merge 1 commit into
JSONbored:mainfrom
philluiz2323:fix/parse-fractional-underscore-constants
Closed

fix(scoring): parse underscores in the fractional part of upstream constants#993
philluiz2323 wants to merge 1 commit into
JSONbored:mainfrom
philluiz2323:fix/parse-fractional-underscore-constants

Conversation

@philluiz2323

Copy link
Copy Markdown
Contributor

Summary

#969 ("parse underscore separators and scientific notation in upstream constants") fixed the Python-numeric-literal parser for the integer part (1_500_000) and exponents (1e-9) — but left the fractional part underscore-blind. Per PEP 515, underscores are valid between digits in any part of a numeric literal, including after the decimal point. So a valid upstream constant like 0.000_001 matches only up to the first fractional underscore and is read as 0 — a silent truncation that turns a small rate/share into zero (a divide/multiply-by-zero hazard that skews every score preview/breakdown).

// src/scoring/model.ts — before
const match = line.match(/^([A-Z][A-Z0-9_]+)\s*=\s*([-+]?(?:\d[\d_]*\.?\d*|\.\d+)(?:[eE][-+]?\d+)?)/);
//                                                          int: \d[\d_]* (has _)   frac: \d* (no _)   bare: \.\d+ (no _)

Verified by running the exact merged regex + Number(raw.replace(/_/g, "")):

Python literal (valid PEP 515) True value Old parser yields
0.000_001 0.000001 0
3.14_15 3.1415 3.14
1_000.000_5 1000.0005 1000

(Integer grouping and exponents parse correctly — only fractional grouping was broken.)

Reachable: refreshScoringModelSnapshot fetches upstream gittensor/constants.py and runs it through this parser into the scoring constants that feed buildScorePreview / the score breakdown / unmodeled-constant detection.

Fix

Allow _ in the fractional digit classes too (and require a leading digit in the bare-decimal branch), mirroring the integer part. Number(raw.replace(/_/g, "")) already strips the underscores, so no other change is needed:

const match = line.match(/^([A-Z][A-Z0-9_]+)\s*=\s*([-+]?(?:\d[\d_]*\.?[\d_]*|\.\d[\d_]*)(?:[eE][-+]?\d+)?)/);

Verified to parse 0.000_001/3.14_15/1_000.000_5 correctly and leave every previously-passing case unchanged (1_500_000, 5.8e1, 1e-9, -2.5e-3, 0.90, 1.15, .5, 5).

Tests

Added a regression test asserting the fractional-underscore literals parse to their true values (the #969 test covered only integer underscores + exponents + a plain 0.90, so the truncation shipped green). Scoring suite green locally (43 passed); tsc clean.

Closes #992

…nstants

The JSONbored#969 constant parser added underscore support for the integer part and
exponents, but the fractional digit classes (\d* / \.\d+) still rejected '_'.
Per PEP 515 underscores are valid between digits in any part, so a valid
upstream literal like 0.000_001 matched only '0.000' and was read as 0 — a
silent truncation (and divide/multiply-by-zero hazard) that skews every score
preview/breakdown.

Allow '_' in the fractional digits too; Number() already strips them before
parsing. Existing integer-underscore/exponent cases are unchanged.

Closes JSONbored#992
@philluiz2323
philluiz2323 requested a review from JSONbored as a code owner June 22, 2026 06:26
@dosubot dosubot Bot added the size:S label Jun 22, 2026
@ghost

ghost commented Jun 22, 2026

Copy link
Copy Markdown

Note

Gittensory Gate skipped

PR closed before full evaluation. No late first comment was created.

Signal Result Evidence Action
Gate result ⚠️ Skipped #993 is no longer open. No action.

💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →.

Checked by Gittensory, a quiet PR intelligence layer for OSS maintainers.

@ghost ghost added the gittensory:reviewed label Jun 22, 2026
@superagent-security

Copy link
Copy Markdown
Contributor

Superagent didn't find any vulnerabilities or security issues in this PR.

@codecov

codecov Bot commented Jun 22, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 70.01%. Comparing base (363c0fb) to head (b762e99).

❗ There is a different number of reports uploaded between BASE (363c0fb) and HEAD (b762e99). Click for more details.

HEAD has 1 upload less than BASE
Flag BASE (363c0fb) HEAD (b762e99)
2 1
Additional details and impacted files
@@             Coverage Diff             @@
##             main     #993       +/-   ##
===========================================
- Coverage   96.61%   70.01%   -26.60%     
===========================================
  Files         110      110               
  Lines       15079    15079               
  Branches     5451     5451               
===========================================
- Hits        14569    10558     -4011     
- Misses        106     3231     +3125     
- Partials      404     1290      +886     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@ghost ghost added the gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier. label Jun 22, 2026
@ghost

ghost commented Jun 22, 2026

Copy link
Copy Markdown

🟥🟥🟥🟥🟥🟥🟥🟥🟥🟥🟥🟥🟥🟥🟥🟥🟥🟥

⛔ Maintainer review — Blockers found

Reviewed 2 changed file(s) — two independent AI reviewers, synthesized.

ℹ️ A required check is failing. CI failures are closed, not iterated in place — fix it and open a new focused PR.

Merge readiness: ❌ CI is red — approval withheld until it's green. Failing: see the checks tab.

Summary

The change fixes a parsing bug by updating the regex in parsePythonNumberConstants to handle underscore separators in the fractional part of Python numeric literals, and adds a test confirming correct behavior. The implementation correctly strips underscores before Number conversion, preserving existing security and type guarantees. No new security concerns are introduced. The added test improves coverage of this core parsing logic. Overall the modification is correct and maintainable, though the regex is now more complex.

🛑 Hard blockers — must fix before merge

  • The test job (2) failed in CI, which needs to be addressed before merging.

💡 Nits — non-blocking, optional

  • Regex complexity increased; ensure future contributors understand its parts.
  • Potential minor coverage drop causing project-level codecov failure.
  • Consider adding a comment or unit test for handling invalid underscore placements (e.g., trailing or consecutive underscores) to document expected behavior.
  • Clean up stray encoding artifacts (e.g., "â�¦") in warning messages for readability.
  • Consider adding a comment explaining why the regex change is necessary for handling underscores in fractional parts.

This repo uses one-shot review: a rejected PR is closed, not iterated in place. Address the above and open a new focused PR.

@ghost ghost closed this Jun 22, 2026
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: upstream-constant parser truncates fractional digit-grouping (0.000_001 -> 0)

1 participant