diff --git a/README.md b/README.md index 13916c0..d122933 100644 --- a/README.md +++ b/README.md @@ -51,7 +51,7 @@ These paths are produced by `scripts/generate_reports.py` and are intentionally - `output/skill_trees/`: generated class skill tree Markdown - `output/wiki/`: generated static wiki site and GitHub Pages artifact -Diff report directories contain structured JSON DTOs plus renderer outputs. Browser-friendly HTML entry points live at `index.html`; Markdown reports are still emitted for text review while that renderer remains supported. Legacy direct-command defaults such as `output/item_diff_report/` and `output/excel_diff_report/` are ignored. Prefer `scripts/generate_reports.py` for repeatable project output. +Diff report directories contain structured JSON DTOs plus browser-friendly HTML entry points at `index.html`. Markdown diff reports are no longer generated by the canonical report pipeline. Legacy direct-command defaults such as `output/item_diff_report/` and `output/excel_diff_report/` are ignored. Prefer `scripts/generate_reports.py` for repeatable project output. ## Wiki diff --git a/docs/repo-health-roadmap.md b/docs/repo-health-roadmap.md index c2b2ade..fb86e84 100644 --- a/docs/repo-health-roadmap.md +++ b/docs/repo-health-roadmap.md @@ -50,20 +50,20 @@ Acceptance checks: Purpose: stop carrying a broken renderer as an implied deliverable. -- [ ] Decide whether Markdown diff reports are still needed. +- [x] Decide whether Markdown diff reports are still needed. Decision: stop generating Markdown diff reports in the canonical pipeline; keep JSON DTOs plus HTML reports. - [ ] If Markdown stays: - [ ] Replace LaTeX/color formatting in `MarkdownExporter.get_styled_diffs` with plain Markdown-safe text. - [ ] Add tests for mixed unchanged/changed token diffs such as `dmg-undead` -> `dmg-norm`. - [ ] Verify generated Markdown renders cleanly in GitHub. -- [ ] If Markdown goes: - - [ ] Remove Markdown report outputs from the canonical generator. - - [ ] Keep JSON DTOs and HTML reports as the supported outputs. - - [ ] Update docs and tests to reflect the supported renderer set. +- [x] If Markdown goes: + - [x] Remove Markdown report outputs from the canonical generator. + - [x] Keep JSON DTOs and HTML reports as the supported outputs. + - [x] Update docs and tests to reflect the supported renderer set. Acceptance checks: -- [ ] No generated Markdown report contains malformed `$`, `\text`, or tab-expanded `ext{...}` fragments. -- [ ] HTML report generation remains unchanged for readers. +- [x] No generated Markdown report contains malformed `$`, `\text`, or tab-expanded `ext{...}` fragments. +- [x] HTML report generation remains unchanged for readers. ## Milestone 3: Report Templates And Assets @@ -135,7 +135,7 @@ Acceptance checks: ## Open Questions -- Should Markdown reports be supported long term, or should HTML plus JSON become the only canonical report outputs? +- Markdown diff report decision: HTML plus JSON are the canonical report outputs; Markdown remains only for other currently Markdown-backed export surfaces until later milestones address them. - Should the extracted Diablo II data guide replace the source PDF in `docs/`, or should both remain for traceability? - Should cube recipes become a first-class wiki section? - Should report DTO JSON files be published publicly, or are they only intermediate/local artifacts? diff --git a/scripts/cli/compare_all_excel.py b/scripts/cli/compare_all_excel.py index e8bfcfc..799e679 100644 --- a/scripts/cli/compare_all_excel.py +++ b/scripts/cli/compare_all_excel.py @@ -1,9 +1,17 @@ import os -import sys import argparse from d2lib.repository import D2Repository from d2lib.services import ExcelComparisonService -from d2lib.exporters import HtmlReportExporter, JsonExporter, MarkdownExporter +from d2lib.exporters import HtmlReportExporter, JsonExporter + + +def remove_stale_markdown_reports(report_dir: str) -> None: + if not os.path.isdir(report_dir): + return + for root, _, files in os.walk(report_dir): + for filename in files: + if filename.lower().endswith(".md"): + os.remove(os.path.join(root, filename)) def get_key_column(filename: str) -> str: mapping = { @@ -15,7 +23,7 @@ def get_key_column(filename: str) -> str: return mapping.get(filename, 'code') def main() -> None: - parser = argparse.ArgumentParser(description="Compare two Diablo II Excel directories and export Markdown and HTML diffs.") + parser = argparse.ArgumentParser(description="Compare two Diablo II Excel directories and export JSON and HTML diffs.") parser.add_argument("--new-dir", default="../mods/BKDiablo/bkdiablo.mpq/data/global/excel", help="Path to the new/target Excel directory") parser.add_argument("--old-dir", default="../mods/BTDiablo/btdiablo.mpq/data/global/excel", help="Path to the old/base Excel directory") parser.add_argument("--out", default="../output/excel_diff_report", help="Output directory for generated diff reports") @@ -24,9 +32,9 @@ def main() -> None: bk_dir = args.new_dir bt_dir = args.old_dir report_dir = args.out + remove_stale_markdown_reports(report_dir) repo = D2Repository(".") - exporter = MarkdownExporter() html_exporter = HtmlReportExporter() json_exporter = JsonExporter() summary_rows = [] @@ -41,16 +49,16 @@ def main() -> None: table_bt = repo.load_tsv(os.path.join(bt_dir, filename)) diff = ExcelComparisonService.compare_tables(table_bk, table_bt, get_key_column(filename), filename) - report_name = filename.replace('.txt', '.md') + report_stem = filename[:-4] + report_name = f"{report_stem}.html" json_exporter.export( { "schema": "bt-bkdiff.excel-diff.v1", "diff": diff, }, - os.path.join(report_dir, report_name.replace('.md', '.json')), + os.path.join(report_dir, f"{report_stem}.json"), ) - exporter.export_excel_diff(diff, os.path.join(report_dir, report_name)) - html_exporter.export_excel_diff(diff, os.path.join(report_dir, report_name.replace('.md', '.html'))) + html_exporter.export_excel_diff(diff, os.path.join(report_dir, report_name)) summary_rows.append({ "filename": filename, "report_name": report_name, @@ -61,26 +69,7 @@ def main() -> None: "modified_rows": len(diff["modified_rows"]), }) - summary_lines = ["# Excel Diff Summary\n"] - summary_lines.append("| File | Added Cols | Removed Cols | Added Rows | Removed Rows | Modified Rows |") - summary_lines.append("| :--- | ---: | ---: | ---: | ---: | ---: |") - for row in summary_rows: - summary_lines.append( - f"| [{row['filename']}]({row['report_name']}) | {row['added_cols']} | {row['removed_cols']} | {row['added_rows']} | {row['removed_rows']} | {row['modified_rows']} |" - ) - - total_added_cols = sum(row["added_cols"] for row in summary_rows) - total_removed_cols = sum(row["removed_cols"] for row in summary_rows) - total_added_rows = sum(row["added_rows"] for row in summary_rows) - total_removed_rows = sum(row["removed_rows"] for row in summary_rows) - total_modified_rows = sum(row["modified_rows"] for row in summary_rows) - summary_lines.append( - f"| **Total** | **{total_added_cols}** | **{total_removed_cols}** | **{total_added_rows}** | **{total_removed_rows}** | **{total_modified_rows}** |" - ) - os.makedirs(report_dir, exist_ok=True) - with open(os.path.join(report_dir, "SUMMARY.md"), "w", encoding="utf-8") as f: - f.write("\n".join(summary_lines).strip() + "\n") json_exporter.export( { "schema": "bt-bkdiff.excel-diff-summary.v1", diff --git a/scripts/cli/compare_item_db.py b/scripts/cli/compare_item_db.py index 7d5620e..1e77c70 100644 --- a/scripts/cli/compare_item_db.py +++ b/scripts/cli/compare_item_db.py @@ -3,10 +3,18 @@ import json import glob import argparse -from typing import Dict, List, Any -from d2lib.repository import D2Repository +from typing import Dict, Any from d2lib.services import ItemComparisonService -from d2lib.exporters import HtmlReportExporter, JsonExporter, MarkdownExporter +from d2lib.exporters import HtmlReportExporter, JsonExporter + + +def remove_stale_markdown_reports(report_dir: str) -> None: + if not os.path.isdir(report_dir): + return + for root, _, files in os.walk(report_dir): + for filename in files: + if filename.lower().endswith(".md"): + os.remove(os.path.join(root, filename)) def load_json_db(db_dir: str) -> Dict[str, Dict[str, Any]]: # type -> id -> item @@ -36,7 +44,7 @@ def load_json_db(db_dir: str) -> Dict[str, Dict[str, Any]]: return types def main() -> None: - parser = argparse.ArgumentParser(description="Compare two exported item databases and generate Markdown and HTML reports.") + parser = argparse.ArgumentParser(description="Compare two exported item databases and generate JSON and HTML reports.") parser.add_argument("--new-db", default="../exports/item_db", help="Path to the new/target exported JSON item database") parser.add_argument("--old-db", default="../exports/item_db_bt", help="Path to the old/base exported JSON item database") parser.add_argument("--out", default="../output/item_diff_report", help="Output directory for generated diff reports") @@ -45,6 +53,7 @@ def main() -> None: bk_json_dir = args.new_db bt_json_dir = args.old_db out_dir = args.out + remove_stale_markdown_reports(out_dir) print("Loading Item Databases...") bk_db = load_json_db(bk_json_dir) @@ -86,23 +95,9 @@ def main() -> None: os.path.join(out_dir, "diff.json"), ) - exporter = MarkdownExporter() - exporter.export_item_diff(combined_diff, out_dir) html_exporter = HtmlReportExporter() html_exporter.export_item_diff(combined_diff, out_dir, type_counts=type_counts) - - # Inject breakdown into SUMMARY.md - summary_path = os.path.join(out_dir, "SUMMARY.md") - with open(summary_path, 'w', encoding='utf-8') as f: - f.write("# Item Database Comparison Summary\n\n") - f.write("| Category | [Added](ADDED.md) | [Removed](REMOVED.md) | [Modified](MODIFIED.md) |\n") - f.write("| :--- | :---: | :---: | :---: |\n") - for t in ['uniques', 'sets', 'runewords']: - c = type_counts[t] - f.write(f"| {t.capitalize()} | {c['added']} | {c['removed']} | {c['modified']} |\n") - f.write(f"| **Total** | **{len(combined_added)}** | **{len(combined_removed)}** | **{len(combined_modified)}** |\n\n") - f.write("Click the links in the header to see detailed breakdowns.\n") - + print(f"Reports generated in {out_dir}") if __name__ == "__main__": diff --git a/scripts/d2lib/exporters.py b/scripts/d2lib/exporters.py index 3038069..66641ed 100644 --- a/scripts/d2lib/exporters.py +++ b/scripts/d2lib/exporters.py @@ -685,7 +685,9 @@ def export_excel_summary(self, summary_rows: List[Dict[str, Any]], output_path: self._ensure_assets(os.path.dirname(output_path)) rows = [] for row in summary_rows: - html_report = str(row["report_name"]).replace(".md", ".html") + html_report = str(row["report_name"]) + if html_report.endswith(".md"): + html_report = html_report[:-3] + ".html" rows.append( f"{self.escape(row['filename'])}" f"{row['added_cols']}{row['removed_cols']}" diff --git a/tests/test_html_report_exporter.py b/tests/test_html_report_exporter.py index a9b2ad8..80e628e 100644 --- a/tests/test_html_report_exporter.py +++ b/tests/test_html_report_exporter.py @@ -84,7 +84,7 @@ def test_exports_excel_diff_pages(self): [ { "filename": "gems.txt", - "report_name": "gems.md", + "report_name": "gems.html", "added_cols": 1, "removed_cols": 0, "added_rows": 1, diff --git a/tests/test_report_cli_outputs.py b/tests/test_report_cli_outputs.py new file mode 100644 index 0000000..438025d --- /dev/null +++ b/tests/test_report_cli_outputs.py @@ -0,0 +1,111 @@ +import os +import json +import sys +import tempfile +import unittest + + +SCRIPT_DIR = os.path.join(os.path.dirname(os.path.dirname(os.path.abspath(__file__))), "scripts") +if SCRIPT_DIR not in sys.path: + sys.path.insert(0, SCRIPT_DIR) + +from cli import compare_all_excel, compare_item_db + + +def run_cli(main_func, args): + old_argv = sys.argv[:] + try: + sys.argv = [main_func.__module__] + args + main_func() + finally: + sys.argv = old_argv + + +def markdown_files_under(root): + found = [] + for current_root, _, files in os.walk(root): + for filename in files: + if filename.lower().endswith(".md"): + found.append(os.path.relpath(os.path.join(current_root, filename), root)) + return found + + +class TestReportCliOutputs(unittest.TestCase): + def test_excel_comparison_exports_json_and_html_without_markdown(self): + with tempfile.TemporaryDirectory() as temp_dir: + old_dir = os.path.join(temp_dir, "old") + new_dir = os.path.join(temp_dir, "new") + out_dir = os.path.join(temp_dir, "report") + os.makedirs(old_dir) + os.makedirs(new_dir) + os.makedirs(out_dir) + + with open(os.path.join(out_dir, "SUMMARY.md"), "w", encoding="utf-8") as f: + f.write("stale markdown") + with open(os.path.join(old_dir, "gems.txt"), "w", encoding="utf-8") as f: + f.write("name\tweaponMod1Code\nBer Rune\tdmg-undead\n") + with open(os.path.join(new_dir, "gems.txt"), "w", encoding="utf-8") as f: + f.write("name\tweaponMod1Code\nBer Rune\tdmg-norm\n") + + run_cli( + compare_all_excel.main, + ["--new-dir", new_dir, "--old-dir", old_dir, "--out", out_dir], + ) + + self.assertTrue(os.path.exists(os.path.join(out_dir, "gems.html"))) + self.assertTrue(os.path.exists(os.path.join(out_dir, "gems.json"))) + self.assertTrue(os.path.exists(os.path.join(out_dir, "summary.json"))) + self.assertTrue(os.path.exists(os.path.join(out_dir, "index.html"))) + self.assertEqual([], markdown_files_under(out_dir)) + + with open(os.path.join(out_dir, "summary.json"), "r", encoding="utf-8") as f: + summary = json.load(f) + self.assertEqual("gems.html", summary["files"][0]["report_name"]) + + def test_item_comparison_exports_json_and_html_without_markdown(self): + with tempfile.TemporaryDirectory() as temp_dir: + old_db = os.path.join(temp_dir, "old_db") + new_db = os.path.join(temp_dir, "new_db") + out_dir = os.path.join(temp_dir, "report") + os.makedirs(os.path.join(old_db, "uniques")) + os.makedirs(os.path.join(new_db, "uniques")) + os.makedirs(os.path.join(out_dir, "nested")) + + with open(os.path.join(out_dir, "SUMMARY.md"), "w", encoding="utf-8") as f: + f.write("stale markdown") + with open(os.path.join(out_dir, "nested", "old.md"), "w", encoding="utf-8") as f: + f.write("stale markdown") + + old_item = { + "id": "sample-helm", + "name": "Sample Helm", + "display_name": "Sample Helm", + "base_item": "Cap", + "item_type": "Helm", + "lvl_req": "3", + "properties": [{"resolved_text": "+5 to Life"}], + "raw_row": {}, + } + new_item = dict(old_item) + new_item["lvl_req"] = "12" + new_item["properties"] = [{"resolved_text": "+15 to Life"}] + + with open(os.path.join(old_db, "uniques", "helms.json"), "w", encoding="utf-8") as f: + json.dump([old_item], f) + with open(os.path.join(new_db, "uniques", "helms.json"), "w", encoding="utf-8") as f: + json.dump([new_item], f) + + run_cli( + compare_item_db.main, + ["--new-db", new_db, "--old-db", old_db, "--out", out_dir], + ) + + self.assertTrue(os.path.exists(os.path.join(out_dir, "diff.json"))) + self.assertTrue(os.path.exists(os.path.join(out_dir, "index.html"))) + self.assertTrue(os.path.exists(os.path.join(out_dir, "ADDED.html"))) + self.assertTrue(os.path.exists(os.path.join(out_dir, "MODIFIED.html"))) + self.assertEqual([], markdown_files_under(out_dir)) + + +if __name__ == "__main__": + unittest.main()