Skip to content

chore: prettier baseline — config, CI format gate, tree reformat - #438

Merged
ranxianglei merged 3 commits into
masterfrom
2026-09-26_prettier-baseline
Sep 26, 2026
Merged

ranxianglei merged 3 commits into
masterfrom
2026-09-26_prettier-baseline

Conversation

@ranxianglei

Copy link
Copy Markdown
Owner

Closes #435

Root cause

No prettier config in the repo → prettier --check . ran pure defaults against a mixed-style tree (~half of src/*.ts already conformed; JSON/config files hand-indented 4-space; prose lines over 80 cols). 125 files failed on master itself (verified: npm ci → prettier 3.9.6 per lockfile → npm run format:check → 125). CI never gated formatting, so drift accumulated silently and the check required by AGENTS.md §7.2/§7.4 was unsatisfiable.

Commits

  1. chore: add prettier config and CI format gate
    • .prettierrc: explicit pin of prettier defaults (printWidth 80, tabWidth 2, semi, double quotes, trailingComma all, bracketSpacing, arrowParens always, endOfLine lf) — written out rather than {} so intent is documented and a future prettier major's default change cannot silently re-drift the tree.
    • .prettierignore: dist/, node_modules/, .tmp/, and package-lock.json (npm rewrites it in its own formatting on every dep change — formatting it would churn forever; it was ~4k of the original diff lines).
    • ci.yml: new standalone format job (checkout → setup-node 22 → npm ci → npm run format:check), runs on PRs to master + pushes to master.
  2. style: apply prettier defaults across tree — 124 files, layout only (+15,940 / −9,290, mostly line-wrap splits).

Why "pure defaults" (the owner call in #435)

Measured failing-file counts under candidate configs against the current tree:

config failing files
pure defaults 125
printWidth 100 163
printWidth 120 162
tabWidth 4 164

The de-facto style is closer to prettier defaults than to any consistent alternative (double quotes ✓, semis ✓, trailing commas ✓; deviations are only 80-col wrapping + 4-space JSON indent), so adopting defaults minimizes both the reformat diff and future churn.

Proof the reformat changed nothing

  • On the final tree: npm run typecheck ✓ · npm test 1122/1122 ✓ · npm run build ✓ · npm run format:check ✓
  • Idempotent fixed point verified: a second npx prettier --write . over the committed tree produces zero diff.
  • Mechanical char-level comparison of all 114 modified .ts files (whitespace-stripped old vs new): every character identical except
    • redundant parens around wrapped expressions: +40 added / −24 removed,
    • trailing commas at collection boundaries: +1,588 added / −19 removed (removed ones were hand-written single-line trailing commas),
    • ; member separators before } in type literals: ×40 (TS no-op),
    • union-pipe placement normalized: ×1 leading pipe added, ×1 removed,
    • 7 string literals flipped quote style ("→') because that side needed fewer escapes — all 7 verified equal after decoding.
  • package.json / tsconfig.json: value-equal via JSON.parse. pr-artifact.yml: pure 4→2-space reindent (block-scalar relative indentation preserved). Markdown docs: table padding, *x*→_x_, blank lines after headings, embedded-code indent — render-neutral. Prompt fixtures (tests/fixtures/*.txt) untouched — prettier has no parser for them.

Known upstream quirk surfaced (not fixed here): prettier 3.9.6 oscillates across runs on one wrapped signature whose return type is an object literal (parseCallText, src/hide-consumed.ts); two passes reach the fixed point this tree now sits at.

Behavior changes (flagged per policy)

  • New CI gate: formatting is now enforced for every PR/push to master by the format job — previously ungated. Future PRs with drifted formatting fail CI (that is the fix).
  • Tree reformatted across 124 files (layout-only, proven above). No runtime/wire-format/prompt changes; zero logic lines touched.

Deliberate deviations from the issue proposal

  • Gate lives in a standalone format job instead of inside pr-validation: pr-validation is script-only today (no node/npm); adding a full npm ci there for one command duplicates what the test matrix already does per node version.
  • No exact prettier pin (issue item 4, optional): package-lock.json already resolves 3.9.6 and CI uses npm ci (exact lockfile install), so resolution cannot drift without an intentional lockfile update. Happy to add prettier@3.9.6 as a one-line follow-up if you want belt-and-braces.

@github-actions

github-actions Bot commented Sep 25, 2026 •

Copy link
Copy Markdown
Contributor

📦 Built Package Artifact

Branch: 2026-09-26_prettier-baseline (53033aa)

Option A — Install from npm PR tag (recommended)

In your adapter project:

npm install acp-kernel@pr-438

Each push to this PR publishes a new version under the pr-438 npm tag.

Option B — Download artifact

  1. Download the artifact from the Actions run
  2. Extract the tarball and install:
tar xzf acp-kernel-pr438.tgz
npm install ./package

This comment is automatically updated on each push.

@ranxianglei

Copy link
Copy Markdown
Owner Author

🤖 Powered by ework · qwen3.8-27b

[bot] 🏷 Reviewed PR #438 (branch 2026-09-26_prettier-baseline, head 9654f8b). Verdict: mergeable — I independently re-verified the load-bearing claims; two related open PRs need an owner decision (below).

Base & CI: head sits directly on current master 476d440 — no rebase needed. All checks green at this exact head: test(22), test(24), format, pr-validation, build-artifact. mergeable_state: clean.

① Original issue (#435) — direction is right: the unsatisfiable format:check gate (AGENTS.md §7.2/§7.4) is fixed at the root (missing config + missing CI gate), not papered over. The pure-defaults config follows the measured minimum-diff data from the #435 thread. Scope covers proposed items 1–3; item 4 (exact pin) deliberately skipped with sound justification (lockfile + npm ci already resolve 3.9.6 deterministically).

② Regression review — existing behavior on every touched path (127 files):

  • 114 modified .ts files — my own mechanical verification, independent of the author's:
    • Whitespace-stripped char-level compare old vs new: every residual diff is one of trailing commas (+1540/−14), type-literal ; separators (+37), redundant parens (+31/−13), union-pipe normalization (×2) — all semantically neutral in TS.
    • Decoded every string literal (quotes + templates) old vs new per file: zero string-content changes → prompts, wire text, and test assertions are byte-identical. (One scanner false positive in src/parse-compress-input.ts caused by regex literals containing quotes; that file's non-whitespace diff eyeballed directly — pure line-wrap/layout.)
    • tests/fixtures/*.txt: untouched (confirmed via changed-file list).
  • package.json / tsconfig.json: value-equal via JSON.parse; version stays 0.0.94 (untouched, per git-safety rule).
  • ci.yml: purely additive — one new standalone format job; existing jobs byte-identical; inherits the unchanged workflow-level triggers (pull_request + push on master), so the gate fires exactly where claimed.
  • pr-artifact.yml: pure 4→2-space re-indent; block-scalar (run: |) relative indentation preserved.
  • Markdown docs ×7: table padding / emphasis markers / blank lines / fence indent — render-neutral (sampled).
  • §7.3 kernel contracts: none touched — no string/content changes anywhere, no pipeline/node-ordering change.

③ Local checks I ran on the PR head: prettier --check . green; a single prettier --write src/hide-consumed.ts is a no-op → the committed tree is a stable fixed point, so the new CI gate cannot be red on a clean tree.

⚠️ Overlap with two older open PRs (owner decision needed):

Recommendation: merge #438 now (the only clean/green/current-base option; unblocks the §7.2/§7.4 gates), then land #418's core fix (the ParsedCallText extraction, optionally its check-first format loop script) as a small follow-up rebased onto master, and close #361 as superseded. Happy to do the follow-up rebase/cherry-pick if you confirm.

Behavior change (flagged per policy): formatting becomes a hard CI gate for every PR/push to master (previously ungated) — the intended fix, disclosed in the PR body. No runtime/wire-format/prompt changes anywhere.

——
中文摘要:审核了 #438(prettier 基线:配置+CI 格式门禁+全树重排),独立做了逐字符比对与字符串解码比对验证零语义变更(prompt/wire/测试断言逐字节相同、version 未动、fixtures 未动),并本地确认提交树是稳定不动点;发现与两个更早的开放 PR 重叠(#361 同范围但基线旧且冲突,#418 修的是本 PR 遗留的 prettier 两遍非幂等根因),建议先合 #438,随后把 #418 的根因修复(parseCallText 返回类型提取为命名接口)作为小 follow-up rebase 到 master,并关闭被取代的 #361。

@ranxianglei

Copy link
Copy Markdown
Owner Author

🤖 Powered by ework · qwen3.8-27b

[bot] 🏷 Cross-link from #445 (now closed as duplicate of this PR): #445 was filed during #443 implementation after hitting the broken npm run format on master; its cause analysis independently supports the optional exact pin your PR body offers — "prettier": "3.9.6" over "^3.3.0". The caret range is what let 3.9.6 float in silently across months; lockfile + npm ci make resolution deterministic today, but the one-line pin removes the last drift vector at zero cost. Say the word and I'll land it on this branch as a clearly-labeled follow-up commit before merge.

Independent datapoint for the record (pristine master @ 476d440, prettier 3.9.6, all other defaults): printWidth 120 → 162 failing files / +10.6K−12.5K changed lines, vs your width-80 → 125 files / ~+15.9K−9.3K. Width 80 remains the min-diff choice by file count as measured in your PR body; noting only that 120 would preserve long single-line signatures if that ever matters to you. No action needed beyond the pin question — everything else here stands as reviewed.

@ranxianglei
ranxianglei force-pushed the 2026-09-26_prettier-baseline branch from 9654f8b to c74586f Compare September 26, 2026 10:29
@ranxianglei
ranxianglei force-pushed the 2026-09-26_prettier-baseline branch from c74586f to a68221f Compare September 26, 2026 10:50
ranxianglei added a commit that referenced this pull request Sep 26, 2026
The format gate runs inside the test matrix (both node versions) since
the prettier-baseline branch; a separate format job is a redundant
runner. Stacked on 2026-09-26_prettier-baseline (#438).
@ranxianglei

Copy link
Copy Markdown
Owner Author

Rebased onto master (post-#448): now a clean 3-commit stack 780dd2c config+gate → f880188 gitignore → a68221f tree reformat. Conflicts in tests/compress-tools.test.ts, tests/packs.test.ts, tests/surface-config.test.ts resolved by keeping #448 content and re-running prettier; 3 more #448-touched files (src/compress-tools.ts, tests/compress-schema-contract.test.ts, tests/wire-schema-top-level.test.ts) reformatted into the reformat commit. Verified: tsc 0, prettier --check green, full suite 1124/1124 (includes #448's new contract test). CI on the new head a68221f.

ranxianglei added a commit that referenced this pull request Sep 26, 2026
The format gate runs inside the test matrix (both node versions) since
the prettier-baseline branch; a separate format job is a redundant
runner. Stacked on 2026-09-26_prettier-baseline (#438).
@ranxianglei

Copy link
Copy Markdown
Owner Author

🤖 Powered by ework · qwen3.8-27b

[bot] 🏷 Re-reviewed the rebased head a68221f. Verdict: mergeable — everything re-verified independently against the new base.

Base & CI: merge-base == current master b3f7246 (post-#448) — sits directly on master, no stale base. mergeable_state: clean. All checks green at exactly this head: test(22), test(24), format, pr-validation, build-artifact. Commit stack matches your description: 780dd2c config+gate → f880188 gitignore(+.tmp/) → a68221f tree reformat.

Independent diff re-verification vs new master (my own scripts, not the author's):

  • 114 .ts files: whitespace-stripped char-level compare → every residual diff is neutral TS punctuation (trailing commas ×1609, type-literal ; ×46, redundant parens 24/24, union pipes ×2). Decoded string-literal compare (quotes + templates + escapes, per-file ordered lists) → zero string-content changes. Two scanner flags (src/parse-compress-input.ts, src/wire/openai.ts) were false positives from backticks inside JSDoc comments; eyeballed their --ignore-all-space diffs directly — pure indentation normalization (master's fix: use portable typed compress schema alternatives #448 content had over-indented lines prettier now fixes) and line-wrapping only.
  • Conflict resolution check — the 3 conflict files (tests/compress-tools.test.ts, tests/packs.test.ts, tests/surface-config.test.ts) plus the 3 extra fix: use portable typed compress schema alternatives #448-touched files (src/compress-tools.ts, tests/compress-schema-contract.test.ts, tests/wire-schema-top-level.test.ts): no semantic drift from master's (fix: use portable typed compress schema alternatives #448's) content — layout-only in all six. "Keep fix: use portable typed compress schema alternatives #448 content + re-run prettier" confirmed true.
  • One visible source change worth naming: ACP_CACHE_TOOL_DESCRIPTION in src/compress-tools.ts flips double→single quotes. That's prettier minimizing escapes (the string contains detail="full" and request's); the decoded value is byte-identical, so the actual tool description text is unchanged. Not a behavior change.
  • Non-TS: ci.yml still purely additive (standalone format job, existing jobs byte-identical); pr-artifact.yml pure re-indent; .gitignore gains one line (.tmp/); 7 markdown files render-neutral; package.json/tsconfig.json value-equal via JSON parse, version stays 0.0.94; no fixture touched.
  • Fixed point re-confirmed locally: prettier --check . green on the whole tree; single prettier --write src/hide-consumed.ts = no-op → the gate cannot be red on a clean tree.

Related-PR status (good news): #418 has already been retargeted onto this head (a68221f, mergeable_state: clean) and its newest commit b01e912 drops the duplicate dedicated format job in favor of format:check inside the test-matrix job — so the overlap I flagged last round is resolved within #418 itself. Merge sequence is now simply: #438 → #418, then close #361 as superseded (still open/dirty on old base fafc9c8). One nit for when you look at #418: with the in-test-job placement, format:check runs twice per PR (one per node matrix leg) instead of once in a dedicated job — trivial redundancy, either way is fine; your call.

Behavior change (flagged per policy): formatting becomes a hard CI gate (previously ungated) — the intended fix. No runtime/wire-format/prompt changes anywhere.

——
中文摘要:对 rebase 后的新头 a68221f 做了全量独立复核(逐字符比对+字符串解码比对,零语义变更;冲突解决确认保留了 #448 内容仅重排格式;唯一可见源码变化是 compress-tools.ts 一处引号风格翻转、解码后值逐字节相同;本地确认仍是稳定不动点、CI 五项全绿、基线即当前 master),结论可合并;并注意到 #418 已重新指向本 PR 头部且自行消除了 CI 门禁重复,建议合并顺序 #438 → #418,随后关闭被取代的 #361。

@ranxianglei
ranxianglei force-pushed the 2026-09-26_prettier-baseline branch from a68221f to 53033aa Compare September 26, 2026 11:10
ranxianglei added a commit that referenced this pull request Sep 26, 2026
The format gate runs inside the test matrix (both node versions) since
the prettier-baseline branch; a separate format job is a redundant
runner. Stacked on 2026-09-26_prettier-baseline (#438).
@ranxianglei
ranxianglei merged commit f252a31 into master Sep 26, 2026
5 checks passed
ranxianglei added a commit that referenced this pull request Sep 26, 2026
The format gate runs inside the test matrix (both node versions) since
the prettier-baseline branch; a separate format job is a redundant
runner. Stacked on 2026-09-26_prettier-baseline (#438).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

chore: format:check can never pass — no prettier config, 125 files drift under lockfile-resolved prettier 3.9.6

1 participant