From 8a40b8ebb4992b01ddb9180ad509aa29ff1c8525 Mon Sep 17 00:00:00 2001 From: mstrathman Date: Sun, 2 Aug 2026 15:22:21 -0700 Subject: [PATCH 1/4] feat: fit() accepts an optional leading model name Mirror predict(model, ...) on the training side: fit('churn', f1, ..., fN, label) registers the trained student as 'churn' and returns its id, so the train and serve calls read in parallel with predict('churn', f1, ..., fN). A leading argument is only a name when it is TEXT, and features are numeric, so every existing fit(f1, ..., fN, label [, options]) call is unchanged. The name may still be given as {"register":"churn"}; supplying it both ways is a mistake, not a precedence to resolve, and raises PREDICT_ERR_OPTIONS. The leading name, like the options object, must be constant within a group. Adds adversarial tests (name-given-twice, name-with-no-features) and a parity test that a leading-name model predicts identically to an options-register one. Docs and CHANGELOG updated. Co-Authored-By: Claude Opus 4.8 --- CHANGELOG.md | 11 +++ README.md | 2 +- predict-tabular.c | 76 ++++++++++++++----- tests/test_fit.py | 59 ++++++++++++++ website/src/content/docs/guides/operations.md | 2 +- .../src/content/docs/reference/functions.md | 30 +++++--- website/src/content/docs/reference/options.md | 8 +- 7 files changed, 155 insertions(+), 33 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5b02e38..d31733b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,17 @@ project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). ## [Unreleased] +### Added + +- **`fit` accepts an optional leading model name**, mirroring `predict`: + `fit('churn', f1, ..., fN, label)` registers the trained student as `churn` and + returns its id, so the train and serve calls read in parallel with + `predict('churn', f1, ..., fN)`. The name can still be given as + `'{"register":"churn"}'` instead; supplying it both ways raises + `PREDICT_ERR_OPTIONS`. Existing `fit(f1, ..., fN, label [, options])` calls are + unchanged (a leading argument is only a name when it is TEXT, and features are + numeric). + ### Fixed - **Forest prediction accumulates the learning-rate product in double**, matching diff --git a/README.md b/README.md index 6ee19af..703f794 100644 --- a/README.md +++ b/README.md @@ -159,7 +159,7 @@ skills-capable agent, or read them as the condensed operator's manual. | `forecast(ts, value, horizon [, options])` | Where is this metric going? | an aggregate over your rows: forecast steps with prediction intervals, one JSON document per group | | `detect_anomalies(ts, value [, options])` | Which points are abnormal? | an aggregate over your rows: per-point anomaly probability and interval, one JSON document per group | | `backtest(ts, value, horizon [, options])` | How accurate is the model here? | an aggregate over your rows: per-fold MAE, RMSE, MASE, sMAPE and coverage from rolling-origin evaluation | -| `fit(f1, ..., fN, label [, options])` | Train a model on labeled rows | an aggregate over your rows: a native tabular student, registered by id or returned as a blob | +| `fit([name,] f1, ..., fN, label [, options])` | Train a model on labeled rows | an aggregate over your rows: a native tabular student, named by a leading id (or `register`) or returned as a blob | | `predict(model, f1, ..., fN [, options])` | Classify or regress a row | a scalar: one prediction per row from a fitted student | | `distill_predict(train_query [, options])` | Compress a slow teacher into a fast student | a tiny native model (decision tree, gradient-boosted forest, or MLP), registered and served by `predict` | | `distill_forecast(train_query [, options])` | Compress a forecast foundation model into a fast student | a native DLinear/TiDE forecast net (a linear skip plus a small residual), registered and served by `forecast` | diff --git a/predict-tabular.c b/predict-tabular.c index 1d035ec..1b48044 100644 --- a/predict-tabular.c +++ b/predict-tabular.c @@ -855,12 +855,15 @@ static sqlite3_module predictModule = { /* ====================================================================== * fit() aggregate + predict() scalar: the guessable tabular pair. - * fit(f1, ..., fN, label [, options]) trains a native student over the rows of - * the statement (label is the last positional argument, no target option) and - * returns the plain registered id text when options request registration - * ({"register":"my-id"}), otherwise its serialized model blob. The optional - * trailing options is a TEXT JSON object; a trailing '{...}' is therefore always - * read as options, so a class label must not itself be a JSON-object string. + * fit([name,] f1, ..., fN, label [, options]) trains a native student over the + * rows of the statement (features are numeric and positional, the label is the + * last positional argument, no target option). An optional leading TEXT argument + * names and registers the model, mirroring predict(model, ...); the same name may + * instead be given as {"register":"my-id"}, but supplying it both ways is an + * error. With a name fit() returns the registered id text; with neither it + * returns the serialized model blob. The optional trailing options is a TEXT JSON + * object; a trailing '{...}' is therefore always read as options, so a class label + * must not itself be a JSON-object string. * predict(model, f1, ..., fN [, options]) serves that student per row, features * positional, deserialized once and cached on the model argument. * ====================================================================== */ @@ -912,6 +915,7 @@ static const char *const FIT_OPTION_KEYS[] = {"kind", "student_kind", "task", typedef struct { int configured; int has_opts; + int has_name; /* a leading TEXT model-name argument was present */ char *opts_raw; /* trailing options text of the first row (owned), or NULL */ int nfeat; int classify; @@ -971,11 +975,26 @@ static void fit_step(sqlite3_context *ctx, int argc, sqlite3_value **argv) { } } + /* An optional leading model name: a TEXT first argument. Features are numeric, + * so a text argv[0] is unambiguously the register name — the guessable mirror of + * predict(model, ...). Like the options object it must be constant across the + * group, and it is reconciled against {"register":...} below (supplying the name + * both ways is an error, not a silent precedence). */ + int has_name = (argc >= 1 && sqlite3_value_type(argv[0]) == SQLITE_TEXT); + const char *name_txt = NULL; + if (has_name) { + name_txt = (const char *)sqlite3_value_text(argv[0]); + if (!name_txt) { /* TEXT value but NULL text pointer means an allocation failed */ + c->err = SQLITE_NOMEM; + return; + } + } + if (!c->configured) { - int nfeat = argc - 1 - has_opts; + int nfeat = argc - has_name - 1 - has_opts; if (nfeat < 1) { c->err = SQLITE_ERROR; - c->errmsg = sqlite3_mprintf("%s: fit(feature, ..., label [, options])" + c->errmsg = sqlite3_mprintf("%s: fit([name,] feature, ..., label [, options])" " needs at least one feature and a label", PREDICT_ERR_SCHEMA); return; @@ -998,6 +1017,16 @@ static void fit_step(sqlite3_context *ctx, int argc, sqlite3_value **argv) { return; } } + /* The model name is given at most once: a leading argument OR {"register":...}, + * never both — a conflicting pair is a mistake, not a precedence to resolve. */ + if (has_name && o.reg) { + c->err = SQLITE_ERROR; + c->errmsg = sqlite3_mprintf( + "%s: model name given twice (leading argument and \"register\" option)", + PREDICT_ERR_OPTIONS); + fit_opts_free(&o); + return; + } int classify = 1; if (o.task) { if (strcmp(o.task, "regress") == 0) @@ -1014,10 +1043,20 @@ static void fit_step(sqlite3_context *ctx, int argc, sqlite3_value **argv) { c->classify = classify; c->kind = o.kind; /* transfer ownership */ o.kind = NULL; - c->reg_id = o.reg; - o.reg = NULL; + if (has_name) { + c->reg_id = sqlite3_mprintf("%s", name_txt); + if (!c->reg_id) { + c->err = SQLITE_NOMEM; + fit_opts_free(&o); + return; + } + } else { + c->reg_id = o.reg; /* transfer ownership */ + o.reg = NULL; + } fit_opts_free(&o); c->has_opts = has_opts; + c->has_name = has_name; if (has_opts) { c->opts_raw = sqlite3_mprintf("%s", opts_txt); if (!c->opts_raw) { @@ -1026,11 +1065,13 @@ static void fit_step(sqlite3_context *ctx, int argc, sqlite3_value **argv) { } } c->configured = 1; - } else if (has_opts != c->has_opts || - (has_opts && (!c->opts_raw || strcmp(opts_txt, c->opts_raw) != 0))) { + } else if (has_opts != c->has_opts || has_name != c->has_name || + (has_opts && (!c->opts_raw || strcmp(opts_txt, c->opts_raw) != 0)) || + (has_name && (!c->reg_id || strcmp(name_txt, c->reg_id) != 0))) { c->err = SQLITE_ERROR; - c->errmsg = sqlite3_mprintf("%s: options must be constant within a group", - PREDICT_ERR_OPTIONS); + c->errmsg = sqlite3_mprintf( + "%s: fit options and model name must be constant within a group", + PREDICT_ERR_OPTIONS); return; } @@ -1072,14 +1113,15 @@ static void fit_step(sqlite3_context *ctx, int argc, sqlite3_value **argv) { } f32 *row = &c->X[(size_t)c->n * c->nfeat]; for (int i = 0; i < c->nfeat; i++) { - int t = sqlite3_value_type(argv[i]); + /* features start after the optional leading name (argv[0]) */ + int t = sqlite3_value_type(argv[has_name + i]); if (t != SQLITE_INTEGER && t != SQLITE_FLOAT) { c->err = SQLITE_ERROR; c->errmsg = sqlite3_mprintf("%s: fit feature %d must be numeric", PREDICT_ERR_SCHEMA, i + 1); return; } - f64 fv = sqlite3_value_double(argv[i]); + f64 fv = sqlite3_value_double(argv[has_name + i]); if (!isfinite(fv)) { /* 1e999 -> +Inf as SQLITE_FLOAT; NaN poisons splits */ c->err = SQLITE_ERROR; c->errmsg = sqlite3_mprintf("%s: fit feature %d must be finite", @@ -1088,7 +1130,7 @@ static void fit_step(sqlite3_context *ctx, int argc, sqlite3_value **argv) { } row[i] = (f32)fv; } - sqlite3_value *lv = argv[c->nfeat]; + sqlite3_value *lv = argv[has_name + c->nfeat]; if (c->classify) { /* A NULL class label is not a real class; fail loudly rather than train a * silent empty-string class. A NULL text under memory pressure is NOMEM. */ diff --git a/tests/test_fit.py b/tests/test_fit.py index 029c4f9..b806f80 100644 --- a/tests/test_fit.py +++ b/tests/test_fit.py @@ -35,6 +35,65 @@ def test_fit_register_then_scalar_predict(db): assert preds == ["1", "1", "0", "0"] +def test_fit_leading_name_rhymes_with_predict(db): + """The guessable mirror: a leading TEXT model name registers the model, so + fit('id', f..., label) reads parallel to predict('id', f...). Same churn + signal, no options object needed.""" + _seed(db) + mid = db.execute( + "SELECT fit('churn-lead', tenure, spend, churned) FROM h").fetchone()[0] + assert mid == "churn-lead" + preds = [r[1] for r in db.execute( + "SELECT id, predict('churn-lead', tenure, spend) FROM a ORDER BY id")] + assert preds == ["1", "1", "0", "0"] + + +def test_fit_leading_name_composes_with_options(db): + """A leading name and a trailing options object coexist: the name registers, + the options still select the student kind.""" + _seed(db) + mid = db.execute( + "SELECT fit('lead-tree', tenure, spend, churned, '{\"kind\":\"tree\"}')" + " FROM h").fetchone()[0] + assert mid == "lead-tree" + assert db.execute("SELECT predict('lead-tree', 2, 20)").fetchone()[0] == "1" + + +def test_fit_leading_name_equals_options_register(db): + """The two ways to name a model are equivalent: a model registered by a + leading name predicts identically to one registered via {"register":...} on + the same rows (same features, same training, deterministic).""" + _seed(db) + db.execute("SELECT fit('lead-eq', tenure, spend, churned) FROM h").fetchone() + db.execute("SELECT fit(tenure, spend, churned, '{\"register\":\"opt-eq\"}')" + " FROM h").fetchone() + lead = [r[0] for r in db.execute( + "SELECT predict('lead-eq', tenure, spend) FROM a ORDER BY id")] + opt = [r[0] for r in db.execute( + "SELECT predict('opt-eq', tenure, spend) FROM a ORDER BY id")] + assert lead == opt + + +def test_fit_model_name_given_twice_fails_loud(db): + """A leading name and a {"register":...} option name the model twice. That is + a mistake, not a precedence to resolve silently: fail loudly + (PREDICT_ERR_OPTIONS).""" + _seed(db) + with pytest.raises(sqlite3.OperationalError) as e: + db.execute("SELECT fit('twice-a', tenure, spend, churned," + " '{\"register\":\"twice-b\"}') FROM h").fetchall() + assert "PREDICT_ERR_OPTIONS" in str(e.value) + + +def test_fit_leading_name_still_needs_features_and_label(db): + """A leading name does not substitute for data: fit('id', label) has a name + and a label but no features, and must fail loudly (PREDICT_ERR_SCHEMA).""" + _seed(db) + with pytest.raises(sqlite3.OperationalError) as e: + db.execute("SELECT fit('nofeat', churned) FROM h").fetchall() + assert "PREDICT_ERR_SCHEMA" in str(e.value) + + def test_fit_blob_served_via_cte(db): """No registration: fit() returns a model blob, served in one statement.""" _seed(db) diff --git a/website/src/content/docs/guides/operations.md b/website/src/content/docs/guides/operations.md index f0e29b5..fb97999 100644 --- a/website/src/content/docs/guides/operations.md +++ b/website/src/content/docs/guides/operations.md @@ -16,7 +16,7 @@ e.g. `'{"confidence_level":0.9}'`. | `forecast(ts, value, horizon [, options])` | Where is this metric going? | one JSON document per group: future rows with prediction intervals and a status | | `detect_anomalies(ts, value [, options])` | Which points are abnormal? | one JSON document per group: anomaly-scored rows with expected value and probability | | `backtest(ts, value, horizon [, options])` | How accurate is the model here? | one JSON document per group: per-fold MAE / RMSE / MASE / sMAPE and coverage | -| `fit(f1, ..., fN, label [, options])` | Train a model on labeled rows | a registered model id, or a model blob | +| `fit([name,] f1, ..., fN, label [, options])` | Train a model on labeled rows | a registered model id, or a model blob | | `predict(model, f1, ..., fN [, options])` | Classify or regress a row | a prediction, or a `{prediction, confidence}` document with `proba` | | `distill_predict(train_query [, options])` | Compress a teacher into a fast student | a registered native tabular model | | `distill_forecast(train_query [, options])` | Compress a forecast model into a student | a registered native forecast model | diff --git a/website/src/content/docs/reference/functions.md b/website/src/content/docs/reference/functions.md index cbfd3fd..417b9b0 100644 --- a/website/src/content/docs/reference/functions.md +++ b/website/src/content/docs/reference/functions.md @@ -65,19 +65,27 @@ SELECT avg(mae) FROM backtest_rows( (SELECT backtest(ts, value, 6, '{"folds":20}') FROM readings)); ``` -### `fit(f1, ..., fN, label [, options])` - -Trains a native tabular student over your rows. The features are the leading -positional arguments and the **label is the last argument** (there is no -`target` option). The optional trailing `options` is a TEXT JSON object, so a -trailing `{...}` argument is always read as options: a class **label must not be -a JSON-object string** (it would be consumed as options). Returns the model: -with `'{"register":"churn-v1"}'` it registers into `_predict_models` and returns -the id, otherwise it returns a model blob you can pass to `predict`. Options: -`kind` (`gbt` default, or `tree`), `task` (`classify`/`regress`, inferred from -the label), `register`. +### `fit([name,] f1, ..., fN, label [, options])` + +Trains a native tabular student over your rows. The features are positional and +the **label is the last positional argument** (there is no `target` option). An +optional **leading TEXT argument names and registers the model**, mirroring +`predict(model, ...)` so the train and serve calls read in parallel: +`fit('churn', f..., label)` then `predict('churn', f...)`. The same name can +instead be given as `'{"register":"churn"}'`, but supplying it both ways raises +`PREDICT_ERR_OPTIONS`. With a name, `fit` registers into `_predict_models` and +returns the id; with neither it returns a model blob you can pass to `predict`. +The optional trailing `options` is a TEXT JSON object, so a trailing `{...}` +argument is always read as options: a class **label must not be a JSON-object +string** (it would be consumed as options). Options: `kind` (`gbt` default, or +`tree`), `task` (`classify`/`regress`, inferred from the label), `register`. ```sql +-- name the model with a leading id, and fit/predict read in parallel: +SELECT fit('churn-v1', tenure, spend, churned) FROM history; +SELECT id, predict('churn-v1', tenure, spend) AS churn FROM active; + +-- or name it in options, alongside the student kind: SELECT fit(tenure, spend, churned, '{"kind":"gbt","register":"churn-v1"}') FROM history; ``` diff --git a/website/src/content/docs/reference/options.md b/website/src/content/docs/reference/options.md index 0e993ec..e84aa70 100644 --- a/website/src/content/docs/reference/options.md +++ b/website/src/content/docs/reference/options.md @@ -48,14 +48,16 @@ carry the columns and `GROUP BY` carries the series split. ## fit -Trains a native tabular student over your rows; the label is the last -positional argument, so there is no `target` option. +`fit([name,] f1, ..., fN, label [, options])` trains a native tabular student +over your rows; the label is the last positional argument, so there is no +`target` option. An optional leading TEXT argument names and registers the +model, `fit('id', f..., label)`, mirroring `predict('id', f...)`. | Key | Type | Default | Meaning | | --- | --- | --- | --- | | `kind` | `gbt` \| `tree` | `gbt` | Student architecture. | | `task` | `classify` \| `regress` | inferred | Inferred from the label: integer or text is classify, real is regress. | -| `register` | string | none | Register the model under this id and return the id; without it, `fit` returns a model blob. | +| `register` | string | none | Register the model under this id and return the id; without it, `fit` returns a model blob. The id can instead be given as a leading positional argument, `fit('id', ...)`; supplying it both ways raises `PREDICT_ERR_OPTIONS`. | ## predict From a83689d3fabc14052458005924a0e792caf9ad0c Mon Sep 17 00:00:00 2001 From: mstrathman Date: Sun, 2 Aug 2026 16:57:21 -0700 Subject: [PATCH 2/4] refactor: make fit options-arity discount the leading name Address review: the trailing-options detection used a raw argc >= 3, so a JSON-object trailing argument was consumed as options with a leading name but could land as a class label without one. Detect the leading name first and gate options on argc - has_name >= 3, so both forms treat the trailing object the same way. Adds a consistency test. Co-Authored-By: Claude Opus 4.8 --- predict-tabular.c | 44 ++++++++++++++++++++++++-------------------- tests/test_fit.py | 13 +++++++++++++ 2 files changed, 37 insertions(+), 20 deletions(-) diff --git a/predict-tabular.c b/predict-tabular.c index 1b48044..ff9b0e2 100644 --- a/predict-tabular.c +++ b/predict-tabular.c @@ -956,30 +956,12 @@ static void fit_step(sqlite3_context *ctx, int argc, sqlite3_value **argv) { } if (c->err) return; - /* Determine this row's trailing options: the last argument when it is TEXT - * beginning with '{'. Presence and content must be constant across the group, - * or the feature/label split and the model config would depend on the order - * SQLite happens to visit rows in. Consequence of this convention: the trailing - * '{...}' is ALWAYS the options object, so a class label must not be a JSON - * object string (it would be consumed as options). A malformed options object - * fails loudly at predict0_options_parse; the only quiet case is a label that - * is itself a well-formed, valid-key options object, which is why the label - * contract is documented rather than guessed. */ - int has_opts = 0; - const char *opts_txt = NULL; - if (argc >= 3 && sqlite3_value_type(argv[argc - 1]) == SQLITE_TEXT) { - const char *t = (const char *)sqlite3_value_text(argv[argc - 1]); - if (t && t[0] == '{') { - has_opts = 1; - opts_txt = t; - } - } - /* An optional leading model name: a TEXT first argument. Features are numeric, * so a text argv[0] is unambiguously the register name — the guessable mirror of * predict(model, ...). Like the options object it must be constant across the * group, and it is reconciled against {"register":...} below (supplying the name - * both ways is an error, not a silent precedence). */ + * both ways is an error, not a silent precedence). Detected before the trailing + * options so the options arity can discount it. */ int has_name = (argc >= 1 && sqlite3_value_type(argv[0]) == SQLITE_TEXT); const char *name_txt = NULL; if (has_name) { @@ -990,6 +972,28 @@ static void fit_step(sqlite3_context *ctx, int argc, sqlite3_value **argv) { } } + /* This row's trailing options: the last argument when it is TEXT beginning with + * '{'. It is options only when a feature and a label still precede it after the + * optional name (argc - has_name >= 3); with fewer arguments the trailing '{...}' + * is the label, exactly as the name-less form treats it, so both forms stay + * consistent. Presence and content must be constant across the group, or the + * feature/label split and the model config would depend on the order SQLite + * happens to visit rows in. Consequence: a trailing '{...}' with a feature and + * label present is ALWAYS the options object, so a class label must not be a + * JSON-object string (it would be consumed as options). A malformed options + * object fails loudly at predict0_options_parse; the only quiet case is a label + * that is itself a well-formed, valid-key options object, which is why the label + * contract is documented rather than guessed. */ + int has_opts = 0; + const char *opts_txt = NULL; + if (argc - has_name >= 3 && sqlite3_value_type(argv[argc - 1]) == SQLITE_TEXT) { + const char *t = (const char *)sqlite3_value_text(argv[argc - 1]); + if (t && t[0] == '{') { + has_opts = 1; + opts_txt = t; + } + } + if (!c->configured) { int nfeat = argc - has_name - 1 - has_opts; if (nfeat < 1) { diff --git a/tests/test_fit.py b/tests/test_fit.py index b806f80..66ad5ea 100644 --- a/tests/test_fit.py +++ b/tests/test_fit.py @@ -94,6 +94,19 @@ def test_fit_leading_name_still_needs_features_and_label(db): assert "PREDICT_ERR_SCHEMA" in str(e.value) +def test_fit_leading_name_json_object_still_options(db): + """The leading name discounts the options arity, so a trailing JSON-object + argument is consumed as options with or without a name: fit('id', f1, f2, obj) + fails loudly (PREDICT_ERR_OPTIONS) just like fit(f1, f2, obj), rather than one + form treating the object as a class label and the other as options.""" + db.execute("CREATE TABLE jl2(f1 REAL, f2 REAL, lbl TEXT)") + db.executemany("INSERT INTO jl2 VALUES (?, ?, ?)", + [(i * 1.0, (i % 3) * 1.0, '{"category":"A"}') for i in range(20)]) + with pytest.raises(sqlite3.OperationalError) as e: + db.execute("SELECT fit('jlmodel', f1, f2, lbl) FROM jl2").fetchall() + assert "PREDICT_ERR_OPTIONS" in str(e.value) + + def test_fit_blob_served_via_cte(db): """No registration: fit() returns a model blob, served in one statement.""" _seed(db) From 2e305185e9eec62610ba988a7c823048935e317f Mon Sep 17 00:00:00 2001 From: mstrathman Date: Sun, 2 Aug 2026 17:00:17 -0700 Subject: [PATCH 3/4] docs: correct fit task default and document per-group constancy Address review: the fit reference claimed task is "inferred from the label", but fit defaults to classify and only regresses when task=regress is given explicitly (a real-valued label without it is trained as classification). State the real default. Also document that the leading name and the options object must be constant within an aggregate group. Co-Authored-By: Claude Opus 4.8 --- website/src/content/docs/reference/functions.md | 6 ++++-- website/src/content/docs/reference/options.md | 5 +++-- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/website/src/content/docs/reference/functions.md b/website/src/content/docs/reference/functions.md index 417b9b0..5c2d8a0 100644 --- a/website/src/content/docs/reference/functions.md +++ b/website/src/content/docs/reference/functions.md @@ -77,8 +77,10 @@ instead be given as `'{"register":"churn"}'`, but supplying it both ways raises returns the id; with neither it returns a model blob you can pass to `predict`. The optional trailing `options` is a TEXT JSON object, so a trailing `{...}` argument is always read as options: a class **label must not be a JSON-object -string** (it would be consumed as options). Options: `kind` (`gbt` default, or -`tree`), `task` (`classify`/`regress`, inferred from the label), `register`. +string** (it would be consumed as options). Within an aggregate group the leading +name and the `options` object must be constant, or the call raises +`PREDICT_ERR_OPTIONS`. Options: `kind` (`gbt` default, or `tree`), `task` +(`classify` default, or `regress` for a numeric target), `register`. ```sql -- name the model with a leading id, and fit/predict read in parallel: diff --git a/website/src/content/docs/reference/options.md b/website/src/content/docs/reference/options.md index e84aa70..bc876bb 100644 --- a/website/src/content/docs/reference/options.md +++ b/website/src/content/docs/reference/options.md @@ -51,12 +51,13 @@ carry the columns and `GROUP BY` carries the series split. `fit([name,] f1, ..., fN, label [, options])` trains a native tabular student over your rows; the label is the last positional argument, so there is no `target` option. An optional leading TEXT argument names and registers the -model, `fit('id', f..., label)`, mirroring `predict('id', f...)`. +model, `fit('id', f..., label)`, mirroring `predict('id', f...)`. The leading +name and the options object must be constant within an aggregate group. | Key | Type | Default | Meaning | | --- | --- | --- | --- | | `kind` | `gbt` \| `tree` | `gbt` | Student architecture. | -| `task` | `classify` \| `regress` | inferred | Inferred from the label: integer or text is classify, real is regress. | +| `task` | `classify` \| `regress` | `classify` | Default `classify` (the label is read as a class). Pass `regress` for a numeric target; a real-valued label is not inferred. | | `register` | string | none | Register the model under this id and return the id; without it, `fit` returns a model blob. The id can instead be given as a leading positional argument, `fit('id', ...)`; supplying it both ways raises `PREDICT_ERR_OPTIONS`. | ## predict From 24b4c004b0a075bed2296e4b8d863f10f5e91076 Mon Sep 17 00:00:00 2001 From: mstrathman Date: Sun, 2 Aug 2026 19:05:08 -0700 Subject: [PATCH 4/4] fix: address review on the leading-name change - Reject a model name with an embedded NUL byte (PREDICT_ERR_OPTIONS): it would otherwise be truncated by the mprintf/strcmp path and register a different id than the caller passed. Adds a char(97,0,98) regression test. - Add an adversarial test that a leading name varying within an aggregate group (a CASE over a column) fails loud, exercising the group-constancy contract. - Correct the header comment: a trailing '{...}' is options only once a feature and a label precede it (argc - has_name >= 3); at the lowest arity it is the label. - Docs: state both registration forms (leading id argument or the register option) explicitly, and describe the label as the last argument before the optional options. Co-Authored-By: Claude Opus 4.8 --- README.md | 2 +- predict-tabular.c | 15 ++++++++++-- tests/test_fit.py | 24 +++++++++++++++++++ .../src/content/docs/reference/functions.md | 3 ++- website/src/content/docs/reference/options.md | 4 ++-- 5 files changed, 42 insertions(+), 6 deletions(-) diff --git a/README.md b/README.md index 703f794..78ae972 100644 --- a/README.md +++ b/README.md @@ -159,7 +159,7 @@ skills-capable agent, or read them as the condensed operator's manual. | `forecast(ts, value, horizon [, options])` | Where is this metric going? | an aggregate over your rows: forecast steps with prediction intervals, one JSON document per group | | `detect_anomalies(ts, value [, options])` | Which points are abnormal? | an aggregate over your rows: per-point anomaly probability and interval, one JSON document per group | | `backtest(ts, value, horizon [, options])` | How accurate is the model here? | an aggregate over your rows: per-fold MAE, RMSE, MASE, sMAPE and coverage from rolling-origin evaluation | -| `fit([name,] f1, ..., fN, label [, options])` | Train a model on labeled rows | an aggregate over your rows: a native tabular student, named by a leading id (or `register`) or returned as a blob | +| `fit([name,] f1, ..., fN, label [, options])` | Train a model on labeled rows | an aggregate over your rows: a native tabular student, registered under a leading id argument or a `register` option, or returned as a blob | | `predict(model, f1, ..., fN [, options])` | Classify or regress a row | a scalar: one prediction per row from a fitted student | | `distill_predict(train_query [, options])` | Compress a slow teacher into a fast student | a tiny native model (decision tree, gradient-boosted forest, or MLP), registered and served by `predict` | | `distill_forecast(train_query [, options])` | Compress a forecast foundation model into a fast student | a native DLinear/TiDE forecast net (a linear skip plus a small residual), registered and served by `forecast` | diff --git a/predict-tabular.c b/predict-tabular.c index ff9b0e2..d223992 100644 --- a/predict-tabular.c +++ b/predict-tabular.c @@ -862,8 +862,10 @@ static sqlite3_module predictModule = { * instead be given as {"register":"my-id"}, but supplying it both ways is an * error. With a name fit() returns the registered id text; with neither it * returns the serialized model blob. The optional trailing options is a TEXT JSON - * object; a trailing '{...}' is therefore always read as options, so a class label - * must not itself be a JSON-object string. + * object; once a feature and a label precede it (argc - has_name >= 3) a trailing + * '{...}' is read as options, so a class label at that arity must not itself be a + * JSON-object string. At the lowest arity (a single feature and the label, as in + * fit(x, '{}')) the trailing object is the label. * predict(model, f1, ..., fN [, options]) serves that student per row, features * positional, deserialized once and cached on the model argument. * ====================================================================== */ @@ -970,6 +972,15 @@ static void fit_step(sqlite3_context *ctx, int argc, sqlite3_value **argv) { c->err = SQLITE_NOMEM; return; } + /* The name flows through mprintf("%s") and strcmp below, so an embedded NUL + * would silently truncate it to a different id than the caller passed. Reject + * it rather than register/return the wrong name. */ + if (memchr(name_txt, '\0', (size_t)sqlite3_value_bytes(argv[0]))) { + c->err = SQLITE_ERROR; + c->errmsg = sqlite3_mprintf("%s: model name must not contain a NUL byte", + PREDICT_ERR_OPTIONS); + return; + } } /* This row's trailing options: the last argument when it is TEXT beginning with diff --git a/tests/test_fit.py b/tests/test_fit.py index 66ad5ea..aada2a2 100644 --- a/tests/test_fit.py +++ b/tests/test_fit.py @@ -94,6 +94,30 @@ def test_fit_leading_name_still_needs_features_and_label(db): assert "PREDICT_ERR_SCHEMA" in str(e.value) +def test_fit_leading_name_varying_within_group_fails_loud(db): + """The leading name, like the options object, must be constant within an + aggregate group. A first TEXT argument that evaluates to different names + across rows fails loudly (PREDICT_ERR_OPTIONS) rather than silently binding + to whichever row SQLite happened to visit first.""" + _seed(db) + with pytest.raises(sqlite3.OperationalError) as e: + db.execute( + "SELECT fit(CASE WHEN tenure < 10 THEN 'lo' ELSE 'hi' END," + " tenure, spend, churned) FROM h").fetchall() + assert "PREDICT_ERR_OPTIONS" in str(e.value) + + +def test_fit_leading_name_rejects_embedded_nul(db): + """A model name with an embedded NUL would be truncated by the C-string path, + registering and returning a different id than the caller supplied. Reject it + loudly (PREDICT_ERR_OPTIONS) instead of silently binding to the prefix.""" + _seed(db) + with pytest.raises(sqlite3.OperationalError) as e: + db.execute("SELECT fit(char(97, 0, 98), tenure, spend, churned)" + " FROM h").fetchall() + assert "PREDICT_ERR_OPTIONS" in str(e.value) + + def test_fit_leading_name_json_object_still_options(db): """The leading name discounts the options arity, so a trailing JSON-object argument is consumed as options with or without a name: fit('id', f1, f2, obj) diff --git a/website/src/content/docs/reference/functions.md b/website/src/content/docs/reference/functions.md index 5c2d8a0..257e9b8 100644 --- a/website/src/content/docs/reference/functions.md +++ b/website/src/content/docs/reference/functions.md @@ -68,7 +68,8 @@ SELECT avg(mae) FROM backtest_rows( ### `fit([name,] f1, ..., fN, label [, options])` Trains a native tabular student over your rows. The features are positional and -the **label is the last positional argument** (there is no `target` option). An +the **label is the last argument before the optional `options`** (there is no +`target` option). An optional **leading TEXT argument names and registers the model**, mirroring `predict(model, ...)` so the train and serve calls read in parallel: `fit('churn', f..., label)` then `predict('churn', f...)`. The same name can diff --git a/website/src/content/docs/reference/options.md b/website/src/content/docs/reference/options.md index bc876bb..3fcea64 100644 --- a/website/src/content/docs/reference/options.md +++ b/website/src/content/docs/reference/options.md @@ -49,8 +49,8 @@ carry the columns and `GROUP BY` carries the series split. ## fit `fit([name,] f1, ..., fN, label [, options])` trains a native tabular student -over your rows; the label is the last positional argument, so there is no -`target` option. An optional leading TEXT argument names and registers the +over your rows; the label is the last argument before the optional `options`, so +there is no `target` option. An optional leading TEXT argument names and registers the model, `fit('id', f..., label)`, mirroring `predict('id', f...)`. The leading name and the options object must be constant within an aggregate group.