From 8c8f6ea1dc4d4f2507da499b3ca6151383e45359 Mon Sep 17 00:00:00 2001 From: mstrathman Date: Sun, 2 Aug 2026 16:58:27 -0700 Subject: [PATCH 1/2] fix: resolve open CodeQL alerts (float widening, workflow permissions) Two high alerts (cpp/integer-multiplication-cast-to-long): forest serving multiplied f->lr (float) by a float tree value before widening to the double accumulator, so the product could overflow to infinity, and under x87 excess precision vary by platform, before the widening. Promote to double before the multiply in both spots, matching the trainer (predict-train.c already does). Two medium alerts (actions/missing-workflow-permissions): wheels.yml had no permissions block, so the build jobs got the default token scopes. Default the workflow token to contents:read; the publish job keeps its id-token:write override for PyPI OIDC. Co-Authored-By: Claude Opus 4.8 --- .github/workflows/wheels.yml | 5 +++++ CHANGELOG.md | 9 +++++++++ predict-student.c | 8 ++++++-- 3 files changed, 20 insertions(+), 2 deletions(-) diff --git a/.github/workflows/wheels.yml b/.github/workflows/wheels.yml index e4d62e0..7a11149 100644 --- a/.github/workflows/wheels.yml +++ b/.github/workflows/wheels.yml @@ -5,6 +5,11 @@ on: tags: ["v*"] workflow_dispatch: +# Default the GITHUB_TOKEN to read-only; the publish job opts into +# id-token: write for PyPI OIDC and nothing else needs write. +permissions: + contents: read + # Builds the Python wheels (`pip install sqlite-predict`). Each wheel bundles the # zero-dependency loadable compiled for its platform. Staging runs `make # python-src` on the host to drop the amalgamation + SQLite ext headers into the diff --git a/CHANGELOG.md b/CHANGELOG.md index 7be2549..1fe666b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,15 @@ project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). ## [Unreleased] +### Fixed + +- **Forest prediction accumulates the learning-rate product in double**, matching + the trainer. The serving path computed `float * float` before widening to the + `double` accumulator, which could overflow to infinity (and, under x87 excess + precision, differ by platform) before the widening. Predictions are computed + consistently in double now; served values may shift in their least-significant + digits. + ## [0.2.0] - 2026-07-31 ### Changed diff --git a/predict-student.c b/predict-student.c index ff342fe..1946bfd 100644 --- a/predict-student.c +++ b/predict-student.c @@ -276,7 +276,10 @@ int predict0_forest_predict_row(const Forest *f, const f32 *x, f64 *scbuf, char if (f->task == 1) { f64 s = f->init[0]; for (int j = 0; j < f->n_trees; j++) - s += f->lr * forest_tree_value(f, j, x); + /* (f64) before the multiply: keep the product in double, matching the + * trainer (predict-train.c), so a float*float never overflows or picks up + * platform-dependent excess precision before it reaches the accumulator. */ + s += (f64)f->lr * forest_tree_value(f, j, x); *has_conf = 0; *pred = sqlite3_mprintf("%.17g", s); return *pred ? SQLITE_OK : SQLITE_NOMEM; @@ -285,7 +288,8 @@ int predict0_forest_predict_row(const Forest *f, const f32 *x, f64 *scbuf, char scbuf[c] = f->init[c]; for (int r = 0; r < f->n_rounds; r++) for (int s = 0; s < f->n_score; s++) - scbuf[s] += f->lr * forest_tree_value(f, r * f->n_score + s, x); + /* (f64) before the multiply, as above and in the trainer */ + scbuf[s] += (f64)f->lr * forest_tree_value(f, r * f->n_score + s, x); f64 mx = scbuf[0]; int arg = 0; for (int c = 1; c < f->n_score; c++) From 55d6e2ec0ea02423f89af2546bca3fcfe980c24f Mon Sep 17 00:00:00 2001 From: mstrathman Date: Sun, 2 Aug 2026 17:18:21 -0700 Subject: [PATCH 2/2] docs: narrow the changelog precision claim Address review: the fix moved the learning-rate/tree-value product and the forest accumulation to double, but individual tree values are still f32. Say that precisely instead of claiming the whole prediction is computed in double. Co-Authored-By: Claude Opus 4.8 --- CHANGELOG.md | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1fe666b..5b02e38 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,11 +9,12 @@ project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). ### Fixed - **Forest prediction accumulates the learning-rate product in double**, matching - the trainer. The serving path computed `float * float` before widening to the - `double` accumulator, which could overflow to infinity (and, under x87 excess - precision, differ by platform) before the widening. Predictions are computed - consistently in double now; served values may shift in their least-significant - digits. + the trainer. The serving path multiplied the learning rate by a tree value as + `float * float` before widening to the `double` accumulator, which could + overflow to infinity (and, under x87 excess precision, differ by platform) + before the widening. The product and the forest accumulation are now computed in + double (individual tree values are still `f32`); served predictions may shift in + their least-significant digits. ## [0.2.0] - 2026-07-31