Split pr-explain/lib.sh by concern #223

Merged

vnprc merged 3 commits from agent/pr-explain-lib-split into master

2026-09-22 03:37:33 +01:00

allod-agent commented

2026-09-22 02:20:46 +01:00

Member

pr-explain/lib.sh is split into five files under pr-explain/lib/, one per concern, and nothing about the tool changes: every one of its 58 functions keeps its name and its exact body, lib.sh stays as the thin file that sources the parts in order, and explain and the tests are untouched. What proves it is a per-function body comparison before and after, shellcheck, and the three pr-explain suites green. If it is wrong, revert the one commit.

What changed

pr-explain/lib/common.sh: the primitives every other part uses (HTML escaping, the validation error and warning arrays and their recorders, the regex counters, UTF-8 safety, runner display). Sourced first.
pr-explain/lib/assets.sh: asset directory and forge resolution, and the emit_* family.
pr-explain/lib/fragments.sh: outline validation and the front, section and quiz fragment checks.
pr-explain/lib/report-validators.sh: the 26 well-formedness and canonical-form predicates over the assembled report, plus reading cost, layers, objectives, quiz and slop checks.
pr-explain/lib/report.sh: the report driver (asset extraction, quoted-code stripping, tag reporting, validate_report).
pr-explain/lib.sh keeps PR_EXPLAIN_LIB_DIR and sources the five in that order. Each new file opens with a paragraph on what it holds and depends on.

Found on the way and filed separately, not fixed here: running the pr-explain suites leaves two self-referential symlinks in the source tree (allod/tools#222).

Risk

R2. Shell library split with no function edited; the failure modes are sourcing order (a top-level statement reading something defined in a later file) and a path computed from BASH_SOURCE that now names the part file. Both are exercised by the suites, which drive the library through allod pr explain. Rollback is a straight revert.

Validation

Function set: 58 names before and after, identical. Bodies: the 48 multi-line functions extracted by brace-matching and diffed, identical; the 10 one-line functions diffed as lines, identical. Run by the integrator, independently of the worker's own extraction.
bash -n and shellcheck -x on lib.sh and all five parts: clean.
bash tests/pr-explain/components.sh, validation.sh, command.sh from the worktree root, re-run by the integrator: 86, 104 and 704 tests passed. The allod binary resolves the library from ALLOD_TOOLS_DIR at run time, so the suites exercised this worktree's files without a Go rebuild.
Not run: the allod-parity flake check that wraps the same three suites, because it rebuilds the Go binary; the suites were run directly instead.
One cold cross-family review of the diff (gpt-5.6-terra on codex, high effort): three findings, all header paragraphs that omitted a dependency (die and make_temp_file are caller-provided by explain; report-validators.sh calls into report.sh at run time; report.sh calls into assets.sh), fixed in the second commit; the worker then found and fixed one more header of the same kind in fragments.sh in the third. No function moved or changed in either; re-verified by the same body extraction.

Closes allod/tools#221

`pr-explain/lib.sh` is split into five files under `pr-explain/lib/`, one per concern, and nothing about the tool changes: every one of its 58 functions keeps its name and its exact body, `lib.sh` stays as the thin file that sources the parts in order, and `explain` and the tests are untouched. What proves it is a per-function body comparison before and after, shellcheck, and the three pr-explain suites green. If it is wrong, revert the one commit. ## What changed - `pr-explain/lib/common.sh`: the primitives every other part uses (HTML escaping, the validation error and warning arrays and their recorders, the regex counters, UTF-8 safety, runner display). Sourced first. - `pr-explain/lib/assets.sh`: asset directory and forge resolution, and the `emit_*` family. - `pr-explain/lib/fragments.sh`: outline validation and the front, section and quiz fragment checks. - `pr-explain/lib/report-validators.sh`: the 26 well-formedness and canonical-form predicates over the assembled report, plus reading cost, layers, objectives, quiz and slop checks. - `pr-explain/lib/report.sh`: the report driver (asset extraction, quoted-code stripping, tag reporting, `validate_report`). - `pr-explain/lib.sh` keeps `PR_EXPLAIN_LIB_DIR` and sources the five in that order. Each new file opens with a paragraph on what it holds and depends on. Found on the way and filed separately, not fixed here: running the pr-explain suites leaves two self-referential symlinks in the source tree (allod/tools#222). ## Risk R2. Shell library split with no function edited; the failure modes are sourcing order (a top-level statement reading something defined in a later file) and a path computed from `BASH_SOURCE` that now names the part file. Both are exercised by the suites, which drive the library through `allod pr explain`. Rollback is a straight revert. ## Validation - Function set: 58 names before and after, identical. Bodies: the 48 multi-line functions extracted by brace-matching and diffed, identical; the 10 one-line functions diffed as lines, identical. Run by the integrator, independently of the worker's own extraction. - `bash -n` and `shellcheck -x` on `lib.sh` and all five parts: clean. - `bash tests/pr-explain/components.sh`, `validation.sh`, `command.sh` from the worktree root, re-run by the integrator: 86, 104 and 704 tests passed. The `allod` binary resolves the library from `ALLOD_TOOLS_DIR` at run time, so the suites exercised this worktree's files without a Go rebuild. - Not run: the `allod-parity` flake check that wraps the same three suites, because it rebuilds the Go binary; the suites were run directly instead. - One cold cross-family review of the diff (`gpt-5.6-terra` on codex, high effort): three findings, all header paragraphs that omitted a dependency (`die` and `make_temp_file` are caller-provided by `explain`; `report-validators.sh` calls into `report.sh` at run time; `report.sh` calls into `assets.sh`), fixed in the second commit; the worker then found and fixed one more header of the same kind in `fragments.sh` in the third. No function moved or changed in either; re-verified by the same body extraction. Closes allod/tools#221

allod-agent added 3 commits

2026-09-22 02:20:46 +01:00

pr-explain: split lib.sh into pr-explain/lib/*.sh by concern 6a08fa1dfe

lib.sh was 2566 lines and 58 functions with no section boundaries.
Split it into common.sh (shared primitives), assets.sh (asset
resolution/emission), fragments.sh (per-pass fragment tripwires),
report-validators.sh (the ~30 assembled-report predicates), and
report.sh (the report driver), each with a header naming what it
holds and depends on. lib.sh is now a thin loader that sources the
five files in dependency order; every pr_explain_* function keeps
its name and body byte-for-byte, verified by an awk-based
before/after extraction and diff. explain and validate-report still
source only lib.sh, so no caller changes.

Refs allod/tools#221

pr-explain: correct the split lib header dependency notes 99451dcbb5

Cross-family review of the lib.sh split found three header
inaccuracies, all header text only, no function moved or edited:

- assets.sh claimed a lib/common.sh dependency it does not have
  (no function in the file calls into common.sh) and omitted that
  pr_explain_resolve_forge and pr_explain_emit_asset call die,
  which is caller-provided by explain, not defined in any lib file.
- report-validators.sh named lib/common.sh but not that
  pr_explain_visible_prose calls pr_explain_strip_quoted_code from
  lib/report.sh, sourced after it; the call still resolves because
  it happens at pr_explain_validate_report run time, not at source
  time.
- report.sh omitted lib/assets.sh, which pr_explain_validate_report
  calls into for pr_explain_emit_css/_js/_gallery, and the
  caller-provided make_temp_file; it also claimed a lib/fragments.sh
  dependency the file does not have.

Refs allod/tools#221

pr-explain: correct fragments.sh split lib header too 001950b2c9

Same rule as the prior header-fix round: fragments.sh claimed a
lib/common.sh dependency no function in it has. Header text only,
no function moved or edited. It calls no other file under
pr-explain/lib; only check_front_fragment, check_section_fragment,
and check_quiz_fragment call the caller-provided die.

Refs allod/tools#221