coverage-gap stage 1: preflight + ABI-break-sweep skills, make_pr gate fixes#3098
Merged
Conversation
…e fixes skills/preflight.md maps every PR-triggered CI lane (build matrix incl. sanitizer/Debug/mingw/clang-cl, bundle_smoke, every extended_checks step, the six doc gates, wasm, eastl) to its exact local mirror command, or an honest "not mirrorable" with the WSL pointer. skills/abi_break_sweep.md: externals checklist — both-worlds spellings (the isArray() precedent) over feature macros, externals-merge-first ordering, stale-.shared_module 20605 trap, daspkg-index as scope. skills/make_pr.md: step-4 triggers gain positional doc validation (removed fields are CI-fatal too), the six-gate / first-panic note, and the untracked-RST check; 4f now runs BOTH sphinx builders; new step 2.7 routes type-system/generics changes to sequence smoke + externals sweep. Two stale claims fixed (probe-verified): das-fmt is in-tree at utils/das-fmt/dasfmt.das, and MCP format_file now agrees with it on named-arg spacing; kept the das-fmt.exe name trap (the cmake target is still the v1-to-v2 converter). CLAUDE.md skill table: register preflight + abi_break_sweep + the previously unlisted wsl_ci_repro. COVERAGE_GAP.md stage 2: cross-platform (Windows/mac/WSL) is a requirement for utils/preflight, with degrade-to- explicit-SKIP semantics for missing host tools. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Docs-only Stage 1 of the coverage-gap plan: adds a “preflight” process layer that maps PR-triggered CI lanes to local reproduction commands, plus an externals/ABI-break sweep checklist, and updates the PR-making checklist/gates accordingly.
Changes:
- Added
skills/preflight.mdmapping each PR-triggered workflow lane to a local mirror (or explicitly “not mirrorable”). - Added
skills/abi_break_sweep.mdchecklist for handling ABI/API breaks that affect external modules (e.g., dasImgui viaextended_checks). - Updated
skills/make_pr.md,COVERAGE_GAP.md, andCLAUDE.mdto reflect the new gates/skills and cross-platform requirements for the plannedutils/preflighttool.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| skills/preflight.md | New CI-lane ↔ local-mirror command reference for PR-triggered workflows |
| skills/make_pr.md | Updates PR checklist/gates (docs gates, formatter reality, type-system triggers) |
| skills/abi_break_sweep.md | New externals compatibility/ABI-break sweep checklist |
| COVERAGE_GAP.md | Stage 2 tool requirements clarified (cross-platform constraints, SKIP semantics) |
| CLAUDE.md | Registers the new skills in the skill index table |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+256
to
260
| rm -rf doc/sphinx-build site/doc build/latex | ||
| sphinx-build -W --keep-going -b latex -d doc/sphinx-build doc/source build/latex | ||
| sphinx-build -b html -d doc/sphinx-build doc/source site/doc 2>&1 | sed 's/\x1b\[[0-9;]*m//g' | tee /tmp/sphinx_out.txt | ||
| tail -3 /tmp/sphinx_out.txt | ||
| grep -iE "warning:|error:" /tmp/sphinx_out.txt |
Comment on lines
+196
to
198
| - **Struct fields or enum values added, REMOVED, or reordered** in any C++ type documented under `doc/source/stdlib/handmade/` — das2rst validates handmade docs **positionally** (line 1 = type description, line N+1 = Nth field/value), so a removed field is just as CI-fatal as an added one | ||
| - RST files in `doc/source/` (handwritten tutorials, reference pages, TOCs) | ||
| - `doc/reflections/das2rst.das` or `doc/reflections/rst.das` |
|
|
||
| | Workflow | Trigger | Jobs | | ||
| |---|---|---| | ||
| | `build.yml` | every PR | `build` matrix (5 targets × Debug/Release/RelWithDebInfo × sanitizers), `bundle_smoke`, `build_windows_mingw`, `build_windows_clangcl` | |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stage 1 of
COVERAGE_GAP.md(follow-up to #3095, plan landed in #3097). Docs-only — the process layer that the Stage 2utils/preflighttool will later automate.New:
skills/preflight.mdCI-lane ↔ local-mirror table for every PR-triggered lane, built from a fresh read of the five workflows:
build.ymlmatrix: interp/JIT/ctest/AOT mirrors, plus the lanes that need honesty — sanitizers (WSL only), Debug (fused-path divergence note), ARM/mac, mingw (80/20 = clang syntax pass), clang-cl (fully mirrorable on Windows, commands included)bundle_smoke: WSL-verbatim, when to run itextended_checks: per-step commands, led by the defining divergence — CI configures with ALL release modules ON while local dev builds don't, which is exactly how thesafe_addr(bool[4])regression hid in dasOpenGLdoc.yml: the six gates as a table, with the stop-at-first-panic warningwasm_build/build_eastl: what puts them at risk, when to let CI carry themNew:
skills/abi_break_sweep.mdThe externals checklist distilled from the #3095 experience: why
extended_checksbuilds external dasImgui's master against your branch, both-worlds spellings (theisArray()precedent) preferred over shims preferred over feature macros, externals-merge-FIRST ordering, the stale-.shared_module→ misleadingerror[20605]trap, and daspkg-index as the sweep scope.skills/make_pr.mdgate fixes-Wtoo, and it warns on things html accepts)das-fmt/dasfmt.dasis in-tree now (utils/das-fmt/), and MCPformat_fileagrees with it on named-arg spacing (both wrapdaslib/das_source_formatter; probedFoo(a=1)→ both rewrite toa = 1). Kept the name trap: a locally builtbin/Release/das-fmt.exeis still the v1→v2 converter fromutils/dasFormatter/.Housekeeping
wsl_ci_repro.md(existed but was never indexed)utils/preflight— pure.das+ clargs,clang-cl /Zsvsclang -fsyntax-onlybehind one flag, multi-config vs single-config binary discovery, missing host tools degrade to explicit SKIP🤖 Generated with Claude Code