perf: reduce multi-diagram generation latency without weakening quality gates - #208
perf: reduce multi-diagram generation latency without weakening quality gates#208sunsunsun-java wants to merge 12 commits into
Conversation
Add authored-language enforcement, bounded structural reflow, workflow wide-column fitting, and marker-safe endpoint routing. Regenerate affected examples, gallery assets, and the packaged skill.
tt-a1i
left a comment
tt-a1i
left a comment
There was a problem hiding this comment.
Reviewed at exact head 40fca32. The overall direction is valuable, and CI, package freshness, and the full local test suite are green, but the three correctness/performance issues below break the new fail-closed contracts and should be fixed before merge.
The performance acceptance evidence is also not complete yet: issue #175 starts from 2637.592s aggregate work, while the supplied 6e2f1d1-era run reports 2689.103s and includes uncommitted fixes. Those numbers are not a matched A/B comparison and do not bind the current head. Please attach current-head matched A/B receipts for the same pinned Pi revision, runtime, prompts, semantic coverage, and visual gates before claiming the 20-30% reduction.
| id: 'repository-revision', | ||
| kind: 'repository-revision', | ||
| executable: 'git', | ||
| args: ['-C', repoRoot, 'rev-parse', 'HEAD'], |
There was a problem hiding this comment.
[P1] Bind suite execution to a clean pinned checkout
This gate checks only git rev-parse HEAD, while manifest exec commands may run with cwd: repository. A checkout with modified tracked files or untracked files therefore passes, and a generator can consume those bytes while the final receipt still attributes the run to the clean commit SHA. That breaks the documented revision-pinned and fail-closed contract. Please reject dirty worktrees before creating output and re-check after generator commands or before the final receipt, with regression coverage for both tracked and untracked drift.
| }); | ||
| } | ||
| } | ||
| const matches = found.slice(0, query.maxResults); |
There was a problem hiding this comment.
[P1] Make maxResults bound the source-search work
maxResults is applied only after all selected blobs have been loaded and every matching line has been materialized. On this exact head, source-search --term function --max-results 1 against this repository scanned 517 files, accumulated 38,522 matches, returned one result, and took about 19.8 seconds. This recreates the repeated repository-scan cost that #175 is intended to remove. Please bound the files and total bytes and stream or stop once the requested result budget is reached, or require an explicit narrow path filter, instead of building the complete found array.
| const titleMatches = requiredLanguage === 'zh-CN' ? hasCjk : hasLatin && !hasCjk; | ||
| const technicalIdentifiers = entries.filter((entry) => isTechnicalAuthoredText(entry.text)); | ||
| const proseEntries = entries.filter((entry) => !isTechnicalAuthoredText(entry.text)); | ||
| const proseViolations = requiredLanguage === 'zh-CN' |
There was a problem hiding this comment.
[P1] Enforce both language directions without rejecting product names
For en, this branch always makes proseViolations empty. A candidate with meta.locale: en, an English title, and Chinese reader-facing body labels returns success with violations: 0. Conversely, isTechnicalAuthoredText rejects any string containing whitespace, so a Chinese candidate rejects an exact product name such as GitHub Actions, despite the contract requiring product and code identifiers to be preserved. Please add symmetric reader-facing validation plus bounded identifier/product-name handling, with regression tests for both cases.
|
@tt-a1i Thank you for the detailed review. I have pushed the new head 1195611. It addresses the three P1 findings: clean revision-pinned checkout enforcement, source search that stops materializing blobs once maxResults is proven truncated, and symmetric authored-language validation while preserving bounded product and technical names. It also adds the workflow main-path lane reflow guard and regression coverage for repeated lane re-entry. Local verification on this exact head passed: 1,258 tests with 0 failures and 27 expected skips; deterministic archify.zip byte comparison passed; packaged-skill smoke passed. GitHub CI for this exact head has started and is currently queued. I am not treating the existing local A/B artifacts as exact-head acceptance evidence, and I will not claim the 20–30% target until a matched current-head receipt is attached. Could you please re-review the correctness fixes after CI completes? |
PR #208 完整基准：多模型、多强度、多代码库五图生成
Important 发布这份报告时，PR 已前进到新 head 先给结论
flowchart LR
A["A：PR 前串行流水线<br/>36.457 秒"]
B["B：PR #208 批处理流水线<br/>25.859 秒"]
A -->|"配对中位 -29.5%"| B
QA["A 盲审<br/>语义 98.8 / 视觉 78.8"]
QB["B 盲审<br/>语义 100.0 / 视觉 92.8"]
QA -->|"没有质量下降<br/>视觉 +14.0"| QB
xychart-beta
title "固定候选流水线墙钟（越低越好）"
x-axis ["A 串行", "B PR #208"]
y-axis "秒" 0 --> 40
bar [36.457, 25.859]
1. 测试设计与版本固定
为避免把“模型差异”误算成“PR 性能”，测试分成两层：
Pi 合同固定：
测试环境验证：
2. 严格机器 ABBA：PR 自身是否加速测试采用 warmup=1、rounds=3、ABBA 顺序，共获得每臂 6 个正式样本。
因此可以确认：共享项目索引、共享预检、批量 suite 和共享视觉批次确实减少了机械重复开销，同时没有删除事实、节点、关系、view 或视觉步骤。 需要特别区分：Issue #175 中的 2,637.592s 是 aggregate agent work，不是并发墙钟。它与本次 B 的 38:35.149 不是同口径，不能用二者直接宣称 12.2% 端到端加速。 3. Pi 五种图：完整交付与每图耗时Pi + Sol-high 在 PR B 上完成并交付 5/5：
基线 A 的严格串行 20 阶段耗时：
独立 A/B 十图盲审：
B 的 workflow、dataflow、lifecycle 对画布利用明显更充分；A 的 workflow/dataflow 虽通过自动门禁，但在 1440×900 下主图偏小。这也是为什么自动几何检查必须配合实际渲染与独立人审。 4. Codex 多 Session、模型与推理强度本次使用并行 session/agents 分离源码合同、基线 A、OpenRouter 盲审和 Sol A/B 盲审，并对 Luna、Terra、Sol 的不同强度进行测试。
结论：模型创作/修复时间远大于候选冻结后的流水线时间。 PR 优化解决了机械重复工作，但无法自动消除弱候选、修复循环和模型工具调用效率差异。 5. OpenRouter 常见模型同题测试所有运行均使用同一个固定 Pi revision、同一架构图合同、同一 PR B CLI，并要求最多 12 轮修复。只向模型提供公开 Pi 源码证据；没有发送私有代码库或凭据。 pie showData
title OpenRouter 八组运行交付结果
"完成自动门禁、交付与视觉检查" : 5
"限定窗口内未完整交付" : 3
八组实测合计费用 $5.858879，5/8 完成交付。 观察：
6. 跨代码库五图测试
FastAPI 与 uv 并不是“没跑”：两者都固定源码证据、生成五份候选、绑定正确 repo-root/revision，并真实执行 flowchart TD
S["固定源码证据与 revision"] --> C["并发生成五种候选"]
C --> P{"共享预检"}
P -->|"Pi 5/5"| D["Deliver + shared visual batch"]
P -->|"FastAPI 0/5"| F1["Fail closed<br/>不生成伪成功"]
P -->|"uv 1/5"| F2["Fail closed<br/>保留失败收据"]
7. 如何理解“更快”和“质量是否降低”可以确认
不能确认
当前主要瓶颈
换句话说：PR #208 已经实质缩短“流水线机械部分”，但真实五图任务绝大多数时间仍花在 authoring 和 repair。 下一步性能工作应该把 Agent authoring receipt 标准化，并在相同模型、相同负载、相同提示、相同机器条件下做端到端 ABBA。 复现实验建议与验收口径
局限
最终判断在固定提交 但目前最准确的表述应是：
这比“整体已经稳定快 20–30%”更符合当前证据。 |
There was a problem hiding this comment.
Reviewed current head c8fc9f5. The earlier dirty-worktree/read-bound regressions have focused fixes and passing tests. One language-gate compatibility issue still reproduces.
[P2] Allow valid single-character Chinese branch labels and preserved product names (archify/authoring/authored-language.mjs:95-107). Starting with a valid workflow fixture, validate --required-language zh-CN succeeds. Changing only two edge labels to 是 and 否 makes it fail in the language phase; changing only a node label to Google Cloud likewise fails. The former are ordinary Chinese branch labels, and the latter is an original proper product name that SKILL.md:93-94 and authoring-contract.md explicitly allow preserving. The chineseCount < 2 rule and narrow name heuristic reject legitimate authoring rather than language leakage. Cover both cases through the public CLI and preserve real mixed-language detection.
Verification:627 tests passed,0failed,5canonical ZIP checks skipped under Node24; golden passed and all28 checked generated-receipt relationships match,91ZIPpayload files match. Additional CLI control/negative fixture confirms this finding despite that green suite. No claim of matched end-to-end performance improvement or manual visual acceptance from these checks. No edits or merge.
3 participants
Problem and value
Repository-backed multi-diagram generation repeatedly loaded authoring contracts, schemas, examples, and repository evidence, then started separate validation/browser processes for each diagram. That made five-diagram runs unnecessarily slow and also encouraged repair loops that could drift in language, semantics, routing, or visual quality.
This change reduces avoidable orchestration and validation overhead while keeping the quality gates fail-closed. It also records stage-level timings so future regressions can be located instead of inferred from total wall time.
Closes #175.
Scope
Stability impact
Tests run
npm testfromarchify/: 1,218 tests; 1,191 passed; 0 failed; 27 skipped.Visual evidence
viewer/viewport-overflow; they are explicitly not claimed as passing, and this PR does not suppress or hide that diagnostic.Generated artifacts
archify.ziparchify/examples/andexamples/docs/gallery/docs/assets/archify-live-proof.gifand its JSON receiptChecklist