fix(reports): unique insight ids, working HTML filters, honest stakeholder change and completeness wording - #371
Merged
Merged
Conversation
…older change and completeness wording insight_report (#329): issue ids were built as issue.<slug(column)>.<hint>, so columns whose names slug the same ("a b", "a_b", "A-B") shared one id. Issue ids and the fallback recommended_action_id now go through the same seen-counter used for action ids (base, base.2, base.3, ...) in profile order. Ids for non-colliding columns are unchanged, and issues still point at the real action id when a clean report is supplied. HTML filters (#336): filterable_table derived a global function name from the table id (function fdFilter_fd-ledger), which is a JavaScript syntax error, so no filter box in any HTML report worked. The inline oninput handlers and the global function are gone; a static IIFE emitted right after the table finds its own table and filter inputs through document.currentScript and DOM siblings. Nothing in the script depends on the table id, output stays deterministic, and several reports can share a page. The committed docs/examples pages get the same markup swap. Change counting (#337): stakeholder_summary counted every action with a column as a change, and Peel's plain_step reworded informational "preserved N missing value(s)" notes (count=0) as "filled missing values". A shared changed_values() predicate (count > 0 and status not skipped or suggested) now drives the "column(s) changed meaningfully" count, the technical "Steps applied" list, and plain_step, which keeps the action's own description when nothing changed. Completeness (#339): _pct returned 100.0 for a zero-cell frame, so a run that dropped every column claimed 100% completeness. _pct now returns None for zero cells; the completeness sentence is skipped when either side is unknown, metrics["completeness"] is "n/a", and the headline says "Cleaning removed every column, ..." (or "every record" when only rows are gone). Un-materialized engine results also stop claiming a completeness figure. Closes #329 Closes #336 Closes #337 Closes #339
Contributor
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
FreshData benchmark report —
|
| fixture | n_rows | n_cols | p50 s | p95 s | peak MB | repair % | false-repair % | preserve % | trust | monotonic | export % |
|---|
Authored-code reduction (Metric 6)
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
1 participant
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
insight_reportissue ids collided for columns whose names slug the same ("a b","a_b","A-B"). Issue ids and the fallbackrecommended_action_idnow use the same seen-counter as action ids (base,base.2,base.3, … in profile order). Non-colliding ids are unchanged.filterable_tablegeneratedfunction fdFilter_fd-ledger(){…}, a JavaScript syntax error. The inlineoninputhandlers and global function are replaced by a static IIFE emitted after each table that locates its table and inputs throughdocument.currentScriptand DOM siblings. Output stays deterministic and multiple reports can share a page. The committeddocs/examples/*.htmlpages get the same markup swap (only the filter markup; the pages are otherwise left as committed).stakeholder_summaryand the Peel per-column view described preserved missing values (count=0notes) as changes. A sharedchanged_values()predicate (count > 0, status notskipped/suggested) now drives the "changed meaningfully" count, the technical "Steps applied" list andplain_step, which keeps the action's description when nothing changed.stakeholder_summaryclaimed 100% completeness when every column was dropped._pctreturnsNonefor zero cells, the completeness sentence is skipped,metrics["completeness"]is"n/a", and the headline reads "Cleaning removed every column, leaving no fields to measure across N record(s); …" (or "every record" when only rows are gone). Un-materialized engine results no longer claim a completeness figure either.#338 (Markdown table escaping) is not included here; it is covered by #355.
Tests
New
tests/test_report_rendering_fixes.pywith each issue's reproduction:oninput=/fdFilter_in the filter markup, deterministic output, and every generated<script>passingnode --check(skipped when node is absent);plain_stepwording, and Peel column rows for preserved gaps;Verification
ruff check .clean;mypy src/freshdataclean (202 files)Closes #329
Closes #336
Closes #337
Closes #339