Uh oh!
There was an error while loading. Please reload this page.
docs: PreprocessingConfig knobs — fix stale claims + document the six levers - #65
Conversation
… + document the six levers (backend#1189) how-training-works: the header claim, tabular imputation/scaling steps, TTE bullet, and TSF preprocessing now describe the knob-driven behaviour (strategy enum incl. iterative; knn/QuantileTransformer privacy exclusions; forecasting ffill). hyperparameters: new Preprocessing section with all six knobs, the disable-imputation guidance for BYO/native-NaN models, and the privacy note. Content-gated: merge only after the engine promotion carries RFC-0003 L2/L1b/L3b (+ the open L3a/indicator PRs) to prod images. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Preview deployment for your docs. Learn more about Mintlify Previews.
💡 Tip: Enable Workflows to automatically generate PRs for you. |
Preview deployment for your docs. Learn more about Mintlify Previews.
💡 Tip: Enable Workflows to automatically generate PRs for you. |
LukasWodka
commented
Jul 23, 2026
👋 Heads-up — Code review queue is at 32 / 30 Above the WIP limit. The team convention is to review existing PRs before opening new work. Open PRs currently in Code review (oldest first):
Pull from review before opening new work. (This is a nudge from the kanban WIP check, not a block.) |
shujaatTracebloc
left a comment
There was a problem hiding this comment.
Review — verified against source, not just the PR body
I checked the documented behaviour against the real code in tracebloc-engine, tracebloc-py-package, and backend (branch presence included). The content is accurate and well-written; the only real issue is timing/merge-safety.
✅ Merge gate is honest — every claim checks out
| Gate claim | Reality | Status |
|---|---|---|
| engine #474/#475/#476/#499 "on develop only" | All 4 merged to develop; preprocessing code absent from origin/master | ✅ correctly unchecked |
| engine#507 (TSF imputation) | Still OPEN — not even on develop (only on feat/505-forecasting-imputation) | ✅ correctly unchecked |
| engine#508 + SDK#335 + backend#1192 | engine#508 ✅ + backend#1192 ✅ on develop; SDK side still unmerged (on feat/472-missingness-indicators-sdk) | ✅ correctly unchecked |
| "backend#1095 already on master ✅" | True — commit b1b2b464 + migration 0487_experiment_preprocessing_knobs on origin/master | ✅ accurate |
| "SDK#287 on master ✅" | True — the five knob methods are on origin/master | ✅ accurate |
Nothing is over- or under-claimed.
✅ Technical accuracy — documented API matches the code exactly
- SDK (
training_plan):handle_missing_values,imputation_strategy(median/mean/most_frequent/constant/iterative/none),encoding_strategy(label/onehot),normalize_features,scaler— all exact matches, onmaster. Thescalerallowlist matches precisely,QuantileTransformeris rejected, and the(tabular & time-to-event)scope comment matches the real guard (TSC's scaler routes elsewhere). - Engine (
develop):constant→0.0, categorical"Unknown"/ per-column mode undermost_frequent, label never imputed, median default,iterative/MICE viaIterativeImputer,none/handle_missing_values=Falseboth disable the step, the six-scaler registry withQuantileTransformerinLEAKY_SCALERS, and the<col>__missingindicator channel — all match. The privacy framing even tracks the code comments word-for-word.
⚠️ Please address before this can sit safely
This PR is not a GitHub Draft (
isDraft: false, mergeable/clean). The "⛔ Draft until ALL of…" gate is prose only — GitHub enforces none of it, so one stray merge ships docs for behaviour that's onmasterfor nobody. Suggestgh pr ready --undoto make it an actual draft. This is the most important change.Two content pieces describe unshipped behaviour (fine only behind a real draft gate):
- The new TSF imputation paragraph → engine#507, not even on
develop. (I checkedfeat/505-forecasting-imputation: it does implement forecasting imputation matching the paragraph exactly — forward-fill in timestamp order, leading-gap fallback honouringimputation_strategy, timestamp/label never imputed — so this is content-accurate, purely a timing gate, not a mislabel of the TSC feature.) - The
missingness_indicators(True)snippet → the SDK method exists only on an unmerged feature branch; on the released SDK it raisesAttributeError.
- The new TSF imputation paragraph → engine#507, not even on
Gate under-specifies the SDK side of TSF imputation. On
masterthe SDK guard forhandle_missing_values/imputation_strategydoes not include time-series-forecasting — the TSF widening rides the same unmergedfeat/472branch. So the TSF paragraph needs the SDK guard change and engine#507. Worth adding the SDK half explicitly to gate item 2 so it isn't forgotten.
Minor (non-blocking)
- The
## Preprocessingcode block stackshandle_missing_values(False)withimputation_strategy('iterative')andnormalize_features(True)+scaler(...). The# Or …comments signal these are alternatives, but a whole-block copy-paste sets contradictory values — a one-liner ("illustrative alternatives, not a recommended combination") removes the footgun. - Anchor
#preprocessing-tabular--time-seriesis correct (double hyphen from the&collapse) and the preview built green — worth one click-test of the cross-page link in the Mintlify preview. - Spotted while verifying, out of scope here: the SDK facade
scalerdocstring (link_model_dataset.py:575) still listsQuantileTransformeras supported even though the validator rejects it — worth a small follow-up.
…review)
A whole-block copy-paste would set contradictory values
(handle_missing_values(False) + imputation_strategy('iterative'));
say so explicitly above the block. Requested in Shujaat's docs#65
review.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>LukasWodka
commented
Jul 23, 2026
Thanks for the source-verified review — all four actionable points are handled:
Minor items: anchor format confirmed as you derived it (double hyphen); will click-test in the Mintlify preview before undrafting. Your out-of-scope catch — the facade |
Uh oh!
There was an error while loading. Please reload this page.
LukasWodka
commented
Jul 23, 2026
Gate progress (2026-07-23 12:30): every code dependency is now merged — engine#507 ✅, the indicator trio (engine#508 / backend#1192 / py-package#335) ✅, and the SDK TSF-gate widening rode #335 ✅. The only remaining gate is deployment: the tracebloc-engine develop→staging→master promotion (prod images) and the SDK release cut from master. Undraft once those land. |
Closes tracebloc/backend#1189 (RFC 0003 §8/§9 rollout debt — tracebloc/backend#1091).
What
⛔ Merge gate (docs-exception content rule: never document unshipped behaviour)
Draft until ALL of:
handle_missing_values/imputation_strategyguard does not include time-series-forecasting; that widening rides tracebloc-py-package#335 (review finding)backend#1095 is already on
master✅; SDK#287 is onmaster✅.Known gap, out of scope here: the docs have no time-series-classification use-case section at all — flagged separately.
🤖 Generated with Claude Code
Note
Low Risk
Markdown-only changes with no runtime impact; main risk is documenting behavior before it is released, which the author’s merge gate addresses.
Overview
Documentation-only update for RFC 0003 preprocessing knobs: readers are told preprocessing is configurable from the notebook, not fixed platform-only behavior.
how-training-works.mdxreplaces the “not configurable from the notebook” replication note with a link to the new hyperparameters section and a reminder to match actual experiment settings. Tabular preprocessing steps 2 (imputation on/off, strategy enum, BYO-model guidance, whyknnis excluded) and 6 (scaler allowlist,QuantileTransformerexclusion) are expanded. Time series forecasting gains an imputation paragraph (timestamp-ordered forward-fill plus fitted fallbacks). Time-to-event (PyTorch) preprocessing is aligned with the same knob vocabulary as tabular.hyperparameters.mdxadds Preprocessing (Tabular & Time Series) with exampletraining_plan.*calls for all six levers, when to disable built-in imputation, privacy rationale for excluded options, and a defaults-unchanged note.The PR description flags a merge gate: do not ship until engine/SDK releases expose the documented behavior (including TSF imputation and TSC
missingness_indicators).Reviewed by Cursor Bugbot for commit 5565cbf. Bugbot is set up for automated code reviews on this repo. Configure here.