Skip to content

Float-precision-aware cell change detection in the data-diff grid (DRC-3025)#1458

Closed
danyelf wants to merge 7 commits into
mainfrom
explore/drc-3025-float-precision
Closed

Float-precision-aware cell change detection in the data-diff grid (DRC-3025)#1458
danyelf wants to merge 7 commits into
mainfrom
explore/drc-3025-float-precision

Conversation

@danyelf

@danyelf danyelf commented Jul 4, 2026

Copy link
Copy Markdown
Contributor

The data-diff grid no longer flags float-precision noise like 0.1+0.2 vs 0.3 as a change — including floats nested inside JSON/array/struct cells — so users stop chasing phantom diffs.

What changed

  • Rebuild isCellChanged on lodash _.isEqualWith with a float-leaf comparator: a magnitude-relative epsilon applied at every numeric leaf, scalar and nested.
  • Route the live diff grid — row status, counts, the changedOnly filter, and side-by-side cell highlighting — through it, replacing raw _.isEqual.
  • Keep the near-zero absolute floor; strings/booleans/mixed-types/null/NaN stay exact.
  • Add float-noise regression tests across the live grid paths.

Evidence

  • Frontend suite: 4081/4081 passed (5 skipped), independently re-run at validation and adversarial review.

Review guidance

  • isCellChanged is the single source of truth for value diffs; the epsilon is a private leaf comparator, no longer exported.

Audit: DRC-3025 Float-Precision Rework · entity fg (tracked in local spacedock docs)
Closes DRC-3025

danyelf and others added 7 commits July 3, 2026 14:14
Profile diff flagged numeric cells that differed only in float noise
(e.g. 3.14159000001 vs 3.14159). Route all three comparison sites through
one shared helper isCellChanged(base, current, columnType?, renderMode?):

- formatted modes compare at one digit BELOW displayed precision (N+1)
- raw mode applies a relative epsilon (FLOAT_RELATIVE_EPSILON = 1e-9)
- non-numeric keeps exact _.isEqual; one-sided null/NaN counts as changed

Wired getCellClass (was !_.isEqual), determineRowStatus (was !_.isEqual),
and inlineRenderCell (was ===). getCellClass/determineRowStatus gained
optional render-mode params (backward compatible). +32 TDD tests.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: Danyel Fisher <danyel@gmail.com>
…3025)

Rework the float-precision fix to a simpler, corrected approach per Danyel's
review. These are floating-point representation errors, so drop the
display-precision-relative comparison entirely and use one flat tolerance in
ALL render modes.

- Simplify isCellChanged(base, current) — no columnType / renderMode params:
  two finite numbers compare with |a-b| <= EPS*max(|a|,|b|) (EPS=1e-9) plus a
  1e-12 absolute floor near zero; everything else keeps exact _.isEqual.
- Revert getCellClass and determineRowStatus to their original signatures
  (remove the columnRenderMode threading). This dissolves the cloud-side open
  question — the change is now self-contained in OSS.
- Remove displayedDecimals / toComparableNumber / numericChanged / the
  NUMERIC_COLUMN_TYPES set.
- inlineRenderCell stays wired to the helper.
- Tests rewritten to render-mode-independent cases (0.1+0.2 vs 0.3, near-zero
  floor, large-magnitude scaling, non-numeric exact). Removed the old
  precision-relative / render-mode tests.

Full @datarecce/ui suite green (4032 passed / 5 skipped), type:check + Biome
lint clean.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: Danyel Fisher <danyel@gmail.com>
Wire the float-precision-aware isCellChanged comparison into the live
diff-grid render path, which previously still used raw _.isEqual:

- detectModifications (rowBuilders.ts) drives row __status="modified",
  rowStats.modified, and the changedOnly filter for BOTH inline and
  side-by-side modes.
- createCellClassBase/createCellClassCurrent (toDiffColumn.tsx) drive
  side-by-side red/green cell highlighting.

Float-noise-only rows (e.g. 0.1+0.2 vs 0.3) no longer read as modified
or get highlighted. Adds tests over both surfaces with float-noise inputs.

Co-Authored-By: Claude <noreply@anthropic.com>
Signed-off-by: Danyel Fisher <danyel@gmail.com>
Rework isCellChanged onto _.isEqualWith with a floatComparator customizer so
the magnitude-relative epsilon is applied at every numeric leaf, including
floats nested inside object/array cells (JSON/ARRAY/STRUCT columns). The old
top-level typeof===number gate only tolerated scalar float noise and compared
nested floats exactly, leaving the noise bug live for structured cells.

Internalize FLOAT_RELATIVE_EPSILON: the customizer closes over it, so it is now
module-private -- drop its export in gridUtils.ts and remove it from both
barrels (utils/dataGrid/index.ts, utils/index.ts). It had zero consumers.

Add nested-float regression tests: object/array/deeply-nested cells differing
only by float noise read unchanged, genuine nested and non-numeric changes
still read changed.

Co-Authored-By: Claude <noreply@anthropic.com>
Signed-off-by: Danyel Fisher <danyel@gmail.com>
…oat diff (DRC-3025)

Anchors the fix on the DRC's ACTUAL reported scenario, not just the
0.1+0.2-vs-0.3 scalar proxy: a profile_diff of a table that did not change must
not surface phantom 'modified' rows when a stat column (avg/mean) differs only
in its raw floating-point tail.

Drives the real production pipeline end to end via createDataGrid: profile_diff
-> toDataDiffGrid -> buildDiffRows -> isCellChanged. A float-noise-only stat row
reads __status !== 'modified' in both inline and side-by-side modes; a genuine
stat change still reads modified (control). Raw stat values are compared, so
display precision (2 vs 5 decimals) is irrelevant by construction.

Verified test-first: reverting isCellChanged to raw _.isEqual turns the two
noise assertions red (row reads 'modified'); the isEqualWith fix turns them
green. Full suite green (4085).

Co-Authored-By: Claude <noreply@anthropic.com>
Signed-off-by: Danyel Fisher <danyel@gmail.com>
…how no phantom float diff (DRC-3025)

The DRC's real scenario: a profile diff of a table that did NOT change flags an
avg cell as modified even though both sides display the same value. Root cause
is in the BACKEND, not the grid: agate NUMBER aggregates arrive as Decimal, and
from_agate kept finite Decimals as-is; pydantic v2 then serializes Decimal as a
JSON *string*. So the stat reached the frontend as "0.30000000000000004" vs
"0.3" — two unequal strings — and the grid's numeric epsilon (isCellChanged)
only tolerates float noise between NUMBERS, so the row read modified.

A numeric aggregate arriving as a string is itself the bug, so fix it at the
earliest correct layer: from_agate now coerces finite Decimal values in
NUMBER-typed columns to float, so the stat is a real number end-to-end and the
epsilon applies naturally. These stats are shown at 2-5 decimals, so float64 is
the correct display-precision wire type. INTEGER columns are intentionally left
untouched (already ints; exact-compare, no float noise; avoids >2**53 precision
loss). Non-finite Decimals still coerce to float (GitHub #476), unchanged.

Tests:
- Backend (test_dataframe_number_serialization.py): NUMBER Decimals serialize as
  JSON numbers; a 0.1+0.2 vs 0.3 pair is numerically within epsilon; genuine
  change still differs; non-finite still float; INTEGER unaffected. Proven
  test-first (reverting the coercion turns the NUMBER cases red with str values).
- Frontend (createDataGrid.profile.test.tsx): re-anchored on both wire shapes —
  the post-fix NUMBER shape reads UNCHANGED through the real createDataGrid ->
  toDataDiffGrid -> isCellChanged path (inline + side-by-side), and the pre-fix
  STRING shape still reads MODIFIED, documenting why the fix must live in the
  backend (the frontend does not numerically compare stringified numbers).

Full suites green: backend 1552 passed/5 skipped; frontend 4085 passed/5 skipped.

Co-Authored-By: Claude <noreply@anthropic.com>
Signed-off-by: Danyel Fisher <danyel@gmail.com>
…ta exact (DRC-3025)

Cycle-4 put the NUMBER Decimal->float coercion in the SHARED DataFrame.from_agate,
which also feeds query/query_diff/value_diff. There a NUMBER column is a raw
warehouse DECIMAL/NUMERIC *data* value, so coercing to float64 masked genuine
high-precision changes: e.g. 12345678901234567.89 vs ...90 collapse to the same
float and read UNCHANGED (a real change silently hidden + the displayed value
corrupted), where pre-fix they serialized as distinct exact strings and read
MODIFIED. That is a false-negative on the core data-diff function.

Fix: restore from_agate to its exact behavior (finite Decimals kept as-is; only
non-finite -> float per GitHub #476), and move the coercion into the profile
path (recce/tasks/profile.py "_coerce_number_stats_to_float"), applied to all
three profile build sites. Only profile stats — computed aggregates where float
representation noise is meaningless — become floats; query/query_diff/value_diff
NUMBER data keeps its exact Decimal value and compares exactly, exactly as before
cycle-4. INTEGER-exact and non-finite/#476 behavior preserved.

Tests:
- Backend (test_dataframe_number_serialization.py): profile stats serialize as
  JSON numbers via the profile helper (float-noise within epsilon; genuine change
  differs; non-finite float; INTEGER untouched) AND a high-precision DECIMAL data
  change stays distinct exact strings through from_agate (reads MODIFIED, not
  masked). Regression guard proven test-first: re-adding the coercion to from_agate
  turns the two data-exactness tests RED while profile tests stay green.
- Frontend: comment re-pointed to the profile path; assertions unchanged.

End-to-end seam re-verified both ways: unchanged-table profile avg -> UNCHANGED;
high-precision data change -> MODIFIED. Suites green: backend 1554 passed/5 skipped;
frontend 4085 passed/5 skipped.

Co-Authored-By: Claude <noreply@anthropic.com>
Signed-off-by: Danyel Fisher <danyel@gmail.com>
@danyelf

danyelf commented Jul 8, 2026

Copy link
Copy Markdown
Contributor Author

Closing without merge. DRC-3025 has been rediagnosed: the root cause is that numeric Decimals serialize to the frontend as untyped JSON strings, not the profile float-noise this PR addressed. The profile-scoped float coercion here is superseded by a new entity that fixes numeric type serialization at the DataFrame layer (see the reframed DRC-3025 description). The epsilon-comparison logic will be migrated forward into that work.

@danyelf danyelf closed this Jul 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant