giteadmin/datatools-dev
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

refactor(gui): drop Review page + normalization gate

Browse Source 
Home is now the only entry point: the "Run analysis" button on the
upload section IS the review step (findings render inline via
render_findings_panel). Tool pages no longer gate on a passed
normalization — running the analyzer is sufficient context.

Removed:
- src/gui/pages/0_Review.py
- src/gui/components/gate.py (re-export seam)
- require_normalization_gate() in src/gui/components/_legacy.py
- "review" section enum in tools_registry.py
- Data Review entry in app.py navigation
- require_normalization_gate() calls + imports in all nine tool pages
- tests/gui/test_gate.py (whole file)
- TestReviewWorkflow in tests/gui/test_workflows.py
- 0_Review entry in tests/gui/test_smoke.py PAGE_SLUGS
- stash_upload's normalization_result+normalization_for stashing
- stash_upload_without_gate (was the gate's negative-path helper)

2017 tests pass (16 retired with the gate flow).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
Michael
2026-05-16 20:04:33 +00:00
parent fc6c22c6a7
commit dad744f17f
19 changed files with 11 additions and 1044 deletions
Download Patch File Download Diff File Expand all files Collapse all files

38
tests/gui/conftest.py
View File

@@ -109,49 +109,19 @@ def app_factory():
# ---------------------------------------------------------------------------
def stash_upload(app: AppTest, *, name: str, data: bytes) -> str:
"""Pre-populate the home-screen upload stash + the gate's normalisation
result so a tool page renders past ``require_normalization_gate()``.
"""Pre-populate the home-screen upload stash so a tool page renders
as if the user had uploaded *name* / *data* on the home screen.
Returns the SHA-256 hex of *data* (used as the gate key) in case the
test wants to assert against it.
The gate checks::
- ``home_uploaded_bytes`` is set
- ``normalization_for == sha256(home_uploaded_bytes)``
- ``normalization_result.passed is True``
We synthesise a passing result via a tiny stub object that satisfies
the gate's only attribute access (``.passed``). Tests that want to
exercise gate-blocking behaviour should NOT call this helper — they
should stash bytes without the normalisation result.
Returns the SHA-256 hex of *data* in case the test wants to assert
against it.
"""
sha = hashlib.sha256(data).hexdigest()
app.session_state["home_uploaded_bytes"] = data
app.session_state["home_uploaded_name"] = name
app.session_state["home_uploaded_size"] = len(data)
app.session_state["normalization_for"] = sha
app.session_state["normalization_result"] = _PassedGateResult()
return sha
class _PassedGateResult:
"""Minimal stand-in for the real NormalizationResult shape — the gate
only reads ``.passed``. Using a real NormalizationResult here would
pull in core.normalize and tie GUI tests to its constructor surface.
"""
passed: bool = True
def stash_upload_without_gate(app: AppTest, *, name: str, data: bytes) -> None:
"""Stash the upload bytes but do NOT pre-pass the gate. Used by gate
tests that want the warning + Go-to-Review button to appear."""
app.session_state["home_uploaded_bytes"] = data
app.session_state["home_uploaded_name"] = name
app.session_state["home_uploaded_size"] = len(data)
# ---------------------------------------------------------------------------
# i18n helpers
# ---------------------------------------------------------------------------

157
tests/gui/test_gate.py
View File

@@ -1,157 +0,0 @@
"""Gate tests — ``require_normalization_gate()`` behaviour.
The gate sits between every tool page and the user's data. Three states
exist, each pinned here:
1. **No upload** — gate is a no-op; the page proceeds and its own
uploader handles the file.
2. **Upload but no normalization result** — gate shows a warning and a
"Go to Review & Normalize" button, then ``st.stop()`` short-circuits
the rest of the page.
3. **Upload + matching passed normalization** — gate is a no-op; the
page proceeds.
We exercise the gate via the Find Duplicates page (any tool page would
work; dedup is the smallest one that doesn't depend on heavy widgets).
"""
from __future__ import annotations
import pytest
from .conftest import (
collected_text,
stash_upload,
stash_upload_without_gate,
with_language,
)
# Find Duplicates is our canary — it calls ``require_normalization_gate``
# on the second line of the module. If the gate blocks, the dedup-
# specific title shouldn't even render.
GATED_PAGE = "1_Deduplicator"
class TestGateNoUpload:
"""No upload → the gate exits early and the page renders normally,
showing its own file uploader. (This is the "user opened the dedup
page first instead of coming from home" path.)"""
def test_no_upload_lets_page_render(self, app_factory):
app = app_factory(GATED_PAGE)
app.run()
assert not app.exception
text = collected_text(app)
# The dedup page title is the unambiguous signal that the gate
# didn't short-circuit.
assert "Find Duplicates" in text
def test_no_upload_no_gate_warning(self, app_factory):
app = app_factory(GATED_PAGE)
app.run()
# The gate's warning string starts with the upload filename. No
# warning should be present when there's no upload.
for w in app.warning:
assert "normalization gate" not in (w.body or "")
class TestGateBlocksWithoutNormalization:
"""Upload present but no passing normalization → gate fires:
warning + Go-to-Review button + page short-circuit."""
def test_gate_warning_renders(self, app_factory, small_csv_bytes):
app = app_factory(GATED_PAGE)
stash_upload_without_gate(app, name="messy.csv", data=small_csv_bytes)
app.run()
warnings = [w.body for w in app.warning if w.body]
joined = " ".join(warnings)
assert "normalization gate" in joined, (
f"expected gate warning; got warnings: {warnings}"
)
assert "messy.csv" in joined, (
"gate warning should name the offending file"
)
def test_gate_renders_go_to_review_button(self, app_factory, small_csv_bytes):
app = app_factory(GATED_PAGE)
stash_upload_without_gate(app, name="messy.csv", data=small_csv_bytes)
app.run()
labels = [b.label for b in app.button]
assert any("Review & Normalize" in lbl for lbl in labels), (
f"missing 'Go to Review & Normalize' button; got: {labels}"
)
def test_gate_short_circuits_page(self, app_factory, small_csv_bytes):
app = app_factory(GATED_PAGE)
stash_upload_without_gate(app, name="messy.csv", data=small_csv_bytes)
app.run()
# When the gate fires it calls ``st.stop()`` after the warning.
# The page-body widgets (e.g., the advanced-options expander, the
# dedup-strategy widgets) must NOT be present.
labels = [b.label for b in app.button]
# The Run-Dedup primary action lives below the gate — make sure
# the gate killed the render before it.
assert not any("Run Deduplication" in lbl for lbl in labels), (
f"gate failed to short-circuit; saw button: {labels}"
)
def test_gate_warning_localizes_to_spanish(self, app_factory, small_csv_bytes):
app = app_factory(GATED_PAGE)
with_language(app, "es")
stash_upload_without_gate(app, name="messy.csv", data=small_csv_bytes)
app.run()
warnings = " ".join(w.body for w in app.warning if w.body)
# Spanish pack: ``debe pasar la verificación de normalización CSV``.
assert "normalización" in warnings
def test_gate_button_localizes_to_spanish(self, app_factory, small_csv_bytes):
app = app_factory(GATED_PAGE)
with_language(app, "es")
stash_upload_without_gate(app, name="messy.csv", data=small_csv_bytes)
app.run()
labels = [b.label for b in app.button]
assert any("Revisar y Normalizar" in lbl for lbl in labels), (
f"Spanish gate button missing; got: {labels}"
)
class TestGateAllowsWithPassedNormalization:
"""Upload + passed normalization → gate is a no-op and the page
renders past the gate."""
def test_passed_gate_lets_page_render(self, app_factory, small_csv_bytes):
app = app_factory(GATED_PAGE)
stash_upload(app, name="messy.csv", data=small_csv_bytes)
app.run()
assert not app.exception, f"page raised past gate: {app.exception}"
# The pickup banner uses the upload name — that's our signal
# that the gate let us through AND the pickup helper engaged.
text = collected_text(app)
assert "messy.csv" in text
class TestGateMismatchedHash:
"""Upload changes (different bytes) but normalization_for still
points at the old hash → gate fires again because the result is
stale. Pins the security-relevant "stale fix doesn't carry over to
a new file" invariant."""
def test_stale_normalization_blocks_new_upload(self, app_factory, small_csv_bytes):
app = app_factory(GATED_PAGE)
# Stash bytes A but a normalization_for hash that points at B.
app.session_state["home_uploaded_bytes"] = small_csv_bytes
app.session_state["home_uploaded_name"] = "new.csv"
app.session_state["home_uploaded_size"] = len(small_csv_bytes)
app.session_state["normalization_for"] = "different-hash-from-an-old-upload"
# A passed-result object exists but is keyed to a different file.
class _Passed:
passed = True
app.session_state["normalization_result"] = _Passed()
app.run()
warnings = " ".join(w.body for w in app.warning if w.body)
assert "normalization gate" in warnings, (
"stale gate result should not unlock a new upload"
)

2
tests/gui/test_smoke.py
View File

@@ -25,7 +25,6 @@ from .conftest import collected_text, with_language
# Every page that ships in the sidebar nav. Slugs match the filenames
# under ``src/gui/pages/`` so failures point at a real file.
PAGE_SLUGS = [
"0_Review",
"1_Deduplicator",
"2_Text_Cleaner",
"3_Format_Standardizer",
@@ -53,7 +52,6 @@ PAGE_SLUGS = [
# When a page gains real Spanish translation, flip its 'es' entry to
# the localized substring — the test surface stays the same.
EXPECTED_SUBSTRINGS: dict[str, dict[str, str]] = {
"0_Review": {"en": "Review", "es": "Review"},
"1_Deduplicator": {"en": "Find Duplicates", "es": "Find Duplicates"},
"2_Text_Cleaner": {"en": "Clean Text", "es": "Clean Text"},
"3_Format_Standardizer": {"en": "Standardize", "es": "Standardize"},

44
tests/gui/test_workflows.py
View File

@@ -151,50 +151,6 @@ class TestPipelineRunnerWorkflow:
assert "Automated Workflows" in text
# ---------------------------------------------------------------------------
# Review page — special: doesn't gate on upload, has its own analyzer flow
# ---------------------------------------------------------------------------
class TestReviewWorkflow:
"""The Review page is the gate-fixer. Without an upload it shows
its own file uploader so the user can start the flow from this
page directly. With an upload it runs the analyzer and shows
findings."""
def test_no_upload_shows_inline_uploader(self, app_factory):
app = app_factory("0_Review")
app.run()
text = collected_text(app)
# Page should invite the user to upload, not redirect home.
assert "Upload" in text or "Choose a file" in text, (
f"Review page should expose an inline uploader; got:\n{text[:400]}"
)
# The 'Back to home' button is gone — the page is self-contained now.
labels = [b.label for b in app.button]
assert not any("Back to home" in lbl for lbl in labels), (
f"Back-to-home button should be removed; got buttons: {labels}"
)
def test_with_upload_shows_review_content(
self, app_factory, small_csv_bytes,
):
app = app_factory("0_Review")
# Review page only needs the upload bytes, not a pre-passed gate.
app.session_state["home_uploaded_bytes"] = small_csv_bytes
app.session_state["home_uploaded_name"] = "messy.csv"
app.session_state["home_uploaded_size"] = len(small_csv_bytes)
app.run()
assert not app.exception
text = collected_text(app)
# Page ran the analyzer — either we get findings or the
# "already clean" success message. Either way confirms the
# analyzer pipeline ran end-to-end with the stashed bytes.
clean_msg = "No findings to review" in text
encoding_section = "File encoding" in text
assert clean_msg or encoding_section, (
f"Review page didn't surface analyzer output; got:\n{text[:400]}"
)
# ---------------------------------------------------------------------------
# Coming-Soon pages still render (just a stub) — pinned so we know if a