2.2.1 - Bugfixes - #155
Closed
mike-finopsorg wants to merge 25 commits into
Closed
Conversation
…ng and entity type Object. (#132) Signed-off-by: Mike Fuller <mike@finops.org> --------- Signed-off-by: Mike Fuller <mike@finops.org>
Bumped version Signed-off-by: Mike Fuller <mike@finops.org> Signed-off-by: Mike Fuller <mike@finops.org>
Signed-off-by: Mike Fuller <mike@finops.org> Signed-off-by: Mike Fuller <mike@finops.org>
Signed-off-by: Mike Fuller <mike@finops.org> Signed-off-by: Mike Fuller <mike@finops.org>
Co-authored-by: Matt Cowsert <matthew@finops.org> Signed-off-by: Mike Fuller <mike@finops.org>
Co-authored-by: Matt Cowsert <matthew@finops.org> Signed-off-by: Mike Fuller <mike@finops.org>
Co-authored-by: Matt Cowsert <matthew@finops.org> Signed-off-by: Mike Fuller <mike@finops.org>
Co-authored-by: Matt Cowsert <matthew@finops.org> Signed-off-by: Mike Fuller <mike@finops.org>
Co-authored-by: Matt Cowsert <matthew@finops.org> Signed-off-by: Mike Fuller <mike@finops.org>
Co-authored-by: Matt Cowsert <matthew@finops.org> Signed-off-by: Mike Fuller <mike@finops.org>
Co-authored-by: Matt Cowsert <matthew@finops.org> Signed-off-by: Mike Fuller <mike@finops.org>
Apply review feedback across the FOCUS 1.4 check generators: - Make every leaf generator row-condition-aware by wrapping its predicate_sql in _apply_condition, matching FormatJSON. Previously only FormatJSON did this; the predicate is read directly off the stored SQLQuery, so the condition must be baked in at construction. - Fix CheckColumnComparison null handling. The violation condition now requires both columns non-null before comparing, so null rows are no longer flagged. This aligns it with CheckSameValue's convention. - Collapse CheckGreaterOrEqual, CheckGreaterThan, and CheckLessOrEqual into a shared _CheckScalarComparisonGenerator base. Each subclass now sets only its operators, message phrase, and check type. This also standardizes literal quoting and predicate gating across the three. Tests: - Add a row-condition predicate test covering the named comparison generators to lock in the now-uniform behavior. - Update the CheckStringEndsWith assertion to expect the ends_with SQL. - Add tests/config_objects/test_generator_edge_cases.py, which executes generated SQL against a real in-memory DuckDB to cover null handling, multi-byte suffixes, malformed JSON paths, a missing jsonschema dependency, and row-filtered de-duplication. Signed-off-by: Mike Fuller <mike@finops.org>
Co-authored-by: Matt Cowsert <matthew@finops.org>
The Utf8 datetime parsing strategies each required a zero null_count, so a single null cell (an originally-null value or one unparseable entry) forced every strategy to fail and the column to stay a string. Nullable datetime columns are common in FOCUS, so this path was hit in practice. Judge parse success against the original null count instead of zero: a strategy now succeeds when parsing introduces no new nulls beyond those already present and the column has at least one real value. A nullable single-format column converts, a genuinely mixed or unparseable column is still rejected, and an all-null column is left as a string rather than coerced. Add regression tests covering nullable ISO (Z and offset), date-only, and space-separated columns, plus a mixed-format column that must be dropped. Also fix a stale TypeJSON test assertion: the generator now emits NOT json_valid(...) rather than typeof(...) != 'JSON'. Signed-off-by: Mike Fuller <mike@finops.org>
Signed-off-by: Mike Fuller <mike@finops.org>
Signed-off-by: Mike Fuller <mike@finops.org>
Clean up issue with main having a squash marge on it Signed-off-by: Mike Fuller <mike@finops.org>
Corrections for issues found on PR: #147 --------- Signed-off-by: Mike Fuller <mike@finops.org>
…umnValue
ColumnByColumnEqualsColumnValueGenerator emitted ({a} * {b}) <> {r} with no
tolerance. The CSV loader infers numeric columns as Float64/DOUBLE, so the
product is computed in IEEE-754 and compared bit-exactly. Rows whose
exact-decimal product equals the result but whose float64 product differs by a
rounding bit (e.g. 0.000015 * 20 -> 0.00030000000000000003 vs 0.0003) were
wrongly flagged non-conformant.
Replace the exact comparison in both the violation and predicate SQL with a
relative epsilon via a shared _tolerance_expr helper so the two can't drift:
ABS(({a} * {b}) - {r}) <= 1e-9 * GREATEST(ABS({r}), 1). The tolerance absorbs
only float representation error, staying faithful to FOCUS exact-equality intent.
Resolves false failures for CAU-ListCost-C-011-C, CAU-ListUnitPrice-C-014-C,
CAU-ContractedCost-C-011-C, and CAU-ContractedUnitPrice-C-013-C.
Add end-to-end DuckDB regression tests and update the composite-calculation
generator test to assert the new tolerance SQL.
Signed-off-by: Mike Fuller <mike@finops.org>
Signed-off-by: Mike Fuller <mike@finops.org>
Signed-off-by: Mike Fuller <mike@finops.org>
Co-authored-by: Matt Cowsert <matthew@finops.org>
Matt-Cowsert
approved these changes
Aug 4, 2026
Matt-Cowsert
left a comment
Matt-Cowsert
left a comment
Collaborator
There was a problem hiding this comment.
The tolerance is sized well above the error it absorbs. I ran 400,000 decimal price-by-quantity rows through the path the loader takes (exact decimal product stored as the result, both sides parsed to float64, product computed in DuckDB). Worst relative discrepancy was 3.5e-16, against a theoretical bound near 3.3e-16 for a single multiply. 1e-9 leaves roughly 2.8 million times more room than that.
Where the slack lands: a one-cent discrepancy on a $10,000,000 ListCost goes unreported, since the tolerance reaches $0.01 at that magnitude. At 1e-12 it is reported, still with ~2,800x headroom over the measured error, and the full suite passes.
Two suggestions inline. They are a pair: the test fails unless the constant moves, so take both or neither. Happy to see this ship as-is.
Contributor
Author
|
Merged via feature branch to avoid merge conflict errors. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.