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

New issue

Closed

opened 2026-09-22 01:30:53 +01:00 by allod-agent · 0 comments

allod-agent commented

2026-09-22 01:30:53 +01:00

Member

Split pr-explain/lib.sh, a 2566-line file of 58 functions with no section boundaries, into one file per concern, so a change to a report validator stops rendering as a change to asset emission and a reader can find a function by what it is for.

Primary goals:

One file per concern — asset emission, fragment validation, the report validators and the report driver each become their own file under pr-explain/lib/, sourced in a fixed order by lib.sh or by explain.
Names stay — every pr_explain_* function keeps its name, so explain and the tests are unchanged apart from where they source from.
The tests are the witness — tests/pr-explain/ runs green before and after, with no test edited.
explain is looked at, not split — it is 1306 lines with a 450-line pr_explain function; whether its passes want their own files is decided after the library split makes the seam visible.

Current state and specifics

The function list groups itself. Asset resolution and emission are pr-explain/lib.sh:7-46 (pr_explain_asset_dir, pr_explain_resolve_forge, the pr_explain_emit_* family). Fragment validation is :58-231 (pr_explain_validate_outline, pr_explain_check_front_fragment, pr_explain_check_section_fragment, pr_explain_check_quiz_fragment and their helpers). The report validators are :320-1775: about thirty pr_explain_*_are_well_formed, pr_explain_*_are_canonical and pr_explain_check_* functions, each a self-contained predicate over the assembled report. The report driver is :1775-1857 (pr_explain_extract_asset, pr_explain_strip_quoted_code, pr_explain_report_tags, pr_explain_validate_report). Shared primitives such as pr_explain_html_escape, pr_explain_validation_error and pr_explain_count_regex (:239-260) are used across groups and belong in a small common file sourced first.

explain sources lib.sh once (pr-explain/explain, one lib.sh reference), so the cheapest shape keeps lib.sh as a thin file that sources the parts in order, and nothing outside pr-explain/ changes. The tests under tests/pr-explain/ (command.sh alone is 1393 lines) exercise the public functions and are the gate; shellcheck over every new file is the second.

Scope

In scope: the split, the sourcing order, and shellcheck over the result.

Explicitly not in scope: changing what any validator accepts or emits, and splitting explain itself, which is a follow-up decided on the seam this exposes.

Split `pr-explain/lib.sh`, a 2566-line file of 58 functions with no section boundaries, into one file per concern, so a change to a report validator stops rendering as a change to asset emission and a reader can find a function by what it is for. Primary goals: - **One file per concern** — asset emission, fragment validation, the report validators and the report driver each become their own file under `pr-explain/lib/`, sourced in a fixed order by `lib.sh` or by `explain`. - **Names stay** — every `pr_explain_*` function keeps its name, so `explain` and the tests are unchanged apart from where they source from. - **The tests are the witness** — `tests/pr-explain/` runs green before and after, with no test edited. - **`explain` is looked at, not split** — it is 1306 lines with a 450-line `pr_explain` function; whether its passes want their own files is decided after the library split makes the seam visible. ### Current state and specifics The function list groups itself. Asset resolution and emission are `pr-explain/lib.sh:7-46` (`pr_explain_asset_dir`, `pr_explain_resolve_forge`, the `pr_explain_emit_*` family). Fragment validation is `:58-231` (`pr_explain_validate_outline`, `pr_explain_check_front_fragment`, `pr_explain_check_section_fragment`, `pr_explain_check_quiz_fragment` and their helpers). The report validators are `:320-1775`: about thirty `pr_explain_*_are_well_formed`, `pr_explain_*_are_canonical` and `pr_explain_check_*` functions, each a self-contained predicate over the assembled report. The report driver is `:1775-1857` (`pr_explain_extract_asset`, `pr_explain_strip_quoted_code`, `pr_explain_report_tags`, `pr_explain_validate_report`). Shared primitives such as `pr_explain_html_escape`, `pr_explain_validation_error` and `pr_explain_count_regex` (`:239-260`) are used across groups and belong in a small common file sourced first. `explain` sources `lib.sh` once (`pr-explain/explain`, one `lib.sh` reference), so the cheapest shape keeps `lib.sh` as a thin file that sources the parts in order, and nothing outside `pr-explain/` changes. The tests under `tests/pr-explain/` (`command.sh` alone is 1393 lines) exercise the public functions and are the gate; shellcheck over every new file is the second. ### Scope In scope: the split, the sourcing order, and shellcheck over the result. Explicitly not in scope: changing what any validator accepts or emits, and splitting `explain` itself, which is a follow-up decided on the seam this exposes.

allod-agent referenced this issue from a commit

2026-09-22 02:18:58 +01:00

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

allod-agent referenced this issue from a commit

2026-09-22 02:18:58 +01:00

pr-explain: correct the split lib header dependency notes

allod-agent referenced this issue from a pull request that will close it,

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

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

allod-agent referenced this issue from a commit

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

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