Generate comprehension-first PR explanation reports on command #138
Labels
No labels
bug
duplicate
enhancement
help wanted
invalid
question
wontfix
bug
duplicate
enhancement
help wanted
invalid
question
wontfix
No milestone
No project
No assignees
2 participants
Notifications
Due date
No due date set.
Dependencies
No dependencies set
Reference
allod/tools#138
Loading…
Reference in a new issue
No description provided.
Delete branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Turn pull-request diffs into durable, self-contained teaching reports on demand, using one tested visual language so the operator can understand complex changes faster and iteratively improve both the report and its generator.
Primary goals:
allod pr explain <number> --codex|--claude --output <file>resolves an immutable PR snapshot, invokes the selected installed subscription CLI, validates the result, and writes one self-contained HTML report.--replaceregenerates a report after human feedback; the tool never edits the PR, commits, or pushes. The human critiques the artifact and an agent revises the generator/template on its own PR.Current state and specifics
agent-memory/skills/explain-diff/SKILL.mdreliably prompts agents to investigate a diff and produce a report, but every report hand-builds its HTML/CSS/JS. The resulting drift is visible in the latest Forgejo migration explanation: on narrow screens its flow nodes align left while sibling arrow boxes stretch across the row and center their rotated glyphs. The root cause is duplicated layout code with separate alignment authority for nodes and connectors.A comparative audit found independently reinvented page shells, flow diagrams, quiz engines, callouts, and code blocks across existing report artifacts. The strongest pieces should become a component vocabulary rather than another fixed narrative: connectors belong to ordered-list nodes and change glyph (
→to↓) instead of rotating a full-width box; sequence animation is discrete, user-controlled, reduced-motion safe, and readable without JavaScript; exact code useswhite-space: pre; every figure has a claim-bearing caption; every report starts with an operator summary and ends with provenance and validation limits.The public tool owns only the generic mechanism. It accepts an explicit output path and contains no private notes path, hostname, provider credential, or deployment convention. Private skills may wrap it to choose a report path and commit/push the artifact.
Proposed interface
The provider flag is the disclosure/consent boundary: the command shows repository, PR, immutable base/head SHAs, and selected runner before sending source to that subscription provider. It unsets API-key/provider-override environment variables and uses the user's installed Codex or Claude subscription CLI; it does not consume a Pi/nullsink provider.
Implementation shape
forge pr snapshot <number>JSON interface rather than parsingforge pr viewprose.Scope
In scope: the public
allod/toolscommand, Forge snapshot interface, template/component gallery, prompt, validator, shell-fixture tests with mocked runners, and docs.Out of scope: committing/pushing reports, private notes paths, modifying PRs, autonomous acceptance of generated explanations, API-key providers, unattended server jobs, and retrofitting every existing report. The private
explain-diffskill can adopt the command in a separate private change after this public interface lands.Validation
Tests cover snapshot JSON, same-repo/fork PRs, immutable-SHA mismatch, explicit provider invocation with API/provider variables absent, dry-run, overwrite refusal, runner/no-output failure, atomic replacement, and validator sabotage fixtures. A component-gallery check covers narrow-flow alignment, no rotated connectors, reduced-motion fallback, internal anchors, code whitespace, accessible quiz semantics, and external-resource refusal. One attended smoke run per subscription CLI generates a report from a real PR for human inspection.
vnprc-agent referenced this issue2026-08-15 06:06:52 +01:00
Second reader-measurement datapoint, worse than the first. The owner ran the v2 pipeline on allod/archetypes#39 (triage: T3) and abandoned the report halfway: "too dense, not approachable AT ALL unless you are already a nix expert... it doesn't use English it uses tech jargon and assumes the reader speaks that language." The report failed its only job — the owner still cannot merge the PR it explains.
Two defects, both slated for the next slice (register/reader fixes, the slice promoted by the recorded reorder):
1. The prompt has no reader contract. It governs structure, layers, budget, and slop, but nowhere states who the reader is. The author model defaults to writing for a peer — someone already fluent in the diff's stack. Fix: an explicit reader contract in the author prompt (sharp generalist engineer, NOT fluent in this stack; every term of art defined in plain English at first use or not used; concept layer must read as plain English; jargon spends its budget only in receipts), and the triage prompt must size reading_minutes for a reader who lacks the prerequisite concepts triage itself lists.
2. The quiz locks corrective feedback after a wrong answer. On first commit, the enhancement seals every other choice's feedback body — including the correct answer's rationale when the reader is wrong. Commitment-before-feedback and first-attempt capture are correct; withholding correction is the opposite of the research (retrieval practice without corrective feedback is the one configuration that reinforces errors). With scripts disabled the quiz is complete, so the enhancement degrades learning below its own no-JS fallback. Fix: lock the input, reveal everything — first answer recorded and immutable, then all choice explanations open.
Both land in one PR so the next real run (the owner needs archetypes#39 explained to clear a deploy pileup) exercises them together via the live-checkout fallback, before merge.
Third reader datapoint, from the allod/archetypes#39 rerun under the reader-contract prompt: better, still gapped. Two defects, both fixed by amendment to the reader-contract change before relay.
1. Definitions decay into insider shorthand. The report defined its central dependency once, appositively ("Upstream microvm.nix runs small virtual machines under systemd"), then used bare "upstream" as a proper noun for the rest of the document — including in a prediction question 35 lines later, where the reader had to stop and ask what the word meant. A definition that does not survive the distance to its next use never happened. The contract's "defined at first use" rule was satisfied by the letter and failed in effect. Fix: outside projects are called by their actual name at every mention; bare insider nouns for dependencies are banned outright.
2. Prediction reveals hold the worst register in the report. The reveal answering that same prediction read: "It is not a plain
true. Upstream writesdefault = config.config != null: the flag is on exactly when the guest was declared in the first shape. A guest handed over already evaluated defaults to being left alone." The facts are correct; the prose is unreadable compression. Two structural causes: (a) the contract demanded end-to-end plain English only for concept sections, and this reveal sits in a mechanism-layer section where the weaker "jargon where precision pays" clause governs — the author model found the seam; (b) the slop pass is deletion-only by design, so it can cut a garbled sentence but never rewrite one, and it cannot delete a reveal without gutting the prediction element. Broken register inside disclosure elements is therefore invisible to every net downstream of the author pass. Fix, in the author prompt where it has to live: a fourth reader-contract rule — text inside anydetailselement (prediction reveals, choice explanations, optional depth) is body text held to the plainest register in the report, because a reader opens it at their moment of greatest uncertainty; and a prediction question may only use terms the prose above it has already established.Pattern worth keeping for PR B: both defects are places where the shape of an element (appositive definition, disclosure element) let compressed prose hide from a contract written about sections. The register contract has to bind to text, not to structure.
Fourth defect from the same allod/archetypes#39 read, distinct failure mode from the register defects above: the quiz taught instead of tested. The consequence story around the restart-on-rebuild behavior — what restarting a changed guest actually does, why microvm.nix's default avoids it for handed-over machines, and why overriding that default is right for this fleet — never appeared in the body. Its only surface was a quiz element, where the reader meets it for the first time while being scored on it. That inverts the pedagogy twice over: retrieval practice only strengthens material that was taught, and an override explained nowhere reads as either arbitrary or reckless.
Two prompt fixes, in the reader-contract change before relay: (a) quiz rule — the quiz tests only what the report taught; before keeping an item, point to the body paragraph that gives a first-time reader enough to answer it, and if the knowledge lives only in a stem or feedback body, the body is missing a paragraph — add the teaching and keep the question; (b) evidence rule — every deliberate value the change sets gets its tradeoff taught in the body, and when the change overrides an outside project's default, teach what that default protects before explaining the departure.
Dogfood catch in the unrelayed reader-contract change, from the same read: the first quiz-unlock design was buggy and overbuilt. It force-opened every choice while the choices still shared a
nameattribute — and same-namedetailselements are the browser's own exclusive accordion, the very mechanism the no-JS fallback depends on. So every explanation flashed open, then the browser closed all but the last, including the explanation for the answer the reader had just picked.Replaced with the simpler contract the situation actually called for: record the first answer (immutable, for the future reader model), remove the
nameattributes so the native exclusivity ends, and let the reader open and close every explanation at their leisure. No locking, no forced opening, no disabled summaries. The correct choice is highlighted once the item is answered. Lesson recorded: an enhancement must not fight the native mechanism its own fallback relies on.Fifth datapoint from the allod/archetypes#39 read, and the most instructive: report quality decays with output position. The owner rated the opening sections readable and the late mechanism/receipts sections ("Why a green check was checking nothing") unreadable — compressed clause-chains in the model's native register, despite the reader contract at the top of the prompt.
Three causes, one of them self-inflicted:
Amendment also adds a decay-directed self-check as the first item of the final re-read: the last third of the body is re-read against the reader contract specifically, because that is where the drift concentrates. Realistic expectation, recorded for honesty: (2) and the self-check are enforceable by prompt; (1) is architectural, and if late-section drift survives the amendment, the fix is bounded per-section authoring in the pipeline, not a sixth rule.
Sixth datapoint from the allod/archetypes#39 read: a content-blind layout made a correct diagram unreadable. The "one credential, end to end" trace was authored as a five-step
rx-flowwhose nodes carried literal paths and a full-fw_cfgcommand line. The flow component is vertical by default but auto-flips to side-by-side columns on wide containers, keyed only on step count and container width — it never measures the text. Five nodes of command-line payload became five crushed columns, while the neighboring section'srx-timeline(top-down, prose-width rows) stayed readable with equally long content.Two fixes in the reader-contract change: (a) an author rule making the flip's assumption explicit — a flow node's entire text must survive being one narrow column, so spans stay phrase-length and literal paths, command lines, and full sentences disqualify the flow in favor of a top-down
rx-sequenceor a codewalk; (b) a CSS defense (overflow-wrap: anywhereon flow spans) so an errant long token wraps inside its box instead of breaking the row. Considered and rejected: an author-facing orientation attribute — the validator pins the flow tag's exact shape, and the defect is content in the wrong component, not a missing knob.Seventh datapoint from the allod/archetypes#39 read, this one an improvement request rather than a defect: the reports test mechanical understanding but not conceptual reasoning. The multiple-choice items ask the reader to recognize correct behavior; nothing asks the reader to derive the design. The request came directly out of a conversation where an open why-question — "why can't the host repository know the guest's credential names?" — forced a reconstruction of the drift-prevention argument that no recognition exercise would have produced. The learning literature agrees: elaborative interrogation and self-explanation are among the highest-utility techniques, and both need why-questions posed before the answer is available.
Two prompt additions in the reader-contract change, using the existing
details.rx-predictgrammar — no component or validator changes: (a) a "reasoning before explanation" rule — wherever the body teaches a deliberate design decision (a fact owned by one repository and not another, a default overridden, a check deliberately not written, a gate placed here and not there), pose the why-question before explaining the decision, with a reveal that gives the reasoned answer in plain body sentences and names what the rejected alternative would break; (b) a quiz requirement that at least one item tests design reasoning — why this design rather than a plausible alternative — with the tempting alternatives as distractors whose feedback names what each would break. Grading stays where it can live in a no-script, self-contained file: recognition items are scored, derivation questions are commit-then-compare against the revealed reasoning.Eighth datapoint from the allod/archetypes#39 read: section headings vaguepost. The report's table of contents reads like a feed, not an outline — "The two halves, and the words for them", "The crossing, line by line", "Two lifecycle flags and one destructive overlap", "Why a green check was checking nothing". Every one withholds its subject to manufacture intrigue. In an educational document a heading is navigation: a reader returning weeks later must be able to find one fact from the headings alone, and a table of contents read by itself should outline the change. Teaser headings defeat both, and they are the model's native headline-writing register leaking into infrastructure.
Prompt fix in the reader-contract change, placed where section naming is defined: headings state the noun or single mechanism their section explains; never withhold the subject, tease an unnamed problem, or write a heading whose referent is only clear after reading the section; a why- or how-clause is welcome only when it names its subject in the same breath ("Why the assertion check passed on unbuildable hosts", never "Why a green check was checking nothing"). The rule extends to disclosure
summarylines and figure captions: say what the thing is, not how surprising it is.Status checkpoint. Merged to master: the v2 pipeline (#141), pi as a third API-metered runner (#150), and the reader-contract slice — eight fixes surfaced by human reads of one real T3 report, one datapoint comment each above (5082, 5095, 5097, 5099, 5100, 5102, 5104, 5106).
Remaining slices, in order of measured pain: (1) primer library — carrying two watch items from the T3 read: if register decay in a report's final third survives the prompt-level fix, the real fix is per-section authoring with the reader contract re-anchored per call, and watch for components whose visual shape hides compressed prose; (2) reader model + quiz capture + card export (the quiz handler already records first answers immutably in the DOM for this); (3) SVG and the remaining interactive vocabulary.
Validation plan: no rerun of the already-read report. The next generated report is the test — check whether the decay, heading, and flow rules held.
Ninth datapoint, from the owner's read of the third allod/archetypes#39 generation (regenerated under the merged reader-contract prompt): much better than the previous generation, and the positional decay survived anyway. The opening sections hold the contract; the back half slides into teaser headings and compressed model-register prose. That is the exact trigger the fifth datapoint recorded — the prompt-level fix class is exhausted, and the pipeline change it named becomes the active slice: bounded per-section authoring with the reader contract re-anchored per call.
Outside evidence agrees with the recorded diagnosis, for the archive:
Dev plan:
allod/strategydev-plans/pr-explain-sectioned-authoring.md. This slice supersedes the watch item the status checkpoint parked on the primer-library slice; the primer library follows it unchanged.Also recorded so nobody chases it as a report defect: the "massive blank space at the end of the page" the owner saw is the forge file view, not the template. Forgejo embeds committed HTML as an
iframewithheight="300", takes the firstframeHeightpostMessage from the reporter script it appends to the rendered document, sets the height once, and removes its message listener — nothing re-measures on window resize or zoom, and the report's height is strongly width-dependent (about 25.5k px at 380 px wide versus 18.8k px at 1400). Any window-size change after load strands the iframe at a stale height. The standalone page renders flush at every width with scripts on or off, measured in headless Chromium. Workaround: refresh after resizing, or open the/render/URL directly.