Conversation
PR Review: BBC News Test DataThanks for contributing test data for a real-world HTML-to-Markdown transformation scenario. Here is my feedback. Critical: No test code consuming these files The PR adds two data files under commonTest/data/01-bbc-news/ but there is no corresponding test that reads them and asserts anything. The existing TransformerTest.kt builds its inputs programmatically via the semanticEvents {} DSL and does not read files from disk. Without a test, this data is unreachable dead code that will never run or fail a CI build. The PR needs a new @test function (or new test file) that reads bbc-news.html, parses it, transforms it, and compares the result to bbc-news.md. This also requires a multiplatform file-reading mechanism: since the library targets JVM, JS (Node + browser), WASM, and native, loading files from commonTest requires either expect/actual file-reading helpers per platform or a shared test-resource utility. The browser target cannot access the filesystem at all, so this needs careful design. Critical: Copyright of scraped BBC content bbc-news.html appears to be a live capture of the BBC News homepage, containing BBC trademarks, branding, and editorial content. Adding an Apache 2.0 license header to this file does not relicense the BBC content inside it. Committing scraped third-party web pages into an open-source project under a permissive license is a potential copyright and Terms of Service violation. Consider replacing it with a trimmed representative excerpt or a synthetic HTML fixture that mimics real-world structure (nav, article, header, footer, aside) without containing proprietary content. Significant: HTML file contains minified JavaScript and ad-tech blobs bbc-news.html contains large inline script blocks with analytics configuration, ad-targeting data, and flagpole flags. A test fixture should be trimmed to elements that actually exercise the transformation logic -- the JS payloads add significant size without testing anything meaningful. Significant: Missing trailing newline in bbc-news.md The diff shows 'No newline at end of file' at the end of bbc-news.md. This should be fixed. Minor: PR title and description The PR title 'bbc news' provides no context. A title like 'Add BBC News real-world test fixture for HTML-to-Markdown transformation' would help reviewers and future git log readers understand the intent. A PR body describing the purpose and how the fixture was generated would also be helpful. What looks good The directory naming convention (01-bbc-news/) with a numeric prefix is a clean way to order fixtures for a growing test suite. The bbc-news.md output shows thoughtful transformation decisions: YAML front matter for head metadata, svg/ as a placeholder for icon SVGs, golem-id attributes preserved on interactive elements, and inline Markdown for ul/li navigation. This is a useful design reference. The Apache 2.0 license header is present on both files. Summary of requested changes
|
There was a problem hiding this comment.
The footer navigation and language list are useless.
Social follow buttons and Cookie consent section at the bottom are also useless.
Duplicate nav (header buttons +
There was a problem hiding this comment.
Regarding footer navigation language list, and other useless sections - even if they are useless, we could only eliminate them by heuristics and transformers defined per website. This is something worth considering in the future, but I don't think we can have a conditional logic for skipping them now. The transformer should strip out the style and leave meanings, but remain generic at the same time. The same applies to repetitions in navigation.
Code Review: PR #24 — BBC News & DSGVO test fixturesThanks for opening this PR with real-world transformation examples. I have reviewed the three files added and have the following feedback. Critical: 02-dsgvo/dsgvo.html has no corresponding .md fileThe BBC fixture pair is complete (bbc-news.html + bbc-news.md), but the DSGVO directory contains only dsgvo.html with no dsgvo.md counterpart. Without the expected-output .md file, the DSGVO fixture cannot be used as a test case. Either complete the pair before merging, or move the dsgvo.html to a follow-up PR once the semantic Markdown representation is ready. Critical: Both HTML files are massive full-page captures (300+ KB each)bbc-news.html is ~316 KB, and dsgvo.html is ~332 KB — over 640 KB of minified content committed to the test tree. Problems with this:
The right approach is a trimmed synthetic fixture: keep the structural HTML elements that exercise the transformer logic (nav, header, article, h1/h2/p, ul/li, figure, footer) but strip everything else. A well-crafted 50-100 line fixture tests the same semantics without the size cost. Significant: Missing trailing newline at end of bbc-news.mdThe file ends at line 640 without a trailing newline. Most editors, linters, and POSIX tools expect a trailing newline. This should be fixed. Significant: Copyright still applies to bbc-news.md contentThe existing review already flagged bbc-news.html. Worth noting that bbc-news.md reproduces the same third-party content in transformed form: article titles, image alt text, navigation labels, and the explicit footer notice "Copyright 2026 BBC. All rights reserved." Adding an Apache 2.0 header does not clear the underlying copyright. In contrast, the DSGVO content is EU law (GDPR), which is generally public domain for reproduction purposes under EU legal acts policy — so the copyright situation there is genuinely different and more permissive. Minor: Semantic conventions in bbc-news.md are implicitThe markdown uses mixed conventions that would benefit from documentation:
These decisions look reasonable. Capturing them as a brief comment in the .md file or a README.md in the test-data directory would help future contributors write the transformer correctly. What looks good
Summary of requested changes
|
Code ReviewThanks for putting together these real-world test fixtures — grounding the transformer work in concrete examples is a great approach, and the variety across BBC News, DSGVO, and EUR-Lex covers meaningfully different document structures. Critical: Missing
|
Follow-up Review: New Test Cases (02-dsgvo, 03-eul)Thanks for expanding the fixture set. Here is focused feedback on the two new pairs added since my previous comment. Critical: Browser extension artifacts in the HTML filesBoth <script src="chrome-extension://lgblnfidahcdcjddiepkckcfdhpknnjh/content/popups-script.js"></script>
<style id="stndz-cookie-banners-1">...</style>This is the "Standz" cookie-banner-blocker extension. The issues are:
Please strip these tags before committing. A clean scrape can be produced with Significant: eul.html is 11,971 linesA test fixture does not need to contain the entire page in its original form. Positive: Copyright situation is much better hereDSGVO/GDPR text is an EU regulation, and EUR-Lex content is published under the EU Open Data licence — both are free to reproduce. This is a significant improvement over the BBC fixture where copyright is ambiguous. Good choice of test cases. Positive: dsgvo.md qualityThe Reminder from previous review (still open)The most important outstanding issue is that no test reads any of these files. All six fixtures are currently dead code. The minimum needed before merging is at least one Summary of changes still needed
|
Code ReviewThanks for setting up this test data foundation! The approach of pairing real-world HTML dumps with expected Markdown output is a solid strategy for driving transformer development. Here are observations and suggestions: Blocking Issues1. Missing The directory 2. No test code consuming these fixtures The PR adds test data but no automated test code that reads and asserts against it. While the description says this is intentional ("we can do automated test for this later"), this creates orphaned fixtures that can silently drift out of sync with the implementation. At minimum, consider:
The existing Significant Concerns3. Copyright of BBC News content The files in Suggestion: Replace real article content with a minimal fabricated example that exercises the same HTML structure, or use a clearly public-domain source. The DSGVO/EUR-Lex examples (EU law text) are much safer choices as EU legal texts are public domain. 4. The <button aria-label="Open menu" golem-id="0">These are not present in the original BBC HTML — they appear to have been injected by a browser extension or preprocessing tool (likely something called "Golem"). This means:
Please document: what tool generated these, are the Minor Issues5. Minified HTML makes diffing difficult
6. Copyright year inconsistency
7. Naming clarity
Positive Observations
SummaryThe core idea is sound and the Markdown output quality looks thoughtful. The main things to address before merging: resolve the missing |
Follow-up Review: 04-eule fixture + open blockers summaryGood progress adding the New:
|
| # | Issue | File(s) |
|---|---|---|
| 1 | No test code reads these fixtures — all 8 files are dead code with no CI coverage | All |
| 2 | BBC copyright: bbc-news.html body is © BBC; the Apache 2.0 header on the file does not relicense the content |
01-bbc-news/ |
| 3 | Missing trailing newline in bbc-news.md |
bbc-news.md |
| 4 | Browser extension markup (chrome-extension://lgblnfidahcdcjddiepkckcfdhpknnjh, stndz-cookie-banners-1) present in HTML fixtures |
02-dsgvo/, 03-eul/, 04-eule/ |
What looks good in 04-eule
eule.mdis well-structured at 106 lines — nicely scoped fixture covering nav, main sections, and institutional browsing.- EUR-Lex content is published under the EU Open Data licence, so there are no copyright concerns here (same as
03-eul). - The HTML/Markdown ratio (4704 lines → 106 lines) demonstrates a meaningful compression that makes the transformation goal concrete.
remove bloat in <html>
Consolidated Review: All 8 Fixtures Now PresentAll four fixture pairs are now committed (bbc-news, dsgvo, eul, eule). Good progress! Below is a consolidated summary of what is still outstanding, combining observations from the full set of files. Blockers before merging1. No test code runs these fixtures All 8 files are dead code — no
2. BBC copyright — The HTML file is a scraped BBC News homepage (© BBC), and
3. Browser extension artifacts in All three non-BBC HTML files contain markup injected by the "Standz" cookie-banner-blocker extension: <script src="chrome-extension://lgblnfidahcdcjddiepkckcfdhpknnjh/content/popups-script.js"></script>
<style id="stndz-cookie-banners-1">...</style>Issues:
Fix: re-capture with 4. Missing trailing newline in The file ends at line 640 without a Significant concerns5. The EUR-Lex full-page capture is very large for a test fixture. Consider trimming it to the structural elements actually represented in 6. Leading blank line in
7. The Markdown output contains What is in good shape
Summary of changes needed before merging
|
remove bloat from <html>
PR ReviewThis PR adds four HTML + Markdown fixture pairs as test data for the What's Good
Issues / Suggestions1. No test code wires up these fixturesThe PR adds data under 2. Browser automation artefacts in
|
remove bloat from <html>
PR Review: Test Data for Semantic Markdown TransformationThanks for sharing these examples — having real-world input/output pairs is exactly the right approach for driving the transformer design. Here's my review. What this PR doesAdds four pairs of test data under
Issues & Suggestions1. No test code wires these data files up yetThe PR description acknowledges this is intentional ("so that we can do automated test for this later"), but it means this data sits in 2. Copyright header on third-party scraped content (
|
|
PR Review: Test Data for HTML to Semantic Markdown Transformer Great work establishing the test data structure! Having real-world HTML samples from BBC News, DSGVO, and EUR-Lex covering news, legal, and structured document domains gives excellent coverage for the transformer. Here are observations and suggestions. STRUCTURAL ISSUES
SIZE CONCERNS
CONSISTENCY ISSUES
DESIGN QUESTIONS
MINOR
SUMMARY Happy to look at a follow-up PR with the test harness once the data shape is finalised. |
Code Review — PR #24 (follow-up)Previous reviews have covered the major issues thoroughly (no test code, copyright on BBC content, browser-extension artifacts, trailing newline, and file size). This comment focuses on what has changed and a few new observations. What has improved since the previous reviews
New concern:
Consider trimming Inconsistency in In Open issues from previous reviews still unresolved
Suggestion: add a Given the design decisions embedded in these fixtures (which tags are preserved raw vs. converted, how SVGs are handled, what |
fix <html>
fix <html>
fix <html>
Consolidated Review SummaryPrevious reviews on this PR have been thorough. This comment synthesizes the outstanding blockers into a single actionable list to avoid any ambiguity about what is needed before merging. Blockers (must fix before merging)1. No test code exercises these fixtures All 8 files are currently dead code — nothing reads them, transforms them, or asserts against them. CI cannot catch regressions. At minimum, add one of:
2. BBC copyright in
3. Browser extension markup in All three files contain markup injected by the Standz extension: <script src="chrome-extension://lgblnfidahcdcjddiepkckcfdhpknnjh/content/popups-script.js"></script>
<style id="stndz-cookie-banners-1">...</style>The extension ID is a contributor fingerprint that has no place in a public repo, and the injected CSS is not from the source sites. Re-capture with 4. Missing trailing newline in The file ends at line 640 without a Significant (should fix, not hard blockers)5. This is very large for a test fixture and will slow CI on JS, WASM, and native targets. 6. Leading blank line in YAML front matter must start on line 1. A leading blank line causes the 7. Attributes like What looks good
|
|
Consolidated Review - All 4 Fixture Pairs. See full review in next comment. |
| --- | ||
|
|
||
| <nav> | ||
| - <a href="https://dsgvo-gesetz.de/">DSGVO</a> |
There was a problem hiding this comment.
I think simple links would be converted by default to something like:
[DSGVO](https://dsgvo-gesetz.de/)Only elements containing complex content would be preserved.
|
@claude please fix new md files according to: <a href="https://dsgvo-gesetz.de/">DSGVO</a>simple links should be converted by default to something like: [DSGVO](https://dsgvo-gesetz.de/)Only elements containing complex content would be preserved as |
|
Claude finished @morisil's task in 2m 31s —— View job Converted simple
|
Replace <a href="url">plain text</a> with [plain text](url) across all MD fixture files. Links containing nested HTML elements (button, svg, img, etc.) are preserved as raw HTML. Co-authored-by: Kazik Pogoda <morisil@users.noreply.github.com>
PR Review:
|
| Issue | Severity |
|---|---|
Missing eulaw-live.md |
Blocker |
| No test code consuming the data | Should fix (or track) |
| Apache 2.0 header on BBC HTML | Should fix |
Malformed <html> tag in eul/eule HTML |
Minor |
| Nav link format inconsistency | Minor |
golem-id undocumented |
Minor |
| Confusing eul/eule naming | Suggestion |
| Oversized HTML dumps | Suggestion |
The direction is right — these test cases are valuable. Address the blocker and IP issue before merging, and consider the rest as improvements to make the suite more maintainable.
Follow-up Review: 05-eulaw-live fixture + open issues statusThis is a focused follow-up on the new 05-eulaw-live pair (the only fixture not covered by prior reviews) plus a concise status check on previously raised blockers. 05-eulaw-live: what is improved
05-eulaw-live: new copyright concerneulawlive.com is a commercial subscription publication — EU Law Live is a private media outlet, not an EU institution. Reproducing headlines and article titles from a subscription publication in an open-source repo under Apache 2.0 carries a similar copyright concern as the BBC fixture. This content is not EU open-data licensed (unlike EUR-Lex/DSGVO). Consider replacing with fabricated-but-structurally-equivalent article titles, or verify this is acceptable under the site terms of service. Status of previously raised blockers
The 05-eulaw-live pair is the cleanest fixture added so far — good direction. Addressing the browser extension cleanup and golem-id artefacts in particular would get this PR to a mergeable state. |
rewrote headlines and article titles into gibberish to solve the concerns
PR Review: HTML-to-Markdown Test Data (updated review covering all 5 cases)This is a thoughtful collection of real-world test cases for an HTML→Semantic Markdown transformer. The approach of capturing HTML source alongside a hand-crafted expected Markdown output is the right direction for golden-file testing. Here is structured feedback on the full set of files. Still needed: test codeNone of the five cases are wired to an actual
Copyright / licensing — per file
File size concern
Minor issues
SummaryThe structural approach and the DSGVO/EU law cases are solid. The main action items before merging are:
|
introduce gibberish to solve concerns
trimmed the html by preserving a representative excerpt to reduce the size of the document.
Follow-up Review: New Test Fixtures (02-05)The PR has grown since the earlier review to include four more fixture sets. Here is feedback specific to the new additions. 02-dsgvo and 04-eule — Same copyright concern as BBCBoth files come from dsgvo-gesetz.de, a commercial WordPress site (evident from the Matomo tracking code, WP-Rocket, Yoast SEO meta, and pingback URLs). While the underlying GDPR text (DSGVO) is an EU regulation and public domain in the EU, the HTML structure, annotations, and formatting from dsgvo-gesetz.de are the site's own work. Committing a full scrape under Apache 2.0 has the same copyright risk flagged for the BBC file. Suggestion: Use EUR-Lex (europa.eu) as the source for the raw regulation text instead. 03-eul — EUR-Lex source is the right choice
05-eulaw-live — Good use of anonymised synthetic contentThe One observation: the HTML input file already contains a YAML front matter block and inline Markdown syntax. If this is intentional (the transformer is expected to handle a mixed HTML+Markdown+front-matter format), that design assumption should be documented either as a comment in the file or in the PR description. Missing licence headers on new files
Still no test codeThis applies to all five fixture sets. The data remains unreachable dead code until a test reads it. A multiplatform file-reading mechanism is needed before any of these can be exercised from Size summary
Summary of new requested changes
|
Code ReviewThanks for putting together these test fixtures. Having real-world HTML/Markdown pairs is exactly the right approach for driving the transformer implementation. Critical Issues1. eulaw-live.html has YAML frontmatter, making it invalid HTML The file starts with a YAML frontmatter block which is not valid HTML. The frontmatter convention should only appear in the .md output file. 2. eulaw-live HTML and MD titles are inconsistent The HTML frontmatter title is "Blorbex Law Vingle" but the MD reads "Blorf Wumple Zrax". If the MD is the expected transformation output of the HTML, these must match. This suggests both files were generated independently, which invalidates this as a test pair. 3. eule.html has a malformed opening tag and is apparently mismatched with its MD The file opens with invalid HTML (the opening tag is malformed). The HTML content also appears to be from the DSGVO website (German, same WordPress/Matomo structure as 02-dsgvo) while eule.md represents the EUR-Lex homepage in English (lang: en). These are not a genuine HTML-to-Markdown pair. Significant Issues4. No test code references the data files TransformerTest.kt does not load or reference any files in commonTest/data/. The PR adds ~7500 lines of test data with no test code that uses it. Shipping test code and data together makes it easier to verify the fixture format is correct. 5. Missing trailing newline in bbc-news.md The diff ends with no newline at end of file for 01-bbc-news/bbc-news.md. Moderate Issues6. Copyright concern with bbc-news.html The file carries an Apache 2.0 license header from Kazimierz Pogoda / Xemantic, but the content is scraped from BBC News. Bundling third-party copyrighted content under Apache 2.0 is problematic. Consider replacing real article text with placeholder copy (like the eulaw-live approach) or stripping to a structural skeleton. The other HTML files in this PR carry no Apache headers, making the treatment inconsistent. 7. eule.html is 4704 lines Very large fixtures can slow tests running across JVM, JS, WASM, and native targets on every push. Consider trimming to the minimum structure needed. Minor
SummaryPairs 01-03 (bbc-news, dsgvo, eul) are in reasonable shape aside from the copyright header and missing newline issues. Pairs 04 (eule) and 05 (eulaw-live) need rework before they can serve as reliable test fixtures. |
|
@claude, please fix Critical Issue 1. eulaw-live.html has YAML frontmatter, making it invalid HTML. The file starts with a YAML frontmatter block which is not valid HTML. The frontmatter convention should only appear in the .md output file. |
|
Claude finished @kodehaven's task in 1m 1s —— View job Fixed YAML frontmatter in eulaw-live.html
Done. Removed the 7-line YAML frontmatter block (lines 1–6 plus trailing blank line) from |
YAML frontmatter is not valid HTML and should only appear in the corresponding .md output file, not in the HTML input fixture. Co-authored-by: todorovs <kodehaven@users.noreply.github.com>
|
@claude please fix critical issue 2. eulaw-live HTML and MD titles are inconsistent. |
|
Claude finished @kodehaven's task in 1m 5s —— View job Fixed eulaw-live title inconsistency
Done. Updated
Since |
Update eulaw-live.md title to match eulaw-live.html:
- Frontmatter title: 'Blorf Wumple Zrax – Grondex' → 'Blorbex Law Vingle – Grondex'
- H1 heading: '# Blorf Wumple Zrax' → '# Blorbex Law Vingle'
The HTML file is the source; the MD is the expected transformation output,
so both must use the same obfuscated title ('Blorbex Law Vingle').
Co-authored-by: todorovs <kodehaven@users.noreply.github.com>
|
@claude, please fix critical issue 3. eule.html has a malformed opening tag and is apparently mismatched with its MD. |
|
Claude finished @kodehaven's task in 1m 40s —— View job Fixed Critical Issue 3: eule.html malformed and mismatched with eule.md
What was doneReplaced the 4704-line DSGVO scrape with a clean 108-line EUR-Lex homepage HTML file that:
|
The previous eule.html was from the DSGVO website (dsgvo-gesetz.de), German content with a malformed opening tag (<html lang="de" <head>). eule.md represents the EUR-Lex homepage in English, so the HTML must match. Replaced with clean, properly structured EUR-Lex homepage HTML that contains the nav and section structure corresponding to eule.md. Co-authored-by: todorovs <kodehaven@users.noreply.github.com>
|
@claude, please fix issue 7. eule.html is 4704 lines. Very large fixtures can slow tests running across JVM, JS, WASM, and native targets on every push. Consider trimming and keeping a representative excerpt of the html code. Adjust the .md file accordingly if needed. |
|
Claude finished @kodehaven's task in 46s —— View job Trimming eule.html to representative excerpt
Result: Already resolvedThis issue was already addressed when Critical Issue 3 was fixed. The previous task replaced the 4704-line DSGVO scrape with a clean, purpose-built EUR-Lex homepage HTML. Current state:
The pair is already representative and appropriately scoped. No further changes are needed. |
|
@claude dsgvo and eul are in reasonable shape aside from the copyright header and missing newline issues - fix the remaining issues with those files. Your next goal is: Pairs 04 (eule) and 05 (eulaw-live) need rework before they can serve as reliable test fixtures - please fix accordingly. |
|
Claude finished @kodehaven's task in 9m 36s —— View job Fixed Remaining Test Fixture Issues
Summary of changes02-dsgvo/dsgvo.html
03-eul/eul.html
03-eul/eul.md
04-eule/eule.html
04-eule/eule.md
05-eulaw-live/eulaw-live.html
05-eulaw-live/eulaw-live.md
|
02-dsgvo/dsgvo.html: remove browser extension style tag artifact, add source attribution comment, remove stray '---' after </html> 03-eul/eul.html: add source attribution comment 03-eul/eul.md: remove stray code fence (```) from end of file 04-eule/eule.html: add source attribution comment 04-eule/eule.md: fix nav links to use '- ' list markers (consistent with dsgvo.md format) 05-eulaw-live/eulaw-live.html: rewrite as proper HTML document with doctype, head metadata, and body; convert Markdown heading/link syntax to HTML tags (<h1>, <h2>, <a href>) 05-eulaw-live/eulaw-live.md: align article text with HTML source (consistent obfuscated words across both files) Co-authored-by: todorovs <kodehaven@users.noreply.github.com>
|
@kodehaven I am preparing to close this one, since recent changes addressed it:
If you want to retain any of the test cases from this PR, I would suggest to start new PR for each website to test, and the assertion can be completely generated now with the new tranformHtmlToMarkdown(). I am also preparing a "Web as Markdown" browser for rapid testing - see the world the way LLM will see it. |
|
@morisil you can close it. |
@kodehaven @anakori I did analysis how BBC news would look in our semantic markdown. Please take a look and let me know what you think.
I include the orginal HTML dump, so that we can do automated test for this later. We can continue with other test cases, and then let AI code the whole transformer. But these test cases are far more important. @kodehaven will prepare some examples related to retrieval of GDPR and EU law. You can follow the same structure of HTML dump / Markdown assertion.