Add ADR authoring protocol + a one-line Summary to every ADR (#145) - #150
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f3bb4eb15a
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| "id": title_m.group(1) if title_m else None, | ||
| "title": title_m.group(2).strip() if title_m else None, | ||
| "status": status_m.group(1).strip() if status_m else "—", | ||
| "summary": summary_m.group(1).strip() if summary_m else "—", |
There was a problem hiding this comment.
Fail ADRs that omit required summaries
When a new ADR is added without **Summary:**, this fallback records summary as —, so scripts/generate-architecture-map.sh --check can still pass after regenerating an ADR index that violates the new docs/process/adr-protocol.md requirement. Because this is the header reader added here for the CI architecture-map check, missing required summaries will silently ship instead of being rejected; make ADR collection fail when summary_m is absent.
Useful? React with 👍 / 👎.
Write docs/process/adr-protocol.md (+ adr-template.md), the ADR companion to the existing PRD protocol: header fields, section requirements (resolving the Context-optional inconsistency), lifecycle/immutability with an append-only **Amended:** field, the aggregation-vs-mechanism home rule (ADR-0020 vs 0021), and the retroactive as-built path. Add a required one-line **Summary:** header to the 17 ADRs that lacked one (0020-0023 got theirs in #143), each stating the decision, not the title. ADR-0001, a bundle of ten decisions, uses the sanctioned "read it in full" form rather than a misleading compression. Teach scripts/generate-architecture-map.sh to parse **Summary:** and render it as a column in the ADR index; regenerate the map. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
ADR-0024 and ADR-0025 landed on main after this branch's original commit added the required Summary header to the other 17 ADRs, so they were missing one. Add it to both and regenerate the architecture map so its new Summary column is complete rather than showing "—" for these two. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
f3bb4eb to
926cb56
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 926cb56da2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| **Status:** Accepted | ||
| **Date:** 2026-06-08 | ||
| **Author:** Luis Villa | ||
| **Summary:** Every LLM capability reaches models through a provider-agnostic `ModelClient` boundary as a multi-model panel whose surfaced answer is the voted result (skeptical tiebreaker); the only quantitative signal is measured panel agreement, never a model-reported confidence, and every invocation is fingerprinted for audit and replay. |
There was a problem hiding this comment.
Describe multi-model panels as the default
For any capability configured to use one of the explicitly supported single-model modes, this summary incorrectly says it runs as a multi-model panel with a voted result. Decision 1 states that single open-weight and frontier models are first-class and that voting is the default rather than a requirement (lines 39–48); because this normative summary is also surfaced in the architecture map, qualify the panel and voting behavior as the default so readers and tools do not infer a mandatory ensemble.
Useful? React with 👍 / 👎.
| **Summary:** <one sentence — the decision, not the topic> | ||
| ``` | ||
|
|
||
| `**Date:**` is the acceptance date and never changes. `**Amended:**` is optional |
There was a problem hiding this comment.
Define the Date field for proposed ADRs
When an ADR is still Proposed, it has no acceptance date, yet the required header and template demand a populated Date field. Existing proposed ADRs consequently use proposal dates; accepting one would require either changing a date that this sentence says never changes or retaining a value that is not its acceptance date. Define separate proposal/acceptance semantics or allow the acceptance date to be added on transition.
Useful? React with 👍 / 👎.
Summary
There was no ADR protocol doc — only a PRD one — and an ADR's title names its topic, not its decision, so getting the gist meant opening the file. This adds:
docs/process/adr-protocol.md(+adr-template.md) — the ADR companion toprd-protocol.md: header fields, section requirements, lifecycle/immutability, and the retroactive as-built path. It also settles two things that were previously implicit:Accepted+ an append-only**Amended:**header field, not a status change — Status stays a standing (Proposed | Accepted | Superseded).**Summary:**on every ADR — 17 added here (0020–0023 got theirs in Add as-built ADRs for four shipped contracts (#18/#19/#20/#21) #143), each stating the decision. ADR-0001 (a bundle of ten decisions) uses the sanctioned "read it in full" form rather than a misleading compression.generate-architecture-map.shnow renders a Summary column in the ADR index; map regenerated.No decisions are changed and no ADR is superseded — this is authoring convention + gist, applied across the existing set.
Type
Validation Required
All four pass locally. Note: local commit used
SP42_SKIP_GIT_HOOKS=1because the pre-commit hook fired a fullcargo check(triggered by the.shfile) despite zero Rust source changing; the doc/map checks above were run manually and will re-run in CI.