fix: Correctness and consistency improvements across core, render, introspect, and CLI#84
Merged
Merged
Conversation
- validate_fk: use case-insensitive comparison for to_columns against target table columns, matching the from_columns behavior - column_list_has_prefix: use case-insensitive comparison for FK index prefix matching to avoid false positives across dialects - referenced_columns_are_unique: normalize to lowercase before comparing FK to_columns against PK and unique index columns, preventing ForeignKeyNonUniqueTarget false positives on mixed-case names - import_schema: return Result<Schema, ImportError> instead of panicking via assert!, so malformed user-supplied JSON produces an error rather than crashing the CLI/WASM process; also asserts that stable_id is non-empty to prevent downstream overlay/diff corruption
- escape_xml_text: replace 5-chain .replace() with single-pass char scan to avoid 4 intermediate String allocations - ThemeColors: add is_light field so theme detection does not depend on a hardcoded background color string - is_light_theme: read the structural field instead of comparing against a magic "#f7f8fc" value
…tion - generate_table_id/generate_column_id: use '\0' as separator instead of '.' to prevent hash collisions when identifiers contain dots - introspect/parser stable_id construction: quote components that contain '.' so that schema="a.b"+table="c" and schema="a"+table="b.c" no longer collapse to the same string, eliminating ambiguous keys in diff, export, and graph node lookups - WasmRenderRequest: delegate to wasm_input_source() for input validation, consistent with all other WASM request types
…sult - read_sniffed_file: reuse the already-read file content instead of returning a file-path variant that causes a second read from disk - export: replace 5 .expect() calls with .ok_or_else() returning AppError, so that adding a new ExportFormat variant cannot panic
Code Metrics Report
Details | | main (cf85e4b) | #84 (7f0e7cd) | +/- |
|---------------------|----------------|---------------|-------|
- | Coverage | 94.6% | 94.6% | -0.1% |
| Files | 77 | 77 | 0 |
| Lines | 33774 | 33826 | +52 |
+ | Covered | 31981 | 32012 | +31 |
- | Test Execution Time | 1m25s | 1m27s | +2s |Code coverage of files in pull request scope (95.1% → 94.9%)Reported by octocov |
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
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.
Summary
., and convert panicking paths in JSON import andexport into
ResultRenderRequestinput validation with the other variants, switch SVG output to single-pass XML escaping, and replace magic-string theme detection with a structural fieldChanges
validate_fk/column_list_has_prefix/referenced_columns_are_uniquecase-insensitive so they match thefrom_columnsbehavior and stop emitting false-positiveForeignKeyNonUniqueTargeton mixed-case namesimport_schemato returnResult<Schema, ImportError>so malformed JSON input produces an error instead of panicking viaassert!stable_idduring import to prevent empty IDs from corrupting downstream overlay / diff.replace()inescape_xml_textwith a single-pass char scan, removing 4 intermediateStringallocationsis_lightfield toThemeColorssois_light_themereads a structural flag instead of comparing against the hardcoded#f7f8fcbackgroundgenerate_table_id/generate_column_idseparators from.to\0to remove hash collisions when identifiers contain dots.-containing components when building introspect / parserstable_idso thatschema="a.b"+table="c"andschema="a"+table="b.c"no longer collapse to the same key, eliminatingambiguous lookups in diff / export / graph nodes
WasmRenderRequestthroughwasm_input_source()to match the Inspect / Export / Lint / Diff input-validation semanticsread_sniffed_fileinstead of returning a path variant that triggered a second disk read.expect()calls in theexportuse case withok_or_else+AppErrorso that adding a newExportFormatvariant without updatingexport_format_uses_graphcannot panic