Repository navigation
docs(adrs): bring status lines in line with the code - #2988
Conversation
Thirteen ADRs were marked Proposed although source files under crates/ and services/ cite them as the rationale for shipped code. Each was checked decision by decision against the code. Four are fully built and move to Accepted with an Implemented note: 0093, 0774, 1195 and 2414. Nine are built in part and move to Accepted with a Partly implemented note naming the decisions that are not built: 0107, 0699, 0873, 0904, 0979, 0996, 1170, 1196 and 2066. None was left at Proposed. ADR-0105 stays Accepted and now says it is not implemented: section kind 7 is only reserved in the RLOG footer. ADR-0849 had no Status line; it gets the Accepted status the index already gave it and a note that no source file cites it. The index in docs/adrs/README.md now matches each changed status. No decision text changed. Fixes: #2950 Signed-off-by: Panagiotis Moustafellos <pmoust@nofire.ai>
|
@claude-fleet review |
|
Review task queued. Task: 7166c3ff-9abb-422a-8994-6b550fafac69 |
![claude-fleet[bot]](https://avatars.githubusercontent.com/in/4876296?s=60&v=4){kind=small}
There was a problem hiding this comment.
Request changes: two should_fix findings, both in the status claims this PR exists to make true.
The code checks behind the new notes hold up. I opened every path and symbol cited and all of them exist and say what the notes say, with one exception (finding 1). The gap this PR leaves is referential: moving ADR-1170 and ADR-0979 off Proposed falsifies four sentences elsewhere that cite them as Proposed, two of them on lines this PR rewrites.
Verified:
- ADR-0093 resolver, allowlist extraction and the I64
INenvelope —crates/ravel-sql/src/logs_pushdown.rs:187,283,325,428 - ADR-0105 kind 7 reserved with no writer or reader; it is the only
.rscitation of the ADR —crates/ravel-logseg/src/footer.rs:134 - ADR-0699 decision 3 stale:
pub const VERSION: u16 = 5, "the only version this build reads" —crates/ravel-logseg/src/footer.rs:43,138 - ADR-0774
TopKLateMaterialization/LogsRowFetchExec—crates/ravel-sql/src/late_materialization.rs:248,34 - ADR-0849 is cited by no source file (
grep -rn "0849" --include=*.rs crates/ services/→ no hits) - ADR-0873:
ValidatedDeclaredStatsis constructor-gated for the stamp carriers only (crates/ravel-commit/src/declared_stats.rs:272,391,474), while the.cstatpath validates inline with no such type (crates/ravel-sql/src/logs_scan/mod.rs:888-960) — so "decision 2's validated.cstattype is not built" is right - ADR-0904 decision 5:
accounting_stats_jsoncarries no opens-by-shape field, whileQueryAccountingdoes —services/ravel-server/src/query.rs:31-59,crates/ravel-types/src/accounting.rs:1159 - ADR-0996:
RequestCountshas the attempts fields (crates/ravel-bench/src/report.rs:336,341) but the per-statement rows do not (crates/ravel-bench/src/sql_latency.rs:584,606), andrange_amplificationexists only as its denominator counter —crates/ravel-types/src/accounting.rs:328 - ADR-1170 decision 3:
MEMORY_OVERHEAD_RESERVE_BYTES = 2 GiB, documented as "A provisional round figure, not a measurement" —crates/ravel-maintain/src/config.rs:581-596 - ADR-1195 fully implemented:
GetLimiterplus all three flags and the legacy alias —crates/ravel-query/src/limiter.rs:29,services/ravel-server/src/config.rs:1660-1698 - ADR-1196 decision 5: no
latency-firstentry underbenchmarks/clickbench/(grep → no hits) - ADR-2066 decision 1:
bounded_chunk_runs,place_front_sections—crates/ravel-query/src/log_fetcher.rs:6826,7095 - ADR-0107 decision 5:
docs/guides/caching.mdsizes against caps and working set generally (168-185), with nothing derived from measured block-range working sets - The README index matches all 15 changed status lines
Checks run:
- Existence check over all 27 cited paths → all present
grep -rn "still Proposed\|(Proposed)\|is Proposed" docs/filtered to the re-statused ADRs → four hits, finding 1 and 2- No tests run: the diff is documentation only.
Smaller things:
docs/adrs/0979...md:6repeats what the ADR already says at line 323 ("Until D5's--merge-cursor-budget-bytesflag exists there is no way to set the budget explicitly"). Two copies to keep in sync; the body line is the one with the surrounding argument.- ADR-1170's README row reads "Accepted (2026-10-14; partly implemented); amended by 2633 (2026-10-11)", an amendment dated before the acceptance. Correct as a record, but worth a glance.
Not checked:
- ADR-0996 decision 4's bands beyond range_amplification: the note says three other figures have no producer, and I confirmed only the range_amplification and per-statement-attempts halves
- ADR-2066 decision 2's claim that the before/after measurement has no record in the repo: I confirmed no mention under docs/ or benchmarks/, but a measurement could live on issue #2066 outside this checkout
- ADR-2414's B2/B3 claims beyond the existence of docs/internal/clickbench-aws-runbook.md and the #2416 amendment reference
- Whether marking ADR-0849 Accepted reflects a real approval decision, as opposed to aligning the file with the index row that already read Accepted
Task 7166c3ff-9abb-422a-8994-6b550fafac69 on rp1, model claude-opus-5, effort medium.
| Partly implemented: D1 to D4 are in `crates/ravel-maintain/src/rlog.rs`, | ||
| `crates/ravel-maintain/src/config.rs` and `crates/ravel-logseg/src/ranged.rs`; | ||
| D5's `--merge-cursor-budget-bytes` flag is not built | ||
| (`services/ravel-cli/src/main.rs` has the other three flags only). |
There was a problem hiding this comment.
should_fix: The note overstates what landed of D5: compact-bucket carries two of the four flags, not three.
Failure: a reader checking D5's exposure believes maintain compact-bucket accepts --input-read-concurrency and finds it rejected; the ADR's own D5 requires all four flags on both compact-tenant and compact-bucket (docs/adrs/0979:330-335).
Evidence: services/ravel-cli/src/main.rs:1700-1769, the CompactBucket variant, declares l1_part_memory_target_bytes and max_l1_part_bytes and no input_read_concurrency; the CompactTenant variant at 1839-1857 declares all three.
Fix: say the three flags are on compact-tenant and two of them on compact-bucket, which is also one more unbuilt piece of D5 than the note admits.
| # ADR-1170: One process-wide memory budget for ravel-server | ||
|
|
||
| Status: Proposed | ||
| Status: Accepted (2026-10-14). |
There was a problem hiding this comment.
should_fix: Moving ADR-1170 to Accepted falsifies three sentences that cite it as Proposed, two of them in files this PR edits.
Failure: docs/adrs/README.md now says ADR-1170 is Accepted on line 172 and "ADR-1170 (Proposed) is edited in place by the implementing task" on line 187, a contradiction inside the single file this PR rewrote to be the index of record.
Evidence: docs/adrs/README.md:187; docs/adrs/2414-read-path-narrow-projections-and-stock-memory-for-the-reference-suite.md:6-7 ("ADR-1170 (the derived per-query SQL share) is still Proposed, so task B1 edits its share in place"), whose Status line this PR also changed; docs/adrs/2633-resident-aware-memory-admission.md:422 ("ADR-1170 is Proposed, so it is not edited by this draft"), outside the diff.
Fix: the first two are in the diff and should be updated here; 2633 can be noted as follow-up if you want to hold the "no decision text edited" line.
| # ADR-0979: Bounded-memory RLOG compaction merge | ||
|
|
||
| Status: Proposed | ||
| Status: Accepted (2026-10-14). |
There was a problem hiding this comment.
nit: Same class as the ADR-1170 finding, one file and outside the diff: accepting ADR-0979 leaves a stale citation of it as Proposed.
Failure: a reader of the TLA+ suite's maintenance row is told the compaction convergence gap rests on a Proposed decision, when the decision is now Accepted and D1-D4 have landed.
Evidence: docs/adrs/1113-tla-verification-suite.md:31, "ADR-0979 is Proposed and its fail-closed convergence path ships".
1 participant
Thirteen ADRs marked Proposed had decisions that are live in the code. Each was checked decision by decision against the source.
Each partly implemented ADR has a note line under its status naming what is built and what is not, with the code location.
crates/ravel-logseg/src/footer.rs, and nothing writes or reads it.docs/adrs/README.mdindex matches every changed entry.No decision text was edited.
Landed without fleet review at the owner's request. CI is the gate.
Fixes: #2950