Skip to content

v0.8.12

Choose a tag to compare

@github-actions github-actions released this 05 Sep 13:51
· 20 commits to master since this release
7399066
  • Binary pr_auc scored the wrong class on {1, n} labels (OC-146, also
    closes OC-37):
    _add_roc_pr_auc_metrics called average_precision_score
    without pos_label, and that function defaults to pos_label=1 — unlike
    roc_auc_score, which infers the positive class from the sorted uniques.
    With labels {1, 2} or {1, 5} (survey exports, Likert-derived targets,
    R-style factor codes, 1=no, 2=yes CSVs) the literal 1 is the negative
    class while proba[:, 1] is P(classes_[1]), so PR-AUC was computed for
    the inverted problem: 0.32 reported for a model whose true PR-AUC is
    0.97
    , with no warning. For every other non-{0,1} label set (strings,
    {2,3}, {0,2}, {10,20}) sklearn raised instead and the metric silently
    vanished from the report. The same report then contradicted itself on
    screen: the PR curve is built with pos_label=classes[1], so a chart
    hugging the top-right corner carried "0.32" beside it. The binary branch now
    resolves the positive class from model.classes_, the pattern
    _add_binary_unweighted_metrics and the curve builder already used.
    pr_auc is now identical across {0,1}, {1,2}, {1,5}, {-1,1} and
    {"no","yes"} encodings of the same data. Added 7 regression tests.

  • Threshold tuning picked a pathological cutoff on single-class validation
    (OC-36):
    _grid_search_binary broke score ties with a strict >, keeping
    whichever candidate the scan reached first. When the validation split held
    only one class, every candidate scored identically, so tuning returned the
    first grid point — {0: 0.990, 1: 0.0098} — which classified almost
    everything positive (measured: 49/50 rows at F1 0.0) and was persisted as a
    tuned threshold. Metrics like F1 are piecewise constant in the cutoff, so
    tied plateaus are routine even on healthy splits, and the same tie-break
    pinned whichever plateau edge came first. Ties now break toward the default
    0.5 cut; a single-class split warns and keeps {0.5, 0.5}; NaN scores
    from a caller-supplied metric are still skipped. _tune_decision_thresholds
    gains a matching gate so a degenerate split leaves
    decision_thresholds=None (predictions keep the model's default rule)
    instead of reporting a threshold nothing was tuned against. Documented in
    docs/user_guide/threshold_tuning.md. Added 4 regression tests.

  • optimize_thresholds' binary result bypassed its own documented decision
    rule (OC-147):
    apply_thresholds documents "predicts the positive class
    when y_proba[:, 1] >= threshold" and special-cased a bare float and a
    one-entry dict — but _grid_search_binary returns a two-entry dict,
    which fell through to the multiclass scaled argmax, where np.argmax breaks
    exact ties toward the first column. That silently turned >= into a strict
    >, so the search scored tied rows as positive while apply-time scoring
    counted them negative and the tuned score was not quite the score you get.
    Reachable in practice: the grid includes 0.5 and decision trees routinely
    emit p1 of exactly 0.5. The two-class case now compares the scaled
    scores directly, keeping >= authoritative — verified identical to argmax
    on every non-tied row for complementary and non-complementary threshold
    pairs, so a user-saved set still has both entries honored. Added 2
    regression tests.

  • The profiler reported NaN statistics — and a silently wrong median — for
    any float column containing a NaN (OC-39):
    polars keeps NaN distinct from
    null, so EDAAnalyzer's fill_null-based cleaning was a no-op on it.
    Aggregations then propagated the NaN instead of skipping it: std,
    skewness and kurtosis came back NaN, the histogram builder died with
    ComputeError: breaks cannot be NaN (its min_val == max_val guard cannot
    see that nan == nan is False), and worst of all the median was simply
    wrong
    [1, 2, nan, 4] reported 3.0 where pandas reports 2.0, with
    no error and no warning. Nothing caught this because tests build frames with
    pl.from_pandas, which converts NaN to null on the way in; the bug is only
    reachable from pl.read_csv (literal NaN tokens) or direct construction.
    Fixed once at the boundary rather than at six call sites: a _nan_to_null
    pass in EDAAnalyzer.__init__ rewrites NaN to null in every float column, so
    downstream type checks, aggregations and cut() all see the missing-value
    semantics the rest of the profiler already assumed. Added 6 regression tests.

  • Quartiles, skewness and kurtosis used polars' estimator defaults, not
    pandas' (OC-41, OC-42):
    three silent divergences in the same expression
    list. Expr.quantile() defaults to interpolation="nearest" where
    pandas/NumPy use linear, so Q1/Q3 were reported as actual observed data
    points rather than interpolated positions — and _compute_boxplot_stats
    disagreed with NumericStats about the same column's hinges. skew() and
    kurtosis() default to bias=True where pandas reports the bias-corrected
    estimator, and polars' kurtosis() is not Fisher excess kurtosis by default
    while pandas' .kurt() is. The skewness error was user-visible, not just
    numerical: [1, 2, 3, 4, 10] reports 1.30 biased against 1.70 unbiased,
    so the "High skewness — consider a transform" recommendation stayed silent on
    a column that clears the 1.5 threshold. Both quantile sites now pin
    interpolation="linear", and skew/kurtosis pin bias=False with
    fisher=True. Added 6 regression tests (shared with OC-39).

  • PCA and clustering "mean imputation" was actually zero-filling NaN (OC-40):
    _impute_matrix called fill_null(strategy="mean"), which — same root cause
    as OC-39 — is a no-op on NaN, so the values fell through to the
    np.nan_to_num(nan=0.0) guard. Components were fitted with 0.0 exactly
    where SimpleImputer would have put the column mean, which for a
    mean-centered feature is the most distorting value available. Its sibling
    _impute_matrix_drop_empty already normalized first; _impute_matrix now
    mirrors it. Added 2 regression tests.

  • Correlation dropped entire columns and returned nothing at all (OC-43):
    calculate_correlations used drop_nulls() — which keeps NaN rows — and
    then DataFrame.corr(), which is listwise: a single surviving null made
    the whole matrix NaN, the broad except swallowed it, and the profile
    silently lost its correlation section. Two fixes: NaN is normalized to null
    up front, and the matrix is now built pairwise with pl.corr (which
    already does pairwise deletion, matching pandas' .corr()), so a column with
    a few gaps correlates on its overlapping rows instead of taking the matrix
    down with it. Pairs with fewer than 3 overlapping observations are reported
    as 0.0 with a single aggregated warning, because at n=2 a Pearson r is
    always exactly ±1.0 — a schema limitation, since CorrelationMatrix.values
    is list[list[float]] and has no "unknown" cell. Added 4 regression tests.

  • Wasserstein drift reported a number its own threshold contradicted
    (OC-44):
    the drift decision has always been made on the distance normalized
    by the reference standard deviation, but value carried the raw distance in
    the column's own units. A large-scale column that barely moved therefore
    reported wasserstein_distance=50.0, threshold=0.1, has_drift=False — and
    every consumer that re-derives the verdict from value inherited the
    contradiction: the UI's threshold sliders flipped it to "drifted", the CSV
    export published 50.0, the alert modal showed it beside a 0.1 threshold, and
    _build_drift_column_summary persisted it into drift_check_results.summary
    where it is later compared against threshold_wasserstein. The published
    example notebook shows the failure outright — 784.7832 (Thresh: 0.1) [PASS]. Rather than teach each consumer a metric-specific exception, the
    invariant now lives in the schema: value is the number threshold applies
    to for every metric that decides its own verdict, and the untransformed
    distance moved to a new raw_value. A constant reference has no scale to
    normalize by and falls back to the raw distance rather than emitting inf.
    The one documented exception is ks_test_p_value, which is diagnostics only
    and borrows the KS statistic's threshold. No backend change was needed —
    EnrichedDriftReport.column_drifts is dict[str, Any]. Added 5 core, 1
    backend and 3 frontend regression tests.

  • Schema drift was never counted as drift (OC-45):
    drifted_columns_count was built from the per-column metric flags alone, so
    a feature that vanished between training and production left it at 0
    while _classify_drift_severity classified that same report "critical",
    and the drift-status dashboard counts drifted jobs by that field, so a
    critical job was reported as having no drift. The count now includes
    missing_columns and new_columns; the frontend's client-side
    re-evaluation adds them too, since rebuilding the count from metric flags
    alone dropped them the moment a slider moved. Added 2 core and 2 frontend
    regression tests.

  • A profile could serialize an invalid NaN token (OC-46):
    NumericStats accepted NaN and infinity into its float fields.
    backend/eda/tasks.py persists the profile via
    profile.model_dump(mode="json"), which retains a Python nan, and stdlib
    json.dumps then emits a bare NaN — not valid JSON, and rejected by the
    browser's JSON.parse, so the whole EDA result failed to load. The ten
    optional float fields now run through a FiniteFloat annotated type that maps
    any non-finite float to None before validation. BoxPlotStats was left
    alone: its fields are required, and making them optional would be a real
    contract change rather than a fix. Added 1 regression test (shared with
    OC-39).

  • Every cross-origin error response was invisible to the frontend (S8414,
    SonarCloud BLOCKER):
    add_middleware wraps, so the middleware added last
    is outermost. _add_middleware added TrustedHost → CORS → Logging →
    ErrorHandler, which put CORS inside ErrorHandlerMiddleware. Any
    exception that handler converted to a JSON error response was therefore
    produced outside the CORS layer and carried no Access-Control-Allow-Origin
    header, so the browser blocked the frontend from reading the status or body
    of every cross-origin 4xx/5xx — the exact responses a UI most needs to
    explain. Successes worked, which is why this survived: only failures were
    opaque, and they presented as generic network errors. CORS is now added last
    (verified outermost: CORSMiddleware → ErrorHandlerMiddleware → LoggingMiddleware → TrustedHostMiddleware), with a comment recording the
    wrap-order invariant so it is not "tidied" back.

  • Ten integration tests reported assertion failures with an empty reason
    (S5779 ×10):
    tests/integration/test_frontend_nodes.py (9 sites) and
    tests/unit/verify_polars_preprocessing.py (1) wrapped their assertions
    inside try: ... except Exception as e: pytest.fail(str(e)). pytest.fail
    raises Failed, which derives from OutcomeException(BaseException) — not
    Exception — so it does propagate out of the handler and the tests do fail.
    The damage was quieter: when the failing assertion was a bare
    assert result.status == "success", str(AssertionError()) is '', so the
    suite failed with no message at all, and pytest's assertion-introspection
    diff was lost because the assert ran inside a function call frame. A node
    contract regression in this suite — the one guarding every canvas node type —
    was therefore indistinguishable from an engine crash. Each block now keeps
    only the call under test inside the try and asserts after it. Four stale
    "thinking out loud" comments were replaced with one accurate note about where
    node data actually lands (the artifact store, not the return value).

  • kmeans-strategy binning was not reproducible run-to-run (S6709 ×2):
    KBinsDiscretizer(strategy="kmeans") runs k-means internally, and both
    _fit_kmeans and _fit_kbins in preprocessing/bucketing.py constructed it
    without random_state — so the same data could yield different bin edges on
    each fit, breaking the library's stated reproducibility guarantee and
    invalidating SkyulfPipeline.fingerprint() as a seal (fitted edges feed the
    digest, so an identical pipeline hashed differently across runs). Both now
    inject DEFAULT_RANDOM_STATE from skyulf/types.py, the single owner of the
    seed, matching preprocessing/split.py. Two further S6709 hits in
    profiling/_analyzer/multivariate.py were already fixed on this branch.

  • _first_finite could return infinity (S1764): the helper in
    _execution/summary.py filtered non-finite values with if f == f, which
    rejects NaN but happily accepts ±inf — so a metric of inf (a divide-by-zero
    in a ratio metric, or an unbounded loss) was rendered into the node summary
    shown on canvas cards and in the pipeline diagram. The name promised finite;
    the check delivered not-NaN. Now math.isfinite(f), in both _first_finite
    and its unflagged sibling _train_only, which shared the same idiom. A
    second S1764 in data_ingestion/serialization.py (obj != obj) was already
    narrowed to float and is exactly equivalent, but now reads as
    math.isnan(obj). The leak was reachable in three distinct renderings, all
    now pinned: test_accuracy=inf printed acc inf instead of falling through
    to the next finite candidate; an all-infinite metric set printed
    acc inf · f1 -inf instead of no headline; and an infinite train_accuracy
    made the overfit gap inf - 0.80 = inf, which fails the diff < 0.05 guard
    and printed acc 0.80 · ▲inf. Added 4 regression tests (3 locking those
    down, 1 pinning the NaN rejection that already worked, since isfinite is a
    strictly wider filter).

  • User-controlled identifiers were interpolated raw into log lines (S5145 ×3,
    CWE-117):
    job_id (an unvalidated str path param on POST /deploy/{job_id}) in ml_pipeline/deployment/service.py, the same in
    monitoring/router.py, and file_path in data_ingestion/service.py all
    reached a logger without sanitization. A value containing CR/LF forges
    additional log lines, which is how log-injection attacks hide an intrusion or
    frame another user. Added a shared sanitize_for_log helper in
    backend/utils/logging_utils.py that escapes the C0 control block plus DEL to
    a visible \xNN form — escaping rather than deleting, so the attempt stays
    visible in the record. SonarCloud under-reported the rule: two of the
    three flagged statements also interpolated a second tainted value it did not
    report, so sanitizing only the reported ones would have left them forgeable —
    artifact_uri in deployment/service.py is always str(db_job.node_id), and
    node_id reaches the DB from the client-submitted graph through the
    NodeConfig dataclass, which declares it as a bare str with no validation;
    and source_id in data_ingestion/service.py is declared
    delete_source(source_id: str) at the router, not an int. All five values
    are now sanitized. A fourth S5145 on eda/router.py:145 is a false positive:
    that param is an int, validated by the framework.

  • Two async pipeline routes blocked the event loop on disk I/O (S7493 ×2):
    the JSON save and load paths in _internal/_routers/pipelines_io.py used
    synchronous open()/json.load/json.dump inside async def handlers,
    stalling every other in-flight request on that worker for the duration of the
    read/write. Both now use aiofiles with await, matching the convention
    already established at data_ingestion/service.py:441.

  • CodeQL kept calling the pipeline JSON path injectable
    (py/path-injection ×2, High):
    _pipeline_json_path already rejected any
    dataset_id outside ^[A-Za-z0-9_-]+$ before constructing a Path. That
    allowlist is sufficient on its own — the charset excludes ., both separators
    and NUL, so the join appends exactly one segment and storage_dir cannot be
    escaped. But CodeQL does not model a character allowlist as a sanitizer, so
    the aiofiles.open sinks in /save/{dataset_id} and /load/{dataset_id}
    stayed reported scan after scan, even though d0c02371 had added that very
    allowlist to close this finding. Added the containment layer the rest of the
    backend already uses (LocalFileConnector.resolve_safe_path,
    LocalArtifactStore._get_path,
    ArtifactFactory._resolve_local_artifact_path): resolve the joined path and
    require it to be is_relative_to the resolved storage root. Verified with
    layer 1 disabled — widening the regex to .* still rejects ../../evil,
    ../evil and ..\..\evil with HTTP 400 and writes nothing outside the
    storage dir. The helper now returns an absolute path; the default relative
    PIPELINE_STORAGE_PATH (exports/pipelines) resolves against the process CWD
    exactly as before, only earlier.

  • Selected text ran underneath the dropdown arrow in every <select>
    (css:S4657, CRITICAL):
    styles/components.css declared padding-right: 2.5rem to clear the custom arrow background-image, then padding: .55rem .8rem below it — and the shorthand resets all four sides, silently
    discarding the longhand above it. Every select using this rule had its right
    padding collapse to .8rem, so text overlapped the arrow — and that is all
    110 <select> elements in the codebase, since no other select rule in
    src/styles/ declares padding and none of the 110 carries a Tailwind p*-n
    utility that would outrank it. The longhand is now ordered after the
    shorthand, with a comment recording why the order matters. Separately, a dead
    color: #f8fafc in styles/layout.css (css:S4656) was removed from
    .feature-canvas-navbar__brand--gradient: the later color: transparent
    always won at equal specificity, so it never acted as the
    background-clip: text fallback it appears to have been written for — a real
    fallback needs @supports.

  • Three number-matching regexes froze the tab for ~2 seconds on long input
    (S8786 ×3):
    InferencePage.tsx's NUMERIC_RE and two of the
    syntax-highlight patterns in JobDetailsView.tsx all used the shape
    \d+\.?\d* — two digit quantifiers separated by an optional dot, so on a
    failing long digit run the engine can split the run between them in O(n) ways
    and backtracks quadratically. Measured on a 60,001-char input:
    1723.74 ms → 0.11 ms after the rewrite (a repeat run gave 2029.77 ms →
    0.09 ms), four orders of magnitude. This is user-reachable, not theoretical:
    InferencePage runs NUMERIC_RE over pasted CSV cell values, so a long
    numeric-looking paste hung the main thread per cell. Rewritten as
    \d+(?:\.\d*)?, which accepts the identical language (re-verified over 21
    inputs, zero mismatches) but is linear, since digits after a dot become
    reachable only through the now-mandatory dot. A fourth S8786-shaped pattern
    (\d+\.\d+) was left alone — a mandatory dot is already unambiguous.

  • Converted 62 global number coercions to their Number.* statics across 27
    frontend files (S7773 ×62):
    41 parseFloat, 11 parseInt, 7 isNaN,
    2 isFinite, 1 bare NaN. Cosmetic in intent, but this needed auditing
    rather than a blind find/replace: Number.parseInt/Number.parseFloat/
    Number.NaN are the identical objects and values (always safe), whereas
    Number.isNaN/Number.isFinite do not coerceisNaN("abc") is true
    but Number.isNaN("abc") is false, so a naive sweep over a string argument
    silently changes behaviour. All 9 coercion-sensitive sites were checked
    individually; every argument was already statically a number (a Number(...)
    wrapper, a parseFloat return, a Math.max/min result, or a
    typeof x === 'number' narrowing), so all 62 convert with zero behaviour
    change and none needed the Number.isNaN(Number(x)) coercion-preserving form.
    The codebase already used the statics extensively, so this removes an
    inconsistency rather than adding one.

  • Two smaller correctness cleanups: (job.metrics or {}).get("summary") if job.metrics else None in _execution/jobs.py had a dead or {} branch — the
    guard already proved metrics truthy, so the fallback was unreachable
    (S2583); and np.where(preds == -1)[0] in
    profiling/_analyzer/multivariate.py became np.nonzero(preds == -1)[0], the
    direct single-argument form that does not build and discard a tuple (S6729).

  • Triage record: all 118 open master issues were read in the source tree
    and verified — running code where a verdict hinged on runtime behaviour —
    giving 89 genuine / 29 false positive. Findings, per-finding evidence, and
    the false positives worth marking upstream are in
    initiatives/analysis/sonarcloud-master-issues.md.
    Notably the 6 python:S5863 "identical assertion arguments" hits are not
    copy/paste slips: five are deliberate determinism and idempotence assertions
    on artifact_digest (two distinct empty classes digest differently, so
    "strengthening" them would be wrong) and one is the canonical x != x NaN
    test. Only the NaN site got a clarity rewrite to pd.isna.