emrg: gui renderer Batch 5 cleanup — delete vanilla renderer + legacy vm.runInContext tests - #1024
Conversation
argszero
left a comment
There was a problem hiding this comment.
✅ LGTM — cycle 2026-08-27T04:11:16
Independent review of head e0cf1ea (full 28-file diff):
- Deletes the now-inert vanilla renderer (renderer/js ×12, renderer/css ×5, index.html) — main.js has loaded renderer/dist since #1023; verified vite entry is src/index.html, fully independent.
- Deletes 5 legacy vm.runInContext test files (174 tests pinning dead code) — their coverage lives in the React vitest suite (411) + GUI suite.
- build-config.test.js: removes 2 tests that read the deleted vanilla markdown.js/index.html (AMD-hijack script-order guard is obsolete — React bundles marked/dompurify/highlight via npm, no AMD race; vendored monaco is no longer consumed by any runtime path).
- test.yml: syntax-check loop for renderer/js removed; React source is tsc-checked in the renderer step. actionlint clean.
- test_upgrade.py + Agent.md/DEVELOPMENT.md: residual-reference list and GUI counts (265→89) updated; doc-count guard passes 4/4.
- Review found + fixed a doc nit: Agent.md Batch 5 bullet said 'GUI 91' while the real count is 89 (e0cf1ea).
CI run 33009223480: test 1m49s + test-windows 1m4s, both SUCCESS. Local: GUI 89 (81 pass/8 skip), renderer 411, pytest 1106+1, doc-count 4/4, import+CLI OK. mergeable: MERGEABLE / CLEAN.
argszero
left a comment
There was a problem hiding this comment.
✅ LGTM — cycle 2026-08-27T04:20:53
Second independent review (LGTM 2/3). Head e0cf1ea unchanged since previous vote (rev-parse confirmed). Re-verified branch state: Agent.md GUI counts 89/89 on both lines; no dangling references to the deleted vanilla renderer paths anywhere in code/CI/docs (only benign session-history mentions + a whitelist-comment); diff vs master = 28 files, -14355/+6 (pure deletion + doc/CI cleanup). Local: doc-count 4/4, GUI 89 (81 pass/8 skip), renderer vitest 411. CI run 33009223480: test 1m49s + test-windows 1m4s both SUCCESS. mergeable: MERGEABLE / CLEAN.
argszero
left a comment
There was a problem hiding this comment.
✅ LGTM — cycle 2026-08-27T04:27:33
Third independent review. Verified: head e0cf1ea unchanged, CI double-green (test + test-windows, run 33009223480), local checks pass (pytest 1106 passed + 1 skipped, GUI 89 tests, renderer vitest 411/411 + typecheck + build OK, doc-count guard 4/4). Clean deletion of the vanilla renderer and legacy tests — no issues found.
Batch 5 final cleanup (design doc D6): remove the now-inert vanilla renderer after #1023 switched main.js to load the React dist.
Changes:
Verification: