Topic9：DSL错误提示美化器设计文档和开发文档

[mahiru114514]

Summary

• add a design document for integrating structured diagnostics into the DSL parsers
• add a development guide covering implementation stages, testing, error recovery, and acceptance criteria
• distinguish current repository behavior from proposed functionality

Scope

Documentation only. No compiler source code or tests are changed.

Validation

• Markdown rendering passed
• local links and code fences checked
• independent documentation review: 0 critical and 0 important issues
• project L2 tests were not run because the local harness and Python test dependencies were unavailable

[github-actions]

🤖 AI Code Review

共审查 10 个变更文件
⚠️ 另有 8 个文件超过上限（最多 10 个）未审查

📁 .github/workflows/ci.yml

🟡 BASE_SHA fallback semantics wrong on push events — Line 193: github.event.pull_request.base.sha || github.event.before
When triggered by a push (not a PR), this falls back to github.event.before — the previous commit on the same branch. So a direct push to a feature branch benchmarks against its own parent, not against main. This produces misleading regression results.
Suggestion: Either guard this step with if: github.event_name == 'pull_request' (or 'pull_request_target'), or explicitly resolve the main branch SHA via git merge-base HEAD origin/main.

🟡 upload-artifact now fires on every event — Line 118: Changed github.ref == 'refs/heads/main' to always().
This uploads test artifacts on every PR, every push, every fork trigger. Could clutter CI and increase storage costs with no clear user need on non-main events.
Suggestion: Keep always() but scope it: if: always() && github.ref == 'refs/heads/main', or at minimum add if: always() && failure() to only upload on failure.

---

📁 benchmarks/bench_dsl_diagnostics.py

🔴 Unhandled crash: missing optional dependency — build_report lines with "dependencies": {name: importlib.metadata.version(name) for name in ("onnx", "numpy")} is constructed outside the try block. importlib.metadata.version raises PackageNotFoundError when onnx/numpy isn't installed (e.g., a fresh worker image that only needs the parser), so the script dies with a raw traceback and produces no report at all. Move environment/dependency collection inside the try, or wrap each lookup in a per-package fallback (None).

🔴 ZeroDivisionError in compare_parsing — ratio = current["median_s"] / baseline["median_s"]. A tiny corpus or a heavily optimized baseline can yield a perf_counter delta of 0.0, crashing the run. Guard with baseline["median_s"] or 1e-9 (and note the fallback in the report).

🟡 report() idempotency assumed — measure(collector.report, ...) invokes the same bound method ~1005 times per case. If report() mutates state (clears internal buffers, appends to a shared list, or caches on the collector), every measurement after the first is wrong or raises. Either clone the collector per iteration (collector.report → a lambda rebuilding a fresh collector) or assert idempotency explicitly.

🟡 --worker-root silently defaults mode — in main, result = diagnostics_worker(args) if args.worker_mode == "diagnostics" else parse_worker(args) treats a missing --worker-mode identically to "current". Since both flags are argparse.SUPPRESSed internal API, a caller typo or an un-updated script will quietly run parse mode instead of failing. Require both together: if args.worker_root and not args.worker_mode: parser.error(...).

🟡 Silent type assumption on IR hashing — hashlib.sha256(parse_one(name, source).dump().encode("utf-8")) assumes dump() returns str. If a future change makes it return bytes, every worker crashes deep in the subprocess and the top-level report only surfaces a redacted one-line RuntimeError, hiding the real traceback. Decode defensively (if isinstance(ir, bytes): ir = ir.decode(...)) or log exc details into report["error"].

🟡 Broad except Exception erases tracebacks in CI — report["error"] = f"{type(exc).__name__}: {exc}" keeps the report schema stable (good), but a KeyError/AttributeError in a check now surfaces as an opaque string, forcing bisect-by-rebuild. Add a debug flag (--traceback) that includes traceback.format_exc(), or stash sys.exc_info() detail behind --verbose.

🟡 Hardcoded exit_code == 1 for all diagnostic cases — checks["cli_exit_code"] asserts code 1 uniformly. If the CLI ever distinguishes "too many errors" (the error_limit case has 20) or usage errors with a different code, this check fails for the wrong reason. Parameterize the expected exit code per case, the same way expected/hint already are.

💭 Markdown fence breakout — markdown_report interpolates case["rendered"] raw into ```text fences. Current output is trusted, but a rendered diagnostic containing ``` or the literal word text on its own line would break the report. Consider using a 4-backtick fence or escaping.

💭 Double trailing newline in --json mode — json_text already ends with "\n" and print(..., end="\n") adds another. Cosmetic, but it makes byte-exact golden-file comparisons flaky.

💭 Very long lines — the checks dict, the HTML/CSS literal, and the lines.append(...) calls run well past 120 cols. If the repo has a formatter/linter, run it before merge so this doesn't create review noise later.

💭 subprocess.run(..., text=True, encoding="utf-8") in run_worker will UnicodeDecodeError if the child emits non-UTF8 (e.g., a Windows console codepage leak). errors="replace" is a cheap safety net.

Positive notes worth keeping: the top-of-file stdlib-only import discipline (deferred until the worker selects a checkout) is exactly the right shape for A/B measurement; the is_relative_to(worker_root) assertion in parse_worker is a genuinely good guard against accidentally benchmarking the wrong checkout; and revision() correctly refusing to label an unpacked baseline with its enclosing repo's SHA avoids a classic A/B mislabeling bug.

---

📁 docs/superpowers/plans/2026-08-12-dsl-diagnostics-implementation.md

🔴 Breaking ctor: dataclass over DSLParseError — Task 1 Step 3: @dataclass on an Exception subclass generates an __init__ that shadows the parent's positional signature; str(error) falls back to args unless __str__ is defined.
Suggestion: Write an explicit __init__ preserving the existing positional order plus an explicit __str__; add a test for DSLSyntaxError("msg", 3, 5) behaving as before.

🔴 Committed doc embeds agent directives and ends in git push — header "REQUIRED SUB-SKILL..." and Task 5 Step 6.
Suggestion: This file lands in version control, so any harness that ingests repo docs receives an instruction to autonomously commit and push. Move the execution directive out of the repo and drop the push step from the plan.

🔴 Validation gate contradiction — Task 2 Step 5 gates DSLParser.parse() before IR, but Task 4 Step 3 routes the driver's DSL branch exclusively to ExtendedDSLParser.parse(...).
Suggestion: As written, the base parser's validation path is dead code in production. State the dispatch rule: does the driver fall back to DSLParser for non-extended files, or does the extended parser subsume it?

🟡 E112 untracked — Task 3 Step 1 tests E110/E111/E112, but Task 5 Step 4 omits E112 and no step defines what separates the three codes.
Suggestion: Pin a code→condition table in one place and include E112 in the final self-review.

🟡 Two render paths for one failure — Task 4 Step 1 requires errors to hold the complete plain rendering while Step 4 renders diagnostics instead of printing errors.
Suggestion: Either guarantee the two are identical by construction (.errors as a projection of .diagnostics) or document it. Otherwise renderers diverge, and non-CLI consumers keep reading a field the CLI ignores.

🟡 col semantics undefined — Task 2 Step 1 asserts exact 1-based columns on tab input; Task 1 Step 3 computes display columns by expanding tabs.
Suggestion: Specify whether col/end_col are source character offsets or display columns post-expansion. If fields and getters disagree, spans misalign on any tab-indented file.

🟡 Per-line stop vs block recovery unspecified — Task 2 Step 3 halts a line at its first structural error; Task 3 Step 3 pushes and pops a block stack.
Suggestion: Define the interaction. For if x followed by a malformed line, is the frame pushed first, and does it then emit a spurious E111 at EOF? Add that combined case to the suite.

🟡 Undefined "normal mode" — Task 4 Step 4: exit 2 "without traceback in normal mode".
Suggestion: Define the switch (--verbose, env var) or drop the qualifier. An undefined branch can't be tested.

🟡 Vocabulary drift — max_errors, limit_reached, diagnostic_limit, diagnostic_limit_reached across Tasks 1–4.
Suggestion: Standardize on one pair (max_errors + limit_reached) and reuse it in the result fields.

🟡 render_error(stream) -> str — Task 1 Step 3.
Suggestion: Writing to a stream and returning the text is a dual side effect; callers will either double-print or guess which is authoritative. Prefer returning str and let the CLI own the write.

💭 Full-suite fallback can't be in scope — Task 5 Step 2 falls back to pytest tests -q, which may surface failures Step 3's boundary forbids fixing.
Suggestion: Pre-declare acceptable unrelated failures, or pin the fallback to the DSL test set.

💭 memory/memory.md write inside a feature PR — Task 5 Step 5.
Suggestion: The plan already refuses to create unrelated infrastructure; the same logic applies to the memory edit.

💭 end_col=None span width unspecified — Task 1 Step 3.
Suggestion: State the fallback (full line vs. len(message)) so spans are deterministic.

---

📁 docs/topics/09-DSL诊断-CI与Benchmark.md

🔴 Maintenance Risk: Hardcoded SHA 997d2aa — Line 20

This SHA will go stale after any rebase, and the doc acknowledges this but still hardcodes it. Anyone following these instructions 3 months from now will get a dangling reference or, worse, a silently wrong baseline.

Suggestion: Replace with a descriptive placeholder (<PR-base-SHA>) and add a one-liner: git log --oneline main | head -20 to find the correct commit. Alternatively, store the baseline SHA in a small metadata file committed to the PR, not in prose.

---

🟡 Platform Mismatch: PowerShell-only code blocks — Lines 13, 20–22

The doc references ubuntu-latest and self-hosted runners, but all commands use powershell fences. A contributor on macOS/Linux or in GitHub Actions (where shell is bash) will need to guess whether python -m pip install -e . and the benchmark invocation work identically.

Suggestion: Use bash or shell fences, or add a note: "以下命令适用于 PowerShell；bash 用户去除 powershell 后等价。"

---

🟡 Missing Location: 12 built-in error test cases — Lines 39–41

The doc says error samples are "不放入正常 benchmarks/cases/" but never says where they actually live. A new contributor can't find them.

Suggestion: Add the path, e.g. "内置用例定义在 benchmarks/error_cases.json 中" or whichever module/file they reside in.

---

🟡 Unresolved Reference: "设计文档" — Line 62

"设计文档的解析倍率目标是 1.5x" — no link, no title, no path. This is the single most important performance target in the file.

Suggestion: Inline the target or add a relative link: [设计文档](../01-design.md#performance) or similar.

---

🟡 Worktree Cleanup Mechanism Not Specified — Line 67

"退出步骤时清理" — in CI, this could be a shell trap, an explicit teardown step, or an action (actions/checkout with cleanup). Without specifying, a future CI change might remove cleanup.

Suggestion: Name the mechanism, e.g. "通过 git worktree remove 显式清理，或使用 actions/checkout@v4 的 cleanup 参数。"

---

💭 --json vs --json-output distinction — Line 24

The parenthetical is accurate but easy to miss. Since this is a documented API surface, consider making it a callout or adding a short example showing both forms used together:

--json --json-output foo.json

---

💭 Scope limitation is clear but could be a heading — Line 73

"这里不计算常量合并次数…" is an important out-of-scope statement buried at the end. Promoting it to a ### 不包含 subsection would make it easier to discover during future scope discussions.

---

📁 docs/topics/09-DSL错误提示美化器-开发文档.md

这是一份开发文档 review，重点看内部一致性和实施者是否会踩坑。

---

🟡 矛盾：回退策略自相矛盾 — §13.1 同时提出两个互相冲突的策略。前半段说"收窄回退"、"不扫描关键字猜测"，后半段说"优先统一使用 ExtendedDSLParser，不保留两种模式"。如果后者成立，前者讨论的回退逻辑根本不需要存在。二选一并删掉另一边。

🟡 errors 字段语义断裂 — §13.2 说 errors "继续保存完整的无颜色文本"，§14.2 又说"不把抑制消息放进 errors"。如果 errors 是"完整"的，抑制信息缺失就是不完整。明确：errors 是否包含 limit footer，还是 errors 只存前 20 条源码错误文本。

🟡 §11.2 块恢复逻辑仅靠散文描述 — "while ... endif 弹 while 栈"、"if ... while ... endif 弹内层 while 和匹配的外层 if"——这是复杂的状态机转换，没有伪代码会直接导致实现者各行其是。建议给出 recover_mismatched_end(frame, eof) 的伪代码或状态转换图。

🟡 算子签名表不完整 — §10.4 的 OP_SIGNATURES 只有二元和一元混合，但 §16.4 测试矩阵明确包含"一元算子"。neg/abs/exp/log 等一元算子的签名缺失，实现者无法知道 positional=1 还是别的。请补全或指向设计文档的完整表。

🟡 return 关键字在基础 DSL 中是否合法 — §7.1 的示例 retrun result 建议改成 return，但 §2.1 描述基础 DSL 是"逐行正则解析与 IRBuilder 调用"。如果 return 只在扩展 DSL 中存在，对基础 DSL 输入给出 did you mean 'return'? 是错误建议。明确 return 归属哪个解析器，或按解析模式切换建议表。

🟡 SourceBuffer 空文件行为未定义行数 — §8.2 只说"不越界，不虚构源码"，但 "a\n" 是 1 行还是 2 行？影响后续所有行号计算和验证器的 enumerate(..., start=1) 边界。加一行明确：len("a\n".splitlines()) == 1 还是 == 2。

🟡 §13.2 diagnostics/errors 渲染优先级与 limit footer 的交互 — "CLI 只渲染 diagnostics，但根据 diagnostic_limit_reached 输出 footer"——footer 由 renderer 生成还是单独拼接到 stderr？如果 errors 的消费者（旧 CLI）不读 diagnostic_limit_reached，会漏掉 footer。明确 footer 的生成归属。

🟡 §9 异常继承方向反直觉 — 当前 DSLSyntaxError 继承 Exception，DSLParseError 也继承 Exception。目标代码写 isinstance(DSLParseError_instance, DSLSyntaxError) 为真，意味着 DSLParseError 继承 DSLSyntaxError。但两个名字都叫 "SyntaxError" 且 DSLParseError 更像子类——方向对了但文档没有用一张继承图说明，实施者容易搞反。

💭 错误码表散落全文 — E100–E112 分散在 §10–§11，E200–E203 在 §10.4，E100–E103 在 §18 测试清单。作为交付物之一，建议在本文档或设计文档中集中一张表，减少实施者跨节拼凑。

💭 §15.1 "显式上下文提示"优先级最高但无定义 — 前三级中的第一级"显式上下文提示"没说来自哪里（fix_hint 参数？正则表？错误码？）。后三级来源清晰，第一个模糊。

---

文档整体结构清晰、覆盖面广，是一份高质量的开发指南。上述问题主要是内部一致性和对实施者的精确性——修复后可以直接交给 W1–W2 的开发者。

---

📁 docs/topics/09-DSL错误提示美化器-设计文档.md

设计文档 Review

总体评价：结构完整、问题识别准确、恢复规则考虑周到。以下按优先级列出问题。

---

🔴 Breaking change 未纳入兼容性表 — §12.3 说库调用时 IndexError/AssertionError 应直接抛出，但 §14 兼容性表未记录此行为变更。当前调用方可能依赖 except Exception 兜底。必须补充为风险项并说明迁移路径。

---

🟡 render_error() 语义模糊 — §11.2 签名同时接受 stream: TextIO 又返回 str。未说明是"写入 stream 并返回副本"还是"仅返回字符串由调用方写入"。建议明确契约，或拆分为 render_error(err) -> str + write_error(err, stream) 两个函数。

🟡 Rule 4 恢复逻辑多块歧义 — §9.2 说"能匹配更外层块则弹出到该外层块"。当栈中有多个同类型块（如 if … if … endif）时，匹配哪个？应明确为"最近的同名祖先块"，并在恢复示例表中增加该用例。

🟡 去重键可能吞掉合法错误 — §9.3 的 (filename, line, col, error_code, message) 在极端情况下会丢弃同位置不同含义的诊断。建议增加约束：同位置同码去重，不同错误码即使位置相同也不合并（或至少说明取舍理由）。

🟡 算子集合一致性依赖测试而非强制 — §12.2 要求测试断言 Parser 算子名集合与 OP_SIGNATURES 完全一致。测试漏跑或跳过时该约束失效。建议改为从同一注册表导出，让不一致在运行时即报错。

🟡 e2e 测试缺少内部错误路径 — §13.3 只验证用户输入错误退出码为 1，未覆盖 §12.3 提到的 internal compiler error 退出码 2。应增加"触发未预期异常时 CLI 返回码为 2 且不泄露 traceback"的端到端断言。

---

💭 diagnostic_limit 默认值双写 — §7.4 CompileResult.diagnostic_limit: int = 20 与 §7.3 validate(max_errors=20) 是两处独立默认值。建议提取共享常量，否则后续修改容易不一致。

💭 "拟议"标记不一致 — §1 声明未实现的部分标"建议/拟新增"，但 §7.2 的 end_col 字段、§7.4 的 CompileResult 扩展、§11.2 的 render_error 签名等均以确定语气描述。建议在每个新增 API 首次出现时统一标注状态（如 [拟议]），便于读者区分现有契约与提案。

💭 文档长度 — 588 行，§17 风险表 5 行、§18 未来工作 7 条均为元信息。考虑将 §17–§18 移至附录或独立文档，让正文聚焦可实施的设计决策。

---

📁 memory/memory.md

🟡 Missing traceability — 每条记录只有日期，无法回溯到具体 PR/issue/commit。建议追加 [PR #123] 或短 commit hash，否则 6 个月后人无法确认该约束是否已随后续变更失效。

🟡 Scope ambiguity (entry 3) — "适用于本仓库课题 PR 的 CI 接入" 含义模糊。是所有 PR？仅 src/dsl/ 下变更？建议明确触发条件（如 "DSL 相关 PR" 或 changeset 标记），避免后续 PR 作者误判是否需要动 ci.yml。

🟡 Entry 4 缺少验证退出标准 — "性能目标达标状态与功能通过状态分开报告" 说了怎么分，没说 什么算达标。建议补一句量化阈值（如 "诊断生成 P95 < 200ms"），否则验收人无法判断 pass/fail。

💭 Structure — 四个条目横跨 benchmark 报告、格式化、CI、性能对比四个无关主题，建议按主题拆为 memory/benchmark.md、memory/error-formatting.md、memory/ci.md，单文件增长后定位成本会线性上升。

💭 Entry 2 nit — "不能固定为 6" 缺少上下文；首次阅读者不清楚 "6" 从何而来（旧实现对齐宽度？）。加一句 "（旧实现固定使用 6 列前缀）" 即可自洽。

---

📁 scratchv/compiler.py

Code Review: scratchv/compiler.py

🔴 Double file read / TOCTOU — When input_path is a .dsl file and dsl_source is None, the file is opened and read twice: once in compile() for validation (line ~250), once in _parse() (line ~373). If the file is modified between reads, validation and parsing operate on different content. Suggestion: read once in compile(), pass the string to _parse() or refactor _parse to accept pre-read source.

🔴 Removed fallback parser causes unhandled crashes — _parse() previously fell back to DSLParser when ExtendedDSLParser failed. That fallback is removed, and in compile()'s exception handler, non-DSLSyntaxError exceptions are re-raised (raise on the DSL path). Any parser error that isn't DSLSyntaxError (e.g., internal bugs, ValueError, IndexError) will now crash instead of returning a CompileResult. This is a breaking behavioral change.

🟡 Validation + parse does redundant work — ExtendedDSLParser is instantiated twice and the source is processed twice (validate then parse). Consider returning validation results from a single parse() call, or passing the validator/parser instance between compile() and _parse().

🟡 diagnostics: list[Any] is too loose — Any defeats static analysis. If there's a diagnostic type (e.g., DSLSyntaxError or a dedicated Diagnostic class), use it explicitly. At minimum, document what types are expected.

🟡 DSL-specific fields on the general result struct — diagnostic_limit_reached and diagnostic_limit are only populated on DSL validation failure but live on CompileResult unconditionally. Consumers need to know when they're meaningful. Consider nesting them in an optional sub-struct or documenting the invariant.

💭 source or "" in validate call — source is already a str at this point (either from dsl_source or f.read()), so or "" is a no-op safety net. Harmless but noise.

---

📁 scratchv/frontend/__init__.py

🟡 Exporting implementation details as public API — OP_SIGNATURES and SourceBuffer sound like internal utilities, not frontend public API. Once in __all__, downstream consumers may import and depend on them, making future refactoring (renaming, restructuring) a breaking change. Consider keeping them in their own modules and only re-exporting what's intentionally part of the public surface.

💭 Inconsistent import style — dsl_errors uses parenthesized multi-line, dsl_validator doesn't. Pick one convention:

from .dsl_validator import (
    DSLValidator,
    OP_SIGNATURES,
    SourceBuffer,
)

💭 __all__ ordering — Groups aren't consistent (classes, then exceptions, then functions, then back to classes). Consider grouping by type or module of origin for scanability.

---

📁 scratchv/frontend/dsl_errors.py

🔴 Bug: Silent error loss — add(): When limit_reached is true, excess errors are dropped silently and only surfaced via report(). Any caller iterating collector.errors directly won't know errors were suppressed. The old code appended a synthetic warning error so the count was visible. Consider either appending the note to _errors as before, or exposing limit_reached more prominently (e.g., in __repr__, or having errors include the suppression note).

🟡 Dedup key may be too aggressive — add() line ~356: Key is (filename, line, col, error_code, message). Two genuinely distinct errors at the same location (e.g., two different fix hints, or two tokens producing identical messages) will be silently dropped. If dedup is the intent, document the rationale; if not, consider narrowing the key to (filename, line, col, error_code) only, since error_code + location should uniquely identify an issue.

🟡 errors property sorts on every access — sorted(self._errors, ...) allocates a new list each call. Fine if called rarely (like in report()), but if any hot path calls collector.errors in a loop, this becomes O(n log n) per iteration. Consider caching the sorted result and invalidating on add()/clear().

🟡 end_col has no validation — DSLSyntaxError.end_col is Optional[int] with no check that end_col >= col. The max() in format_error prevents crashes, but a col=5, end_col=3 silently degrades to a single-char marker. A debug assertion or docstring note would help.

💭 render_error doesn't honor FORCE_COLOR — Standard env var (FORCE_COLOR=1) for forcing ANSI on non-TTY output (common in CI). Easy add: if "FORCE_COLOR" in os.environ: use_color = True.

💭 Empty DSLParseError — Works for compatibility, but a one-line docstring about which exceptions subclasses it catches (e.g., "catch this to handle all DSL parse errors including future variants") would save the next reader a guess.

---

---

⚠️ 未审查的文件

• scratchv/frontend/dsl_extended.py
• scratchv/frontend/dsl_parser.py
• scratchv/frontend/dsl_validator.py
• scratchv/main.py
• tests/test_dsl_diagnostics_benchmark.py
• tests/test_dsl_diagnostics_cli.py
• tests/test_dsl_errors.py
• tests/test_dsl_validator.py