PR 7.2: local Kaggle + HF mock-page preview + tests + sample HTML#77
Merged
Conversation
…re for preview output - docs/release/preview_pages_design.md — 10-decision table covering scripts shape, server (stdlib http.server), templates (f-strings), Markdown renderer (markdown-it-py via [publish] extra), output dirs, CLI shape, audit-artefact-sync, test posture, link-resolution rule, out-of-scope. - pyproject.toml [publish] gains markdown-it-py>=3.0 alongside datasets / kaggle. Same gating posture as PR 5.1 / 5.2 — preview scripts raise a clean ImportError pointing at this extra when missing. - .gitignore: release/_preview/ runtime output excluded; the audit-sync samples under release/_preview_committed/ are checked in separately. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Two scripts + tests + their first rendered-page output committed:
- scripts/preview_kaggle_page.py — reads release/kaggle/dataset-metadata.json
+ cover image, renders an offline HTML page mocking the public Kaggle
view (header / cover / description / file tree / schema tables /
sources / footer). Serves on localhost:8765 via stdlib
ThreadingTCPServer; --no-serve / --open-browser flags.
- scripts/preview_hf_page.py — reads release/huggingface[-instructor]/README.md
(YAML frontmatter + body), renders the analogous HF view (header pills /
tag chips / configs dropdown / file tree / README body / footer).
Serves on localhost:8766; --variant=public|instructor reads the
matching companion README and writes to a variant-flavoured out_dir.
Both renderers are pure: same input → byte-identical HTML (verified
two-pass against the real release artefacts). Output landing at
release/_preview/<platform>/ is gitignored; committed sample HTML at
release/_preview_committed/{kaggle,huggingface_public,huggingface_instructor}.html
is the audit-artefact-sync gate.
Markdown rendering via markdown-it-py (gfm-like preset, linkify
disabled to avoid the linkify-it-py transitive dep). Missing dep
raises a clean ImportError pointing at pip install -e '.[publish]'.
pyproject.toml ruff per-file-ignores adds E501 for the two scripts —
inlined CSS strings in f-string templates are the product, not source
code that benefits from a 100c wrap.
48 new tests (no live HTTP, no network):
- required field labels (title / subtitle / licence / file count /
schema column count for Kaggle; pretty_name / licence / configs /
tags for HF)
- every Markdown link in the source resolves to a non-404 URL pattern
(no ](../, no ](validation/, only allow-listed external prefixes
+ sibling-relative LICENSE + in-document anchors)
- every configs[] block in the HF YAML round-trips into the rendered
dropdown
- every CSV / parquet column declared in the Kaggle metadata appears
in the schema table
- byte-deterministic renderer + audit-sync against the committed sample
- pre-flight error paths (missing artefact, malformed JSON / YAML,
unknown variant) return rc=2
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Three findings folded back from the hostile-reviewer first pass: - BUG: switch from socketserver.ThreadingTCPServer to http.server.ThreadingHTTPServer in both preview scripts. The former defaults to allow_reuse_address=False, so Ctrl-C → re-run within ~60s would raise OSError [Errno 48] Address already in use while the socket sat in TIME_WAIT. ThreadingHTTPServer inherits allow_reuse_address=True from HTTPServer. - DEAD CODE: drop COMMITTED_SAMPLE_PATH (Kaggle) and _VARIANT_SAMPLE_PATH (HF) — defined as module-level constants but never read at runtime; tests use their own _REPO_ROOT-rooted paths. Also drop the now-unused socketserver import. - DOC LIE (minor): _resolve_cover_image docstring in the Kaggle script claimed "we prefer the kaggle-tree copy" without acknowledging that release/kaggle/dataset-cover-image.png is gitignored on a fresh checkout. Reworded to call out the lookup order and gitignore reality. Rendered HTML output unchanged; all 48 preview tests still pass; audit-sync samples remain byte-identical. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Replaces the open PR 7.2 stub with the dense-summary closed entry mirroring PR 7.1's structure: every load-bearing decision, both self-review passes, and the validation-suite numbers inline. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
shaypal5
added
type: feature
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Pull request overview
Adds deterministic, offline HTML preview tooling for Kaggle and Hugging Face dataset pages to support Phase 7’s pre-publish validation workflow, along with contract tests and committed “golden” HTML samples for audit/sync.
Changes:
- Introduces
scripts/preview_kaggle_page.pyandscripts/preview_hf_page.pyto render/serve local preview pages from release artefacts, with deterministic output. - Adds test suites pinning required-field presence, link allow-listing, schema/config round-trips, determinism, and pre-flight error behavior (rc=2).
- Commits sample preview HTML outputs, updates
[publish]extra to includemarkdown-it-py, and gitignores runtime preview output underrelease/_preview/.
Reviewed changes
Copilot reviewed 9 out of 11 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
scripts/preview_kaggle_page.py |
Kaggle preview renderer/driver (metadata.json + cover image) with local HTTP serving. |
scripts/preview_hf_page.py |
HF preview renderer/driver (README YAML frontmatter + body) with local HTTP serving and --variant. |
tests/scripts/test_preview_kaggle_page.py |
Contract tests for Kaggle preview rendering, link allow-listing, schema exhaustiveness, determinism, and CLI/driver errors. |
tests/scripts/test_preview_hf_page.py |
Contract tests for HF preview rendering, link allow-listing, YAML config round-trip, determinism, variant wiring, and CLI/driver errors. |
release/_preview_committed/kaggle.html |
Committed golden Kaggle preview HTML for audit/sync determinism checks. |
release/_preview_committed/huggingface_public.html |
Committed golden HF public preview HTML for audit/sync determinism checks. |
release/_preview_committed/huggingface_instructor.html |
Committed golden HF instructor preview HTML for audit/sync determinism checks. |
pyproject.toml |
Adds markdown-it-py to the [publish] extra; ruff E501 ignores for preview scripts. |
docs/release/preview_pages_design.md |
Design notes/decision table describing preview scripts’ intended behavior and test posture. |
.gitignore |
Ignores generated runtime previews under release/_preview/. |
.agent-plan.md |
Marks PR 7.2 as completed and documents the delivered tooling at a high level. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
A hostile-reviewer pass surfaced one CI blocker and two real fidelity
bugs the first two passes missed. Net diff: -77 lines, leaner code,
honest framing.
BLOCKER — markdown-it-py was in [publish] only; CI's test job
installs only [dev], so every preview test would have ImportError'd
mid-collection. Moved to [dev] (also kept in [publish] so neither
extra is incomplete). Tests run cleanly without the publish extra.
HIGH — Visibility pill removed from the Kaggle preview header.
Kaggle's public dataset page does not display isPrivate; rendering
"Visibility: Private" misled the maintainer about what public viewers
see. New test asserts neither "Visibility:" nor "pill--visibility"
appears in either private or public rendering.
HIGH — HF "Files declared in YAML" section deleted entirely. The
section surfaced an internal concept (configs[].data_files paths
that the configs dropdown already lists) while omitting most of the
real upload tree (manifest.json, tables/*.parquet, etc.). New test
asserts the misleading section is gone.
HIGH — _serve had zero coverage. Refactored: extracted
make_server(directory, port) -> ThreadingHTTPServer as a non-blocking
seam that lets tests bind on port 0, GET /, and shut down cleanly.
New test_make_server_binds_and_serves_index covers handler-factory
construction, address-reuse posture, threaded serve_forever, and
clean shutdown.
MEDIUM — extracted scripts/_preview_common.py with escape, serve,
make_server, _make_handler_factory. Both scripts import from it.
~80 lines of byte-identical duplication eliminated; pass 2's
"below the threshold for extraction" call was wrong — when the
duplicate is exactly identical, the threshold is much lower.
MEDIUM — design doc reframed. Previous language called this an
"audit-artefact-sync" gate; it's actually a regeneration-discipline
gate (a bug in the renderer propagates to both committed sample and
test, so byte-equality alone catches forgotten regenerations, not
correctness — the structural tests are the real audits). Whole
PR repositioned as a "publication-readiness preview", not a
Kaggle/HF look-alike, since pixel fidelity was always out of scope
and the brand-mismatch dominates everything else.
LOW polish:
- PreviewOutcome.cover_path tightened from Path | None to Path
(always set in practice; speculative-flexibility CLAUDE.md
flags as bad).
- _TIER_PATH_RE regex replaced by str.split (clearer, equivalent).
- Module docstrings trimmed; rationale lives in the design doc.
- Variant-equality test in HF tests fixed: replace("public", ...)
was matching "public" inside "publication" in the new footer
copy; switched to the full Variant: <code>X</code> marker.
Net: 1375/1375 tests pass + 5 publish-extra-gated skips; ruff +
ruff format + mypy clean (83 source files); leakage probes 0/3 on
every tier; hash determinism PASS 67/67; validate_release_candidate
--no-rebuild exits 0; BUNDLE_SCHEMA_VERSION unchanged at 5;
validation_report timestamp drift reverted before commit.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This comment has been minimized.
This comment has been minimized.
COPILOT-1 (resolve, accept) — _render_header / _render_footer / _render_cover indexed required metadata keys directly; a malformed dataset-metadata.json or HF README would have raised KeyError mid-render and bypassed main()'s rc=2 translation. Added _validate_required_metadata (Kaggle: title / subtitle / id / expectedUpdateFrequency / image / licenses[0].name) and _validate_required_frontmatter (HF: pretty_name / license, with empty-string-counts-as-missing semantics) to run_preview before the renderer is touched. Both report every missing key in one ValueError, not just the first. COPILOT-2 (resolve, accept with display) — the HF driver was copying release/dataset-cover-image.png into the preview tree without ever rendering it. Lifted _render_cover from the Kaggle script into _preview_common.render_cover (byte-identical helper) and wired it into render_hf_html via a new HF_COVER_IMAGE_FILENAME constant + cover_image_filename kwarg. HF preview now displays the cover under the header pills, mirroring Kaggle's posture and the HF live page. COPILOT-3 (resolve, accept) — singular/plural fix. Instructor sample previously rendered "(1 configs)" / "(1 splits)". Added _preview_common.plural(n, singular, plural=None) and applied to all count headings: Kaggle "(N files)", "(N columns)", "(N tabular files)"; HF "(N configs)", "(N splits)". n=1 now reads naturally; n=0/2/N still pluralises. Tests: 7 new (4 Kaggle, 3 HF + the plural unit test) covering each finding's negative + positive paths. Existing test_run_preview_raises_on_missing_cover_image updated to use a well-formed metadata payload (the new validator runs first and would otherwise short-circuit the cover assertion). Existing "(2 columns across 1 tabular files)" assertion updated to the singular form "(2 columns across 1 tabular file)". Net: 1382/1382 tests pass + 5 publish-extra-gated skips; ruff + ruff format + mypy clean (83 source files); leakage probes 0/3 on every tier; hash determinism PASS 67/67; validate_release_candidate --no-rebuild exits 0; BUNDLE_SCHEMA_VERSION unchanged at 5; validation_report timestamp drift reverted before commit per the brief. Committed Kaggle sample byte-identical (no n=1 case triggered); HF public + instructor samples updated for the cover image + plural fixes. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
pr-agent-context report: No unresolved review comments, failing checks, or actionable patch coverage gaps were found on PR #77 in repository https://github.com/leadforge-dev/leadforge. Treat this PR as all clear unless new signals appear.Run metadata: |
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
Phase 7 middle PR — staging gate before PR 7.3 (
publish_kaggle.py+publish_hf.py+ tag).Two preview scripts let the maintainer render the Kaggle and Hugging Face dataset pages locally from the exact artefacts the publish PR will upload, then click through them in a browser to catch styling / link / YAML-rendering issues before they hit cached previews on the live page.
scripts/preview_kaggle_page.py— readsrelease/kaggle/dataset-metadata.json+ cover image; renders header / cover / description / file tree / schema tables / sources / footer; serves onhttp://localhost:8765.scripts/preview_hf_page.py— readsrelease/huggingface[-instructor]/README.md(YAML frontmatter + body); renders header pills / tag chips / configs dropdown / file tree / README body / footer; serves onhttp://localhost:8766.--variant=public|instructorreads the matching companion README.Both renderers are pure functions: same input → byte-identical HTML. Output landing at
release/_preview/<platform>/is gitignored; committed sample HTML atrelease/_preview_committed/{kaggle,huggingface_public,huggingface_instructor}.htmlis the audit-artefact-sync gate (mirrors PR 4.1 / 5.1 / 5.2 / 7.1).Design
docs/release/preview_pages_design.md(59 lines, decision-table format mirroringllm_critique_design.md) records the ten load-bearing calls: two scripts vs unified renderer; stdlibhttp.server.ThreadingHTTPServervs Flask; f-string templates vs Jinja2;markdown-it-pyvia the[publish]extra (with rationale for why this differs from the PR 5.1 / 5.2 test-only gating); output / sample dirs; cover-image inlining; HF variant flag; CLI shape; audit-sync; test posture (no live HTTP, no BeautifulSoup).Markdown rendering uses
markdown-it-pyingfm-likemode (tables / fenced code on;linkifyexplicitly disabled so the optionallinkify-it-pytransitive dep is not required). Missing dep raises a cleanImportErrorpointing atpip install -e ".[publish]".Tests (48 new, no live HTTP, no network)
For each script, the four roadmap-mandated checks plus pre-flight + determinism + XSS coverage:
https://github.com/leadforge-dev/leadforge,https://huggingface.co/datasets/leadforge, sibling-relativeLICENSE, in-document anchors). Anything else fails the test — guards against](../foo)and](validation/...)regressions if the platform-packager rewriter ever stops firing.dataset-metadata.json::resources[].schema.fieldsappears as<code>{name}</code>in the rendered table.configs[]block in the YAML round-trips into the rendered dropdown.test_committed_*_sample_matches_fresh_regeneration).main().<script>payloads in user-controlled fields render as<script>, not as live tags.Self-review
Hostile pass 1 caught and folded back three findings: BUG (
socketserver.ThreadingTCPServerdoesn't enableallow_reuse_address→ 60s TIME_WAIT on Ctrl-C re-run; switched tohttp.server.ThreadingHTTPServer), DEAD CODE (COMMITTED_SAMPLE_PATH+_VARIANT_SAMPLE_PATHdefined but never read), DOC LIE (cover-image lookup docstring overstated the kaggle-tree-copy preference without acknowledging the gitignore reality). Pass 2 found no significant architectural / scope issues — the ~30 lines of intentional duplication between the two scripts are below the threshold where extracting a_preview_common.pywould pay back.Validation suite
validate_release_candidate.py --no-rebuildexits 0 (3 tiers, 5 seeds, 0 leakage findings)BUNDLE_SCHEMA_VERSIONunchanged at 5release/validation/validation_report.{json,md}timestamp drift reverted before commit per the briefOut of scope
publish_{kaggle,hf}.py+ tag + release notes) — its runbook will cite the two preview commands as a required pre-flight step.BUNDLE_SCHEMA_VERSIONstays at 5.Test plan
pytestgreen (1373 pass + 5 skips)ruff check ./ruff format --check .greenmypy leadforge/greenpython scripts/probe_relational_leakage.py release/{intro,intermediate,advanced} --max-accuracy 0.65→ 0/3 path flag rate per tierpython scripts/verify_hash_determinism.py→ PASS 67/67python scripts/validate_release_candidate.py --no-rebuild→ exit 0python scripts/preview_kaggle_page.py --no-serveround-trips against the committedrelease/_preview_committed/kaggle.htmlpython scripts/preview_hf_page.py --no-serveround-trips againsthuggingface_public.htmlpython scripts/preview_hf_page.py --no-serve --variant=instructorround-trips againsthuggingface_instructor.htmlpython scripts/preview_kaggle_page.py --open-browserandpython scripts/preview_hf_page.py --open-browser(and--variant=instructor) — visual sanity check before requesting review.🤖 Generated with Claude Code