dropped 5 duplicate row(s) (2.4% of rows, keep='first')
email
missing
medium
100%
0
preserved 23 missing value(s)
tier
missing
medium
70%
51
filled 51 missing value(s) with sentinel "Missing" ('Missing')
revenue
outliers
low
90%
0
preserved 5 outlier(s), 2.5% of values (method=iqr, factor=1.5)
Needs review
id column 'email' has 23 missing value(s) (11.5%); check whether those rows are joinable at all
column 'revenue' has 5 extreme value(s) that were deliberately preserved; review them in their domain context
\ No newline at end of file
diff --git a/docs/examples/baseline_drift.html b/docs/examples/baseline_drift.html
index 417b8023..312a3f7c 100644
--- a/docs/examples/baseline_drift.html
+++ b/docs/examples/baseline_drift.html
@@ -31,5 +31,5 @@
\ No newline at end of file
diff --git a/docs/examples/compare_plans_grid.html b/docs/examples/compare_plans_grid.html
index 9f2d7c27..c41e10b2 100644
--- a/docs/examples/compare_plans_grid.html
+++ b/docs/examples/compare_plans_grid.html
@@ -29,4 +29,4 @@
.fd-del-pos{color:#1a7f37}.fd-del-neg{color:#cf222e}
.fd-mono{font-family:ui-monospace,SFMono-Regular,Menlo,monospace;font-size:.8rem}
-
freshdata strategy comparison
strategy diff grid
column
strategy
missing_model
outlier_action
n_outliers
email
balanced
preserve
0
revenue
balanced
5
tier
balanced
sentinel
0
email
aggressive
preserve
0
revenue
aggressive
5
tier
aggressive
sentinel
0
\ No newline at end of file
+
freshdata strategy comparison
strategy diff grid
column
strategy
missing_model
outlier_action
n_outliers
email
balanced
preserve
0
revenue
balanced
5
tier
balanced
sentinel
0
email
aggressive
preserve
0
revenue
aggressive
5
tier
aggressive
sentinel
0
\ No newline at end of file
diff --git a/docs/examples/profile_cockpit.html b/docs/examples/profile_cockpit.html
index dccbcd27..9d8e8660 100644
--- a/docs/examples/profile_cockpit.html
+++ b/docs/examples/profile_cockpit.html
@@ -31,7 +31,7 @@
freshdata profile
inline quality cockpit
82/100
quality score
205
rows
5
columns
7.4%
missing
5
duplicate rows
4
issues
Type inference
signup_date: object → datetime64[ns]
-
Columns (issue-ranked)
column
dtype
missing
unique
issues
tier
object
25%
3
25.4% missing
email
object
12%
177
11.7% missing
revenue
float64
0%
199
7 potential outlier(s) (iqr)
signup_date
object
0%
200
would convert to datetime64[ns]
customer_id
int64
0%
200
—
+
Columns (issue-ranked)
column
dtype
missing
unique
issues
tier
object
25%
3
25.4% missing
email
object
12%
177
11.7% missing
revenue
float64
0%
199
7 potential outlier(s) (iqr)
signup_date
object
0%
200
would convert to datetime64[ns]
customer_id
int64
0%
200
—
Outlier warnings
revenue
Correlations
Numeric correlations are computed on demand to keep this view fast. Call profile.to_frame() or compute df.corr() when you need them.
\ No newline at end of file
diff --git a/docs/examples/quality_debt.html b/docs/examples/quality_debt.html
index f7bc1f7f..400abde9 100644
--- a/docs/examples/quality_debt.html
+++ b/docs/examples/quality_debt.html
@@ -31,5 +31,5 @@
\ No newline at end of file
diff --git a/src/freshdata/insight.py b/src/freshdata/insight.py
index db0c170e..349da20f 100644
--- a/src/freshdata/insight.py
+++ b/src/freshdata/insight.py
@@ -263,6 +263,11 @@ def _issues_from_profile(
action_lookup: dict[tuple[str, str], str] | None = None,
) -> list[dict[str, Any]]:
issues: list[dict[str, Any]] = []
+ # Distinct column names can slug identically ("a b", "a_b", "A-B"); suffix
+ # repeats in profile order so every issue id (and fallback action id) is
+ # unique while non-colliding ids stay unchanged.
+ seen_issue_ids: dict[str, int] = {}
+ seen_action_ids: dict[str, int] = {}
for column in profile.columns:
if not column.issues:
continue
@@ -270,13 +275,12 @@ def _issues_from_profile(
role = getattr(ctx, "role", "unknown")
severity = _severity(column.issues, column.missing_pct)
hint = _action_hint(column.issues, role)
- action_id = (action_lookup or {}).get(
- (column.name, hint),
- f"action.{_slug(column.name)}.{hint}",
- )
+ action_id = (action_lookup or {}).get((column.name, hint))
+ if action_id is None:
+ action_id = _unique_id(f"action.{_slug(column.name)}.{hint}", seen_action_ids)
issues.append(
{
- "id": f"issue.{_slug(column.name)}.{hint}",
+ "id": _unique_id(f"issue.{_slug(column.name)}.{hint}", seen_issue_ids),
"column": column.name,
"severity": severity,
"finding": "; ".join(column.issues),
@@ -317,8 +321,7 @@ def _actions_from_clean_report(
for action in report.actions:
entry = CleanReport._action_dict(action)
base_id = f"action.{_slug(action.column or 'table')}.{_slug(action.step)}"
- seen[base_id] = seen.get(base_id, 0) + 1
- entry["id"] = base_id if seen[base_id] == 1 else f"{base_id}.{seen[base_id]}"
+ entry["id"] = _unique_id(base_id, seen)
entry["impact"] = _action_impact(action.column, before=before, after=after)
actions.append(entry)
return actions
@@ -665,6 +668,12 @@ def _recommended_next_step(dataset_name: str, config: CleanConfig) -> str:
)
+def _unique_id(base_id: str, seen: dict[str, int]) -> str:
+ """Return *base_id* the first time, then ``base_id.2``, ``base_id.3``, …"""
+ seen[base_id] = seen.get(base_id, 0) + 1
+ return base_id if seen[base_id] == 1 else f"{base_id}.{seen[base_id]}"
+
+
def _slug(value: str) -> str:
out = []
for char in str(value).lower():
diff --git a/src/freshdata/render/_vocabulary.py b/src/freshdata/render/_vocabulary.py
index 7b0f09ff..72fd666b 100644
--- a/src/freshdata/render/_vocabulary.py
+++ b/src/freshdata/render/_vocabulary.py
@@ -34,15 +34,29 @@
}
+#: Action statuses that record a decision *not* to change data.
+_NOT_APPLIED = frozenset({"skipped", "suggested"})
+
+
+def changed_values(action: Action) -> bool:
+ """``True`` when *action* actually changed cells or rows.
+
+ Informational notes (``count == 0``, e.g. "preserved 3 missing value(s)")
+ and actions that were only suggested or deliberately skipped are not
+ changes.
+ """
+ return action.count > 0 and action.status not in _NOT_APPLIED
+
+
def plain_step(action: Action) -> str:
"""A short plain-language phrase for *action*, for per-column summaries.
Falls back to the action's own description — descriptions are already
human sentences; the map only replaces the jargon-heavy step families.
+ Actions that changed nothing keep their description, so a preserved gap
+ is never reworded as a fill.
"""
phrase = _STEP_PHRASES.get(action.step)
- if phrase is None:
+ if phrase is None or not changed_values(action):
return action.description
- if action.count:
- return f"{phrase} ({action.count:,})"
- return phrase
+ return f"{phrase} ({action.count:,})"
diff --git a/src/freshdata/render/html.py b/src/freshdata/render/html.py
index 3ead2879..8da43f04 100644
--- a/src/freshdata/render/html.py
+++ b/src/freshdata/render/html.py
@@ -153,7 +153,10 @@ def filterable_table(
"""A table with client-side text/select filters — no JS libraries needed.
*filters* maps a label to the 0-based column index it filters (a free-text
- box). The whole thing is self-contained vanilla JS scoped by *table_id*.
+ box). The script is a self-contained vanilla-JS IIFE emitted right after
+ the table: it finds its own table and filter boxes through
+ ``document.currentScript`` and DOM siblings, so no global names are
+ derived from *table_id* and several reports can share one page.
"""
tbl = table(headers, rows, raw_columns=raw_columns).replace(
'
', f'
', 1
@@ -162,24 +165,35 @@ def filterable_table(
js = ""
if filters:
boxes = "".join(
- f''
+ f''
for label, idx in filters.items()
)
controls = f'
{boxes}
'
- js = (
- f""
- )
+ js = f""
return f"{controls}{tbl}{js}"
+#: Filter behaviour for :func:`filterable_table`. Static (no interpolation), so
+#: the output stays deterministic and the script is always valid JavaScript.
+_FILTER_JS = (
+ "(function(){"
+ "var s=document.currentScript;if(!s)return;"
+ "var t=s.previousElementSibling;"
+ "if(!t||t.tagName!=='TABLE'||!t.tBodies.length)return;"
+ "var c=t.previousElementSibling;"
+ "if(!c||(' '+c.className+' ').indexOf(' fd-controls ')<0)return;"
+ "var inp=c.querySelectorAll('input[data-col]');"
+ "function run(){var rows=t.tBodies[0].rows;"
+ "for(var i=0;i str:
"""A download link that embeds *content* as a data URI (no server)."""
import base64
diff --git a/src/freshdata/stakeholder.py b/src/freshdata/stakeholder.py
index 9ea41263..23365b5c 100644
--- a/src/freshdata/stakeholder.py
+++ b/src/freshdata/stakeholder.py
@@ -14,6 +14,7 @@
from typing import TYPE_CHECKING, Any
from .render import html as H
+from .render._vocabulary import changed_values
from .render.mixins import SimpleHtmlReport
if TYPE_CHECKING: # pragma: no cover - typing only
@@ -23,8 +24,9 @@
_FORMATS = ("markdown", "html")
-def _pct(part: int, whole: int) -> float:
- return 100.0 * (1 - part / whole) if whole else 100.0
+def _pct(part: int, whole: int) -> float | None:
+ """Percent of *whole* that is not *part*; ``None`` when there are no cells."""
+ return 100.0 * (1 - part / whole) if whole else None
@dataclass
@@ -129,13 +131,16 @@ def stakeholder_summary(
if format not in _FORMATS:
raise ValueError(f"format must be one of {_FORMATS}, got {format!r}")
+ materialized = getattr(report, "materialized", True)
cells_before = report.rows_before * report.cols_before
cells_after = report.rows_after * report.cols_after
comp_before = _pct(report.missing_before, cells_before)
- comp_after = _pct(report.missing_after, cells_after)
+ # An un-materialized result has no computed "after" counts to measure.
+ comp_after = _pct(report.missing_after, cells_after) if materialized else None
changed: list[str] = []
- if report.missing_before != report.missing_after:
+ if (report.missing_before != report.missing_after
+ and comp_before is not None and comp_after is not None):
direction = "rose" if comp_after > comp_before else "fell"
changed.append(
f"Overall data completeness {direction} from {comp_before:.1f}% to "
@@ -150,11 +155,12 @@ def stakeholder_summary(
changed.append(
f"{len(report.columns_dropped)} unusable column(s) were removed: "
f"{', '.join(report.columns_dropped[:6])}.")
- n_changed_cols = len({a.column for a in report.actions if a.column})
+ n_changed_cols = len(
+ {a.column for a in report.actions if a.column and changed_values(a)})
if n_changed_cols:
changed.append(f"{n_changed_cols} column(s) changed meaningfully.")
if audience == "technical":
- steps = sorted({a.step for a in report.actions if a.count})
+ steps = sorted({a.step for a in report.actions if changed_values(a)})
if steps:
changed.append("Steps applied: " + ", ".join(steps) + ".")
@@ -172,13 +178,26 @@ def stakeholder_summary(
review: list[str] = list(report.warnings) + list(report.recommendations)
- headline = (
- f"Cleaning kept {comp_after:.1f}% of fields complete across "
- f"{report.rows_after:,} record(s); {len(review)} item(s) need review.")
+ n_review = f"{len(review)} item(s) need review."
+ if not materialized:
+ headline = (
+ "Cleaning kept the result in the engine, so completeness was not "
+ f"computed; {n_review}")
+ elif comp_after is not None:
+ headline = (
+ f"Cleaning kept {comp_after:.1f}% of fields complete across "
+ f"{report.rows_after:,} record(s); {n_review}")
+ elif report.cols_after == 0:
+ headline = (
+ "Cleaning removed every column, leaving no fields to measure across "
+ f"{report.rows_after:,} record(s); {n_review}")
+ else:
+ headline = (
+ f"Cleaning removed every record, leaving no fields to measure; {n_review}")
metrics = {
"records": f"{report.rows_after:,}",
- "completeness": f"{comp_after:.1f}%",
+ "completeness": "n/a" if comp_after is None else f"{comp_after:.1f}%",
"duplicates removed": f"{report.duplicates_removed:,}",
"needs review": len(review),
}
diff --git a/tests/test_report_rendering_fixes.py b/tests/test_report_rendering_fixes.py
new file mode 100644
index 00000000..793456d8
--- /dev/null
+++ b/tests/test_report_rendering_fixes.py
@@ -0,0 +1,205 @@
+"""Regression tests for report-rendering fixes (#329, #336, #337, #339)."""
+
+from __future__ import annotations
+
+import re
+import shutil
+import subprocess
+
+import pandas as pd
+import pytest
+
+import freshdata as fd
+from freshdata.render import html as H
+from freshdata.render._vocabulary import changed_values, plain_step
+from freshdata.render.normalize import normalize_clean_report
+from freshdata.report import Action, CleanReport
+
+_SCRIPT_RE = re.compile(r"", re.S)
+
+
+# -- #329: insight issue ids are unique ---------------------------------------
+
+
+def test_insight_issue_ids_unique_when_column_slugs_collide() -> None:
+ df = pd.DataFrame({"a b": [1, None] * 20, "a_b": [1, None] * 20, "A-B": ["x", None] * 20})
+ rep = fd.insight_report(df)
+ pairs = [(i["id"], i["column"]) for i in rep.issues]
+ assert pairs == [
+ ("issue.a_b.missing", "a b"),
+ ("issue.a_b.missing.2", "a_b"),
+ ("issue.a_b.missing.3", "A-B"),
+ ]
+ action_ids = [i["recommended_action_id"] for i in rep.issues]
+ assert action_ids == [
+ "action.a_b.missing",
+ "action.a_b.missing.2",
+ "action.a_b.missing.3",
+ ]
+
+
+def test_insight_ids_unchanged_for_non_colliding_columns() -> None:
+ df = pd.DataFrame({"age": [1, None] * 20, "City Name": ["x", None] * 20})
+ ids = sorted(i["id"] for i in fd.insight_report(df).issues)
+ assert ids == ["issue.age.missing", "issue.city_name.missing"]
+
+
+def test_insight_recommended_action_ids_point_at_real_actions() -> None:
+ df = pd.DataFrame({"a b": [1.0, None] * 20, "a_b": [2.0, None] * 20, "keep": range(40)})
+ cleaned, report = fd.clean(df, return_report=True, verbose=False)
+ rep = fd.insight_report(df, clean_report=report, cleaned_df=cleaned)
+ issue_ids = [i["id"] for i in rep.issues]
+ assert len(issue_ids) == len(set(issue_ids))
+ action_ids = {a["id"] for a in rep.actions}
+ by_column = {a["column"]: a["id"] for a in rep.actions if a["step"] == "missing"}
+ for issue in rep.issues:
+ if issue["column"] in by_column:
+ assert issue["recommended_action_id"] == by_column[issue["column"]]
+ assert issue["recommended_action_id"] in action_ids
+
+
+# -- #336: HTML filter script is valid and id-free -----------------------------
+
+
+def _clean_html() -> str:
+ df = pd.DataFrame({"name": [" a", "b ", None, "b "], "v": [1.0, None, 3.0, 3.0]})
+ _, rep = fd.clean(df, return_report=True, verbose=False)
+ return rep.to_html()
+
+
+def test_filterable_table_has_no_inline_handlers_or_global_functions() -> None:
+ out = H.filterable_table("fd-ledger", ["a", "b"], [["1", "2"]], filters={"a": 0, "b": 1})
+ assert "oninput=" not in out
+ assert "fdFilter_" not in out
+ assert "document.currentScript" in out
+ scripts = _SCRIPT_RE.findall(out)
+ assert len(scripts) == 1
+ assert "fd-ledger" not in scripts[0]
+ # controls, then table, then the script: the DOM-sibling lookup relies on it.
+ assert out.index('class="fd-controls"') < out.index("")
+ assert out.endswith("