Skip to content

build: serialize bundled guides one markdown line at a time - #33712

Merged
brennanb2025 merged 3 commits into
mainfrom
brennanb2025/guides-one-line-per-markdown
Oct 27, 2026
Merged

brennanb2025 merged 3 commits into
mainfrom
brennanb2025/guides-one-line-per-markdown

Conversation

@brennanb2025

@brennanb2025 brennanb2025 commented Oct 26, 2026 •

Copy link
Copy Markdown
Contributor
Files Added Deleted Net
Test 0 0 0 0
Prod 3 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​5342 $\color{#cf222e}{\Huge{\mathbf{−}}}$​31 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​5311

ELI5

Concurrent edits to different parts of the same skill guide can merge cleanly in Markdown but conflict in the committed generated file, because each whole guide occupies one source line. This change gives each Markdown line its own generated source line, reducing spurious conflicts for developers. App users see no change.

What Changed

The generator encodes guide, full-guide and reference Markdown as arrays of JSON-quoted lines joined with \n. Splitting retains the trailing empty piece, so trailing newlines and all other runtime bytes stay identical. The committed output is regenerated, and its path joins the existing generated artifacts in the root oxlint ignorePatterns list. No max-lines disable comment is added.

The generator's --check mode and its CI invocation stay in place. The tracked skills/<name>/SKILL.md copies are unchanged.

Why

A replay of the last 30 days of main found 15 conflicting pairs of guide commits, 13 caused only by the single-line generated strings. Two independent reviewers agreed that changing serialization is a better fit than moving generation to build time: that approach adds build prerequisites and maintenance that cost more than they save. The build-time proposal is being superseded by this narrower change, tracked in #30644.

Linked Issue

Tracked in #30644.

Visual Proof

N/A — generated source formatting only; runtime guide contents and UI are unchanged.

Testing

  • I manually tested these changes locally
  • Automated tests added/updated, or explained why not below

Loaded the original and regenerated TypeScript modules with Node and compared all exported guide records, then compared each Markdown value as UTF-8 buffers:

PASS: 10 guides; 35 markdown/fullMarkdown/reference strings; 373280 UTF-8 bytes identical.
Combined SHA-256: 2c8b4bab86b315f0dafc05053dbcd0a16f64267a36685ced8fb7a783d194c4c0
  • node config/scripts/generate-bundled-skill-guides.mjs --check passed after regeneration and formatting.
  • Through tc-gate: node node_modules/vitest/vitest.mjs run --config config/vitest.config.ts config/scripts/generate-bundled-skill-guides.test.mjs src/cli/specs/bundled-guide-flags.test.ts — 2 files, 15 tests passed.
  • A throwaway git merge-file replay changed the title and Screenshots heading independently in the same guide: the original generated file conflicted (exit 1); the regenerated file merged cleanly (exit 0).
  • Existing generator and guide-content tests cover the affected flow; no new test duplicates them. The parity script is throwaway evidence outside the PR.
  • The pre-commit hook passed. CI covers lint, typechecking and broader checks; no standalone build or typecheck was run.

Review

Builder self-review completed. Independent lane review follows separately.

Agent skill upstream boundary

  • Not applicable, or this change copies or mechanically translates no upstream skill-installer source, tests, fixtures, registry entries, path tables, comments, or documentation.

Notes

Serialization is platform-independent and preserves the existing LF bytes. There are no changes to SSH, mobile, saved data, build ordering, generated-file ownership or backwards compatibility. The generated module remains committed and requires no additional build step.

Checklist

  • This PR is small and focused
  • I explained what changed and why (ELI5, the user-facing before/after, the mechanism, and why over the alternatives)
  • Before/after screenshots or videos attached for UI changes, or N/A with reason
  • Self-reviewed for correctness, security, and performance
  • Cross-platform, SSH/remote, and path/shortcut impact considered (or N/A)
  • pnpm lint, pnpm typecheck, pnpm test, and pnpm build pass (or CI will cover; local preferred)

@brennanb2025
brennanb2025 marked this pull request as ready for review October 26, 2026 00:41
@coderabbitai

coderabbitai Bot commented Oct 26, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review.

Recent review info
Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Repository UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: eb760c66-f6ff-475a-945f-698be553f6b9
Commits

Reviewing files that changed from the base of the PR and between 06b5320 and abb7296.

Files selected for processing: 1
  • src/cli/bundled-skill-guides.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 4 remain after this review.

Walkthrough

The guide generator now serializes guide, full-guide, and reference Markdown as JSON-encoded lines joined with \n. The generated TypeScript constants use this representation, with guide content and the exported registry contract unchanged. The lint configuration now ignores the generated src/cli/bundled-skill-guides.ts file.

Priority: Low

Merge Risk: Minimal · up to abb72

No actionable issue is established for the guide serialization change; it is mergeable after normal checks.

Pre-merge checks: Passed 4 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 2 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 4 passed
Check name Status Explanation
Title check Passed The title clearly and concisely describes the main change: serializing bundled guide Markdown one line at a time.
Description check Passed The description is complete and follows the template. It explains the change, motivation, issue reference, lack of visual impact, testing evidence, review status, compatibility considerations, and che…
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Full details: Description check

Explanation

The description is complete and follows the template. It explains the change, motivation, issue reference, lack of visual impact, testing evidence, review status, compatibility considerations, and checklist results.

Finishing Touches: 💡 1
📝 Generate docstrings: 💡
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@brennanb2025

brennanb2025 commented Oct 26, 2026 •

Copy link
Copy Markdown
Contributor Author

Edits to separated Markdown payload lines could merge in the source guide but conflict in the committed embedded guide because each document occupied one generated source line. This PR gives each Markdown payload line its own source line, preserving the exported strings and existing generation/check workflow. App behavior is unchanged.

Full-path review completed with correctness/every-consumer review, an independent architecture challenge, a dedicated performance pass, and the readiness checklist. No code fixes were required. Ordinary same/adjacent-line conflicts and folded-description metadata conflicts remain expected residual cases. The deferred concern is unmeasured cold-load/heap/compiler overhead from finite joins behind the existing lazy import; no repeated scaling work or demonstrated regression was found.

Current-main integration: merge abb7296f744189e4e50424d8f9f6ebdb0bd2ee36 includes main 43097c517b4d8f7ff34f8d899b8f169cae82b15c. The only conflict was src/cli/bundled-skill-guides.ts, resolved solely by regenerating with the PR's unchanged generator from current main guide sources. The required merge hook passed before committing with --no-verify. Exactly the original three PR files differ from main; the generator and lint configuration are unchanged from the reviewed head.

Verified after this merge: all exported records and 35 Markdown values (10 guides, 377,388 UTF-8 bytes) are identical to current main; generator --check passes. Combined SHA-256: ad150a19529f34120fdb7c1c39f91778b49e4391bdf2b483a45bde02760d163f. The original review also verified 14 serialization edge cases, all 15 tests in the two named generator/guide integration files, and a two-edit merge replay that changes from conflict to clean merge with both edits retained. That evidence carries across main integration; the earlier integration merge was 06b53204e2ec3bb07b61fb03a72b90242b95ed69.

Full PR workflow passed on merge head abb7296f744189e4e50424d8f9f6ebdb0bd2ee36, including static/type checks, all five unit-test shards, relay integration, both packaging jobs, selection evidence, and the aggregate gate. Final check snapshot: 18 successes, 20 workflow skips. GitHub confirms MERGEABLE/CLEAN with no unresolved review threads.

QA classification: qa=tests. Main merges preserve the existing QA evidence on 2b7df76ad1eeb447b3f2439aa36e2b7401247f31. This changes generated source formatting and finite string initialization with byte-identical CLI responses. No app round, agents or screenshot is required for this flow.

For Brennan's review: no visual change. The judgments are the line encoding in serializeMarkdown and the existing generated-artifact lint treatment in oxlint.

@brennanb2025
brennanb2025 merged commit 8c07114 into main Oct 27, 2026
38 checks passed
@brennanb2025
brennanb2025 deleted the brennanb2025/guides-one-line-per-markdown branch October 27, 2026 01:40
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.

1 participant