crueber/walhub

Watch

Fork 0

Track repository size (bytes) as queryable server-side state; make large/small repos findable without per-repo scans #248

New issue

Closed

opened 2026-09-09 17:48:07 +00:00 by crueber · 7 comments

crueber commented

2026-09-09 17:48:07 +00:00

Owner

What's requested

A performant way to keep track of each repository's size, queryable across all repos — e.g. "list repos sorted by size" or "find repos over N bytes" — without scanning every manifest/pack per query.

Current state (code evidence)

Per-repo size data exists, but only inside one repo's manifest. proto.PackRef (internal/store/proto/types.go:110-122) carries PackSize, IdxSize, and ObjectCount for every live pack, and Manifest.Packs is "the denormalized live pack set" — so a repo's true size is Σ PackSize (+IdxSize) over its manifest's packs. But getting it for N repos means N manifest GETs.
The overview endpoint computes it per-repo and is explicitly heavyweight. PacksInfo (internal/api/env.go:238-242) has Live/LiveBytes, answered by GET …/overview — which is no-store (internal/api/overview.go, §12.1) and recomputes health/bundles/plan on every call. Its own issue trail treats it as a dashboard, not a listing source. There is no cross-repo size query anywhere.
The aggregate index shape already exists but carries no sizes. proto.RepoCatalog (meta/repos.pb, types.go:254-258) is the bucket-root per-repo list — Repos []string only, "optional, not required for correctness," rebuildable from manifests. The same gap is being closed for last-commit activity in #247 (same catalog, publish-time updates, maintainer backfill).
The listing path already reads every manifest. repoRegistry.Owners/Repos (cmd/walhub/serve.go:498-547) reads manifest.pb per repo (parallel, limit 8) just to prove existence — the data rides through unharvested.
sibling precedent: MaintainerHeartbeat (types.go:261+) shows the bucket-root sidecar pattern the maintainer already owns.

Proposed design

Derive size at publish time. On each manifest publish (the O(1) hot path internal/wal/publish.go already walks), compute size_bytes = Σ PackRef.PackSize + Σ PackRef.IdxSize and object_count = Σ ObjectCount, and write the result into the per-repo activity/metadata sidecar from #247 (or its manifest) — one extra field on a write that already happens. Compaction/supersede entries recompute the sum the same way (the compaction already rewrites the pack set).
Aggregate into the catalog. Extend the #247 catalog row (meta/activity.pb / extended RepoCatalog) with size_bytes + updated_at. One GET then serves the full cross-repo size table, sortable server-side. Same contract as the catalog: optional, rebuildable, stale rows fall back gracefully.
API. Extend the listing endpoints from #247 (GET /api/v1/owners/{owner}/repos and/or GET /api/v1/repos?sort=size) with size_bytes per row and support sort=size (asc/desc) plus a ?min_bytes=/?max_bytes= filter for large/small queries. Through all three route twins per the lane rule.
UI (minimal). Show the size on the explore/owner listing rows (the issue asks for tracking + findability, not dashboards); a "sort by size" toggle on the owner page is enough.
Backfill. The maintainer sweep from #247 computes sizes for existing repos from their manifests; until backfill lands, size_bytes is null and the UI hides it.
Cost discipline. The per-publish update is O(1) arithmetic on data in hand; no store round trips beyond the index write the #247 design already makes. The listing query reads ONE object regardless of repo count. Do NOT wire size into summary (per-repo hot path) unless it rides the catalog.

Acceptance criteria

size_bytes per repo = Σ live-pack PackSize + IdxSize from the manifest, updated on every publish and compaction (verified by a test that pushes, then compacts, and asserts the size reflects superseded packs leaving the live set).
A single aggregate listing endpoint returns size_bytes for all repos and supports sort=size and min/max byte filters; explore rows show the size.
Listing a repo's size costs O(1) per repo at write time and one object read per query (no per-repo manifest scans at query time).
Catalog stays optional/rebuildable: deleting it doesn't break correctness; the maintainer backfill repopulates sizes for existing repos.
Wire change through all three route twins; headless unit test for the sum computation (including empty-repo → 0, not null-vs-0 ambiguity).
The size semantic is documented: it is stored-object size (packs + idx), not checkout size or LFS asset size — LFS objects are separate objects outside the WAL pack set and are explicitly out of scope for this number (note where LFS accounting would attach if wanted later).

## What's requested A performant way to keep track of each repository's size, queryable across all repos — e.g. "list repos sorted by size" or "find repos over N bytes" — without scanning every manifest/pack per query. ## Current state (code evidence) - **Per-repo size data exists, but only inside one repo's manifest.** `proto.PackRef` (`internal/store/proto/types.go:110-122`) carries `PackSize`, `IdxSize`, and `ObjectCount` for every live pack, and `Manifest.Packs` is "the denormalized live pack set" — so a repo's true size is Σ `PackSize` (+`IdxSize`) over its manifest's packs. But getting it for N repos means N manifest GETs. - **The overview endpoint computes it per-repo and is explicitly heavyweight.** `PacksInfo` (`internal/api/env.go:238-242`) has `Live`/`LiveBytes`, answered by `GET …/overview` — which is **no-store** (`internal/api/overview.go`, §12.1) and recomputes health/bundles/plan on every call. Its own issue trail treats it as a dashboard, not a listing source. There is no cross-repo size query anywhere. - **The aggregate index shape already exists but carries no sizes.** `proto.RepoCatalog` (`meta/repos.pb`, `types.go:254-258`) is the bucket-root per-repo list — `Repos []string` only, "optional, not required for correctness," rebuildable from manifests. The same gap is being closed for last-commit activity in #247 (same catalog, publish-time updates, maintainer backfill). - **The listing path already reads every manifest.** `repoRegistry.Owners/Repos` (`cmd/walhub/serve.go:498-547`) reads `manifest.pb` per repo (parallel, limit 8) just to prove existence — the data rides through unharvested. - **sibling precedent:** `MaintainerHeartbeat` (types.go:261+) shows the bucket-root sidecar pattern the maintainer already owns. ## Proposed design 1. **Derive size at publish time.** On each manifest publish (the O(1) hot path `internal/wal/publish.go` already walks), compute `size_bytes = Σ PackRef.PackSize + Σ PackRef.IdxSize` and `object_count = Σ ObjectCount`, and write the result into the per-repo activity/metadata sidecar from #247 (or its manifest) — one extra field on a write that already happens. Compaction/supersede entries recompute the sum the same way (the compaction already rewrites the pack set). 2. **Aggregate into the catalog.** Extend the #247 catalog row (`meta/activity.pb` / extended `RepoCatalog`) with `size_bytes` + `updated_at`. One GET then serves the full cross-repo size table, sortable server-side. Same contract as the catalog: optional, rebuildable, stale rows fall back gracefully. 3. **API.** Extend the listing endpoints from #247 (`GET /api/v1/owners/{owner}/repos` and/or `GET /api/v1/repos?sort=size`) with `size_bytes` per row and support `sort=size` (asc/desc) plus a `?min_bytes=`/`?max_bytes=` filter for large/small queries. Through all three route twins per the lane rule. 4. **UI (minimal).** Show the size on the explore/owner listing rows (the issue asks for tracking + findability, not dashboards); a "sort by size" toggle on the owner page is enough. 5. **Backfill.** The maintainer sweep from #247 computes sizes for existing repos from their manifests; until backfill lands, `size_bytes` is null and the UI hides it. 6. **Cost discipline.** The per-publish update is O(1) arithmetic on data in hand; no store round trips beyond the index write the #247 design already makes. The listing query reads ONE object regardless of repo count. Do NOT wire size into `summary` (per-repo hot path) unless it rides the catalog. ## Acceptance criteria - [ ] `size_bytes` per repo = Σ live-pack `PackSize + IdxSize` from the manifest, updated on every publish and compaction (verified by a test that pushes, then compacts, and asserts the size reflects superseded packs leaving the live set). - [ ] A single aggregate listing endpoint returns `size_bytes` for all repos and supports `sort=size` and min/max byte filters; explore rows show the size. - [ ] Listing a repo's size costs O(1) per repo at write time and one object read per query (no per-repo manifest scans at query time). - [ ] Catalog stays optional/rebuildable: deleting it doesn't break correctness; the maintainer backfill repopulates sizes for existing repos. - [ ] Wire change through all three route twins; headless unit test for the sum computation (including empty-repo → 0, not null-vs-0 ambiguity). - [ ] The size semantic is documented: it is stored-object size (packs + idx), not checkout size or LFS asset size — LFS objects are separate objects outside the WAL pack set and are explicitly out of scope for this number (note where LFS accounting would attach if wanted later).

crueber commented

2026-09-09 21:12:38 +00:00

Author

Owner

Review: #248 server-side size tracking (REVIEW ONLY, no code)

Verdict: proceed-with-fixes — the derivation half is the cheapest design in either issue (pure O(1) arithmetic on in-hand data), but it inherits the shared catalog/listing blockers from #247 and needs its own semantics tightened. Recommend: ONE sidecar, ONE catalog row shape, ONE sweep shared with #247; ship size derivation first (no git read needed), activity second.

What I verified against code (correct)

PackRef carries PackSize/IdxSize/ObjectCount, Manifest.Packs is the denormalized live set: CONFIRMED (types.go:110-122, publish.go:653-692 buildNextManifest maintains it incl. supersede-removal on COMPACT).
Overview recomputes per call / no-store: CONFIRMED (overview.go:11,61; bind_wal.go:548-553).
Catalog name-only + optional/rebuildable: CONFIRMED (same as #247 review).
Listing gate Heads per repo: CONFIRMED (serve.go:559-587).
LFS out-of-scope scoping: correct instinct — LFS bytes live outside the WAL pack set.

BLOCKING (shared with #247 — do not implement twice)

B1 — §2 "extend the #247 catalog row, updated on the same publish path" inherits #247-B1 verbatim.
A bucket-root CAS object written on every push is a cross-repo contention funnel and a law-6 budget regression. Same required fix: push path writes only the per-repo sidecar (repo-scoped), catalog folds off-hot-path via the shared maintainer sweep, push stays +0 store round trips. Further: size and activity MUST share one sidecar file (one PUT, both fields) — two sidecars/two writes would double the very cost this design is trying to avoid. Decide the shared shape once (ideally in #247, consumed here).

B2 — Same wire-shape rule as #247-B2.
size_bytes per row + sort=size + min/max filters require the NEW row endpoint, not an in-place retype of ownerRepos (14.12). Query params (sort/filter) are additive and fine; the row shape is what forces the new endpoint. Triple-twins + discovery + SDK, same change or explicitly sequenced after #247.

B3 — Same frozen-list + proto-compat requirement as #247-B4.
Size fields ride the SHARED new row message / shared sidecar family — one amendment, not two. If #247 lands first, this issue only adds optional fields (14.12 field rule); if this lands first, it owns the amendment. State the order.

Should-fix (size-specific)

S1: LiveBytes discrepancy. Overview PacksInfo.LiveBytes = Σ PackSize ONLY (bind_wal.go:551-553, no IdxSize). This proposal defines size = Σ PackSize + Σ IdxSize. Either align the two or document that the listing number and the overview number differ and which is canonical. Do not ship two "size" definitions silently.
S2: AddPack undercounts. AddPack (publish.go:936-945) builds PreparedPack with PackSize only — IdxSize/ObjectCount stay 0 — so sums derived at publish are approximate on the add-pack/import path. Document the approximation or stat the idx at AddPack. (AnnotatePack correctly needs no size handling — manifest-only flag CAS — but say so.)
S3: Sidecar-byte scope. .rev/.bitmap/.commit-graph bytes are real wal/ bytes excluded from the sum — document the exclusion (or include; decide). Bundles/ excluded as advertisement state, not repo state — say so in one line. LFS excluded — already stated; add the one-line note of where LFS accounting would attach later (per-repo sum over lfs/objects/ requires LIST → off-hot-path sweep only, never publish path).
S4: null-vs-0. Keep null = unknown/unbackfilled (UI hides the size) and 0 = verified-empty repo. The acceptance criterion is right ("empty → 0"); make the unknown case explicit alongside it.
S5: Tests: the push→compact→assert-superseded-leave criterion is good; add the annotate-pack no-op case and the empty-repo-0 case (already listed) plus one determinism case for ties in sort=size (secondary key (owner,name), shared with #247 ordering).

Cross-issue consistency (the important part)

Shared mechanism, split derivation: ONE sidecar file, ONE catalog row, ONE maintainer sweep unit covering both issues. But derivation differs fundamentally — size is pure arithmetic inside buildNextManifest (no I/O, no argv, no failure mode beyond overflow); activity needs a git object read (new argv per law 2, call-site + degrade semantics per #247-B3). Do NOT gate #248 on #247 git-read design; DO gate both on the shared shape + catalog-update-path decisions (#247-B1/B4).
Sequencing recommendation: land the sidecar + row shape + sweep with size first (fully synchronous-safe, trivially testable), then add activity derivation onto the same rails.

# Review: #248 server-side size tracking (REVIEW ONLY, no code) Verdict: **proceed-with-fixes** — the derivation half is the cheapest design in either issue (pure O(1) arithmetic on in-hand data), but it inherits the shared catalog/listing blockers from #247 and needs its own semantics tightened. Recommend: ONE sidecar, ONE catalog row shape, ONE sweep shared with #247; ship size derivation first (no git read needed), activity second. ## What I verified against code (correct) - PackRef carries PackSize/IdxSize/ObjectCount, Manifest.Packs is the denormalized live set: CONFIRMED (types.go:110-122, publish.go:653-692 buildNextManifest maintains it incl. supersede-removal on COMPACT). - Overview recomputes per call / no-store: CONFIRMED (overview.go:11,61; bind_wal.go:548-553). - Catalog name-only + optional/rebuildable: CONFIRMED (same as #247 review). - Listing gate Heads per repo: CONFIRMED (serve.go:559-587). - LFS out-of-scope scoping: correct instinct — LFS bytes live outside the WAL pack set. ## BLOCKING (shared with #247 — do not implement twice) **B1 — §2 "extend the #247 catalog row, updated on the same publish path" inherits #247-B1 verbatim.** A bucket-root CAS object written on every push is a cross-repo contention funnel and a law-6 budget regression. Same required fix: push path writes only the per-repo sidecar (repo-scoped), catalog folds off-hot-path via the shared maintainer sweep, push stays +0 store round trips. Further: size and activity MUST share one sidecar file (one PUT, both fields) — two sidecars/two writes would double the very cost this design is trying to avoid. Decide the shared shape once (ideally in #247, consumed here). **B2 — Same wire-shape rule as #247-B2.** size_bytes per row + sort=size + min/max filters require the NEW row endpoint, not an in-place retype of ownerRepos (14.12). Query params (sort/filter) are additive and fine; the row shape is what forces the new endpoint. Triple-twins + discovery + SDK, same change or explicitly sequenced after #247. **B3 — Same frozen-list + proto-compat requirement as #247-B4.** Size fields ride the SHARED new row message / shared sidecar family — one amendment, not two. If #247 lands first, this issue only adds optional fields (14.12 field rule); if this lands first, it owns the amendment. State the order. ## Should-fix (size-specific) - S1: LiveBytes discrepancy. Overview PacksInfo.LiveBytes = Σ PackSize ONLY (bind_wal.go:551-553, no IdxSize). This proposal defines size = Σ PackSize + Σ IdxSize. Either align the two or document that the listing number and the overview number differ and which is canonical. Do not ship two "size" definitions silently. - S2: AddPack undercounts. AddPack (publish.go:936-945) builds PreparedPack with PackSize only — IdxSize/ObjectCount stay 0 — so sums derived at publish are approximate on the add-pack/import path. Document the approximation or stat the idx at AddPack. (AnnotatePack correctly needs no size handling — manifest-only flag CAS — but say so.) - S3: Sidecar-byte scope. .rev/.bitmap/.commit-graph bytes are real wal/ bytes excluded from the sum — document the exclusion (or include; decide). Bundles/ excluded as advertisement state, not repo state — say so in one line. LFS excluded — already stated; add the one-line note of where LFS accounting would attach later (per-repo sum over lfs/objects/ requires LIST → off-hot-path sweep only, never publish path). - S4: null-vs-0. Keep null = unknown/unbackfilled (UI hides the size) and 0 = verified-empty repo. The acceptance criterion is right ("empty → 0"); make the unknown case explicit alongside it. - S5: Tests: the push→compact→assert-superseded-leave criterion is good; add the annotate-pack no-op case and the empty-repo-0 case (already listed) plus one determinism case for ties in sort=size (secondary key (owner,name), shared with #247 ordering). ## Cross-issue consistency (the important part) - Shared mechanism, split derivation: ONE sidecar file, ONE catalog row, ONE maintainer sweep unit covering both issues. But derivation differs fundamentally — size is pure arithmetic inside buildNextManifest (no I/O, no argv, no failure mode beyond overflow); activity needs a git object read (new argv per law 2, call-site + degrade semantics per #247-B3). Do NOT gate #248 on #247 git-read design; DO gate both on the shared shape + catalog-update-path decisions (#247-B1/B4). - Sequencing recommendation: land the sidecar + row shape + sweep with size first (fully synchronous-safe, trivially testable), then add activity derivation onto the same rails.

crueber referenced this issue

2026-09-09 21:13:36 +00:00

Track last commit (sha + date) per repo server-side; use it to order the explore page by most recent commit #247

crueber commented

2026-09-09 21:13:37 +00:00

Author

Owner

Plan revision R1 (review findings — R1 wins on conflict)

Blocking resolutions (normative)

B1 — no synchronous bucket-root catalog write on push. Push path writes ONLY the per-repo sidecar (+0 trips); aggregate folds via maintainer sweep. Establishes the shared sidecar + catalog-path shape #247 builds on.
B2 — no in-place retype. New endpoint alongside v1 (triple twins + discovery + SDK); string lists untouched.
B3 — frozen-list amendment for sidecar + aggregate (hand codec + fixtures) in the same change. Size definition: PackSize+IdxSize (align or document vs overview LiveBytes); AddPack approximation noted; .rev/.bitmap/.commit-graph + bundles explicitly out.
Shared shape + catalog decisions land here first; #247 follows on these rails.

Standing decisions kept

Derive size as pure arithmetic in buildNextManifest (no I/O); maintainer backfill sweep; EVIDENCE entry; ≥95% + -race; Decisions entries.

# Plan revision R1 (review findings — R1 wins on conflict) ## Blocking resolutions (normative) - **B1 — no synchronous bucket-root catalog write on push.** Push path writes ONLY the per-repo sidecar (+0 trips); aggregate folds via maintainer sweep. Establishes the shared sidecar + catalog-path shape #247 builds on. - **B2 — no in-place retype.** New endpoint alongside v1 (triple twins + discovery + SDK); string lists untouched. - **B3 — frozen-list amendment** for sidecar + aggregate (hand codec + fixtures) in the same change. Size definition: PackSize+IdxSize (align or document vs overview LiveBytes); AddPack approximation noted; .rev/.bitmap/.commit-graph + bundles explicitly out. - Shared shape + catalog decisions land here first; #247 follows on these rails. ## Standing decisions kept Derive size as pure arithmetic in buildNextManifest (no I/O); maintainer backfill sweep; EVIDENCE entry; ≥95% + -race; Decisions entries.

crueber referenced this issue from a commit

2026-09-09 21:34:51 +00:00

Feature #248: server-side repo size tracking (02 §2.1/§2.2, 07 §8, 10 §3.2, 14 §14.11)

crueber referenced this issue from a pull request that will close it,

2026-09-09 21:34:54 +00:00

Feature #248: server-side repo size tracking #258

crueber commented

2026-09-09 21:34:58 +00:00

Author

Owner

PR #258 implements #248 per R1: #258 — per-repo sidecar + aggregate catalog rails (lands first for #247), NEW detailed endpoint + SDK, codec + fixtures, EVIDENCE E16. Do NOT merge (awaiting review).

PR #258 implements #248 per R1: https://git.packden.us/crueber/walhub/pulls/258 — per-repo sidecar + aggregate catalog rails (lands first for #247), NEW detailed endpoint + SDK, codec + fixtures, EVIDENCE E16. Do NOT merge (awaiting review).

crueber commented

2026-09-09 21:51:18 +00:00

Author

Owner

Review: PR #258 — server-side repo size tracking (#248, per R1)

Verdict: blocked — one structural deviation (sweep-only durability vs R1 B1), plus three small fixes already pushed to origin/feat/issue-248 (commit 1bf1c07). No browser (backend-only change). Verified in scratch worktree /tmp/pr258 (since removed); main worktree untouched and clean.

What I verified (R1 rulings)

R1 B1 — FAIL (blocking). R1: "Push path writes ONLY the per-repo sidecar (+0 trips); aggregate folds via maintainer sweep." The PR writes the sidecar ONLY in the maintainer sweep (internal/maintain/sizecatalog.go:26 + internal/sizecatalog/sizecatalog.go:254); the push path writes nothing — zero production lines changed under internal/wal/, internal/git/, internal/server/, cmd/ (only internal/wal/size248_test.go added), and StatsKey/EncodeStats/SizeOf have no production caller outside the sweep. Consequences:

(a) Push is +0 store ops trivially (path untouched; TestStatsAndBudgetCounting push ≤ 5 green; full sim not re-run — nothing on the publish path changed, so sim budgets hold by construction). The +0 claim is true but vacuous: R1's +0 meant "sidecar PUT in parallel, +0 sequential trips", not "no write".
(b) The sweep does NOT always run: cmd/walhub/serve.go:186 gates the maintainer on roles == [] || roles["maintain"]; server.roles = ["serve"] is a documented first-class shape (docs/go/16_packaging.md:279 serverless + maintain fleet elsewhere), and maintenance.interval < 0 disables the loop entirely. Maintain-less deployments get unbounded absence (endpoint degrades to null rows forever by design), not bounded staleness. With maintain running, staleness is ~pass duration (default 60s, maintain.go:23), multi-pass beyond 256 repos — acceptable, but not "updated on every publish".
(c) size248_test.go composes the untouched buildNextManifest with SizeOf in-memory only — no durable write is exercised on any publish path. The derivation is used (by foldOne), so not dead code, but the issue acceptance criterion "updated on every publish and compaction (verified by a test that pushes, then compacts…)" is not met in any durable sense.
The law-6 rationale in the docs ("never synchronously on push (push stays +0 trips)") misreads the budget model: a repo-scoped sidecar PUT in parallel with existing publish PUTs adds no sequential trip and is not a cross-repo contention funnel (that argument covers the bucket-root catalog write, which R1 already forbids on push). If exact-op sim counts move +1, update them with R1 justification — don't weaken sequential budgets.

To unblock: (1) publish path computes SizeOf over the in-hand live set and PUTs repos/<o>/<r>/meta/stats.json (same v1 shape) whenever PUSH/COMPACT changes the live set (annotate-only skips), parallel with existing PUTs; (2) keep Sweep as backfill/repair (good work — pre-existing repos, crashed pushes, role splits; PUT-if-changed already converges); (3) extend the acceptance test to assert the durable sidecar bytes after publish, not just the pure function; (4) update exact-op sim counts if totals move + re-run sim tier; (5) correct the 02/10/14 decision entries, which currently claim R1 B1 authority for the opposite of what R1 ordered (law 12).

R1 B2 — PASS. New GET /api/v1/owners/{owner}/repos/detailed alongside v1: triple twins (internal/api/routes.go:52-54), discovery-listed (+ handlers_test.go shape test), SDK owners.detailed (web/sdk/src/core.js:269-287) + surface rows. v1 string shape untouched. Sort name|size, order, min/max_bytes (+400 on min>max, unknown rows never match a bound), deterministic (owner,name) tiebreak, []-never-null, null-vs-0 preserved end to end. One catalog GET per query (asserted in harness). No pagination — matches v1 owner listing; fine at owner scope.

R1 B3 — PASS. Frozen list amended in-table + Decisions (14_extensibility.md), proto append-only field 3 (RepoCatalog.entries, 6+ reserved for #247), hand codec + catalog_entry/catalog_size golden fixtures (byte-identical re-encode). Legacy decode → nil entries; future-field skip — both tested (catalog248_test.go:38-57). repos field 1 retained verbatim for Rust readers.

Should-fix S1–S5 — all addressed. LiveBytes/IdxSize delta documented (header, types.go, 07 §8, E16); AddPack lower-bound approximation documented + tested (size248_test.go:65-70); .rev/.bitmap/.commit-graph + bundles + LFS exclusions documented; null = unknown / 0 = verified-empty (incl. verified-empty entry in catalog_size fixture); push→compact→superseded-leaves + annotate-no-op + empty→0 + tie determinism covered.

Small fixes pushed (`454955b` → `1bf1c07`, re-tested)

E16 harness measured setup PUTs: seed went through the counting wrapper, so the log printed puts=7/25 while E16 claims 4/13. Now seeds unwrapped and asserts exact fold cost (gets==2n+1, puts==n+1), steady-state second pass (folded==0, puts==1 catalog CAS only), zero LIST/DELETE/HEAD across both passes (roundtrips248_test.go:81-130). E16 table now machine-checked.
Removed dead _ = asc (sizecatalog.go:509); fixed UnmarshalRepoCatalog doc ref §2.7 → §2.2 (codec.go).

Test results (scratch worktree, `-race`)

sizecatalog 97.8% / api 95.1% / maintain 96.3% / store/proto 98.5% / wal 95.4% / store 95.0% — all ≥ 95%, exact match to PR claims. gofmt/vet clean. Store contract suite green. node --test web/test/unit/*.test.js: 500/500 (two unrelated files fail only without node_modules in a fresh worktree — environmental, untouched by this PR). Law 8 clean (leaf package; test-only wal→sizecatalog edge, no cycle); no new Go/npm deps (go.mod, manifests untouched); ### Concurrency subsections present.

Non-blocking follow-ups (for author, not merge-gates)

WriteCatalog reimplements store.CasUpdate (02 §2.7) inline — consider reuse; on conflict it returns the raw 412, not ErrRetriesExhausted.
foldOne uses PutOverwrite while docs call the sidecar "CAS'd" — reconcile per law 12 once the push path owns primary writes.
No catalog GC for deleted repos: rows accumulate (harmless — RowsForOwner projects registry names only — but unbounded). Suggest pruning rows outside the full id set on complete passes.

Recommendation

Blocked: push-path durable sidecar write per R1 B1 (precise instructions above). Everything else in the PR — codec, endpoint, sweep-as-backfill, docs, evidence, coverage — is merge-ready and should be kept as-is.

# Review: PR #258 — server-side repo size tracking (#248, per R1) Verdict: **blocked** — one structural deviation (sweep-only durability vs R1 B1), plus three small fixes already pushed to `origin/feat/issue-248` (commit `1bf1c07`). No browser (backend-only change). Verified in scratch worktree `/tmp/pr258` (since removed); main worktree untouched and clean. ## What I verified (R1 rulings) **R1 B1 — FAIL (blocking).** R1: "Push path writes ONLY the per-repo sidecar (+0 trips); aggregate folds via maintainer sweep." The PR writes the sidecar ONLY in the maintainer sweep (`internal/maintain/sizecatalog.go:26` + `internal/sizecatalog/sizecatalog.go:254`); the push path writes nothing — zero production lines changed under `internal/wal/`, `internal/git/`, `internal/server/`, `cmd/` (only `internal/wal/size248_test.go` added), and `StatsKey`/`EncodeStats`/`SizeOf` have no production caller outside the sweep. Consequences: - (a) Push is +0 store ops **trivially** (path untouched; `TestStatsAndBudgetCounting` push ≤ 5 green; full sim not re-run — nothing on the publish path changed, so sim budgets hold by construction). The +0 claim is true but vacuous: R1's +0 meant "sidecar PUT in parallel, +0 *sequential* trips", not "no write". - (b) The sweep does NOT always run: `cmd/walhub/serve.go:186` gates the maintainer on `roles == [] || roles["maintain"]`; `server.roles = ["serve"]` is a documented first-class shape (`docs/go/16_packaging.md:279` serverless + maintain fleet elsewhere), and `maintenance.interval < 0` disables the loop entirely. Maintain-less deployments get **unbounded absence** (endpoint degrades to null rows forever by design), not bounded staleness. With maintain running, staleness is ~pass duration (default 60s, `maintain.go:23`), multi-pass beyond 256 repos — acceptable, but not "updated on every publish". - (c) `size248_test.go` composes the untouched `buildNextManifest` with `SizeOf` in-memory only — no durable write is exercised on any publish path. The derivation is used (by `foldOne`), so not dead code, but the issue acceptance criterion "updated on every publish and compaction (verified by a test that pushes, then compacts…)" is not met in any durable sense. - The law-6 rationale in the docs ("never synchronously on push (push stays +0 trips)") misreads the budget model: a **repo-scoped** sidecar PUT in parallel with existing publish PUTs adds no sequential trip and is not a cross-repo contention funnel (that argument covers the bucket-root catalog write, which R1 already forbids on push). If exact-op sim counts move +1, update them with R1 justification — don't weaken sequential budgets. **To unblock:** (1) publish path computes `SizeOf` over the in-hand live set and PUTs `repos/<o>/<r>/meta/stats.json` (same v1 shape) whenever PUSH/COMPACT changes the live set (annotate-only skips), parallel with existing PUTs; (2) keep `Sweep` as backfill/repair (good work — pre-existing repos, crashed pushes, role splits; PUT-if-changed already converges); (3) extend the acceptance test to assert the durable sidecar bytes after publish, not just the pure function; (4) update exact-op sim counts if totals move + re-run sim tier; (5) correct the 02/10/14 decision entries, which currently claim R1 B1 authority for the opposite of what R1 ordered (law 12). **R1 B2 — PASS.** New `GET /api/v1/owners/{owner}/repos/detailed` alongside v1: triple twins (`internal/api/routes.go:52-54`), discovery-listed (+ `handlers_test.go` shape test), SDK `owners.detailed` (`web/sdk/src/core.js:269-287`) + surface rows. v1 string shape untouched. Sort `name|size`, `order`, `min/max_bytes` (+400 on `min>max`, unknown rows never match a bound), deterministic `(owner,name)` tiebreak, `[]`-never-null, null-vs-0 preserved end to end. One catalog GET per query (asserted in harness). No pagination — matches v1 owner listing; fine at owner scope. **R1 B3 — PASS.** Frozen list amended in-table + Decisions (`14_extensibility.md`), proto append-only field 3 (`RepoCatalog.entries`, 6+ reserved for #247), hand codec + `catalog_entry`/`catalog_size` golden fixtures (byte-identical re-encode). Legacy decode → nil entries; future-field skip — both tested (`catalog248_test.go:38-57`). `repos` field 1 retained verbatim for Rust readers. **Should-fix S1–S5 — all addressed.** LiveBytes/IdxSize delta documented (header, `types.go`, 07 §8, E16); AddPack lower-bound approximation documented + tested (`size248_test.go:65-70`); `.rev/.bitmap/.commit-graph` + bundles + LFS exclusions documented; null = unknown / 0 = verified-empty (incl. verified-empty entry in `catalog_size` fixture); push→compact→superseded-leaves + annotate-no-op + empty→0 + tie determinism covered. ## Small fixes pushed (`454955b` → `1bf1c07`, re-tested) - E16 harness measured setup PUTs: seed went through the counting wrapper, so the log printed `puts=7/25` while E16 claims 4/13. Now seeds unwrapped and **asserts** exact fold cost (`gets==2n+1`, `puts==n+1`), steady-state second pass (`folded==0`, `puts==1` catalog CAS only), zero LIST/DELETE/HEAD across both passes (`roundtrips248_test.go:81-130`). E16 table now machine-checked. - Removed dead `_ = asc` (`sizecatalog.go:509`); fixed `UnmarshalRepoCatalog` doc ref §2.7 → §2.2 (`codec.go`). ## Test results (scratch worktree, `-race`) `sizecatalog` 97.8% / `api` 95.1% / `maintain` 96.3% / `store/proto` 98.5% / `wal` 95.4% / `store` 95.0% — all ≥ 95%, exact match to PR claims. `gofmt`/`vet` clean. Store contract suite green. `node --test web/test/unit/*.test.js`: **500/500** (two unrelated files fail only without `node_modules` in a fresh worktree — environmental, untouched by this PR). Law 8 clean (leaf package; test-only `wal→sizecatalog` edge, no cycle); no new Go/npm deps (`go.mod`, manifests untouched); `### Concurrency` subsections present. ## Non-blocking follow-ups (for author, not merge-gates) - `WriteCatalog` reimplements `store.CasUpdate` (02 §2.7) inline — consider reuse; on conflict it returns the raw 412, not `ErrRetriesExhausted`. - `foldOne` uses `PutOverwrite` while docs call the sidecar "CAS'd" — reconcile per law 12 once the push path owns primary writes. - No catalog GC for deleted repos: rows accumulate (harmless — `RowsForOwner` projects registry names only — but unbounded). Suggest pruning rows outside the full id set on complete passes. ## Recommendation **Blocked: push-path durable sidecar write per R1 B1** (precise instructions above). Everything else in the PR — codec, endpoint, sweep-as-backfill, docs, evidence, coverage — is merge-ready and should be kept as-is.

crueber referenced this issue from a commit

2026-09-09 22:02:33 +00:00

Review #258: publish-path parallel sidecar PUT on PUSH/COMPACT (#248 R1 B1)

crueber commented

2026-09-09 22:02:38 +00:00

Author

Owner

Rework for review #258 pushed to origin/feat/issue-248 (cd41b39, fast-forward, no force).

Blocking item fixed per the unblock instructions:

Publish path (internal/wal/publish.go runBatch steps 6+7) now computes SizeOf over the in-hand live set and PUTs repos///meta/stats.json (same v1 shape) whenever PUSH/COMPACT changes the live set (ref-only/settings skip; annotate bypasses the ladder), in parallel with the manifest CAS: +1 total op, +0 sequential trips. Best-effort WARN-only (manifest CAS stays the only commit point, law 4).
Sweep kept as backfill/repair (PUT-if-changed converges); law 8 kept clean (wal-side mirror helpers, no production import of feature packages; test-only wal->sizecatalog edge).
Durability test (internal/wal/size248_durable_test.go): push -> sidecar present, compact -> superseded packs leave the sum, ref-only/settings -> no write, failed PUT -> push still commits; helpers cross-checked against sizecatalog (incl. saturation).
Sim counts: push-budget fence sanctions the sidecar PUT (blind overwrite only, <= 4, measured cold 11/warm 10 = E15-era 10/9 +1 each); E16 rewritten (push +1 PUT/+0 sequential); 15_testing push budget <= 6 with R1 justification (sequential budgets untouched).
02/10/14 decision entries corrected (they claimed R1 B1 authority for sweep-only — now state push-parallel + sweep-backfill with the correction noted); 05 ladder, sizecatalog header, keys.go, E15 push rows updated. Endpoint, catalog fold, codec, fixtures untouched.

Verification: gofmt/vet clean; -race green for wal/sizecatalog/store(+proto+contract)/maintain/api/cmd-walhub; full go test -short ./... green (one e2e setup-shell failure was a missing web/dist in the scratch worktree only — green after building/copying dist, unrelated to this change); coverage wal 95.4% / sizecatalog 97.8% / api 95.1% / store 95.0%+; no new Go/npm deps. No merge, no new PR — awaiting re-review.

Rework for review #258 pushed to origin/feat/issue-248 (cd41b39, fast-forward, no force). Blocking item fixed per the unblock instructions: - Publish path (internal/wal/publish.go runBatch steps 6+7) now computes SizeOf over the in-hand live set and PUTs repos/<o>/<r>/meta/stats.json (same v1 shape) whenever PUSH/COMPACT changes the live set (ref-only/settings skip; annotate bypasses the ladder), in parallel with the manifest CAS: +1 total op, +0 sequential trips. Best-effort WARN-only (manifest CAS stays the only commit point, law 4). - Sweep kept as backfill/repair (PUT-if-changed converges); law 8 kept clean (wal-side mirror helpers, no production import of feature packages; test-only wal->sizecatalog edge). - Durability test (internal/wal/size248_durable_test.go): push -> sidecar present, compact -> superseded packs leave the sum, ref-only/settings -> no write, failed PUT -> push still commits; helpers cross-checked against sizecatalog (incl. saturation). - Sim counts: push-budget fence sanctions the sidecar PUT (blind overwrite only, <= 4, measured cold 11/warm 10 = E15-era 10/9 +1 each); E16 rewritten (push +1 PUT/+0 sequential); 15_testing push budget <= 6 with R1 justification (sequential budgets untouched). - 02/10/14 decision entries corrected (they claimed R1 B1 authority for sweep-only — now state push-parallel + sweep-backfill with the correction noted); 05 ladder, sizecatalog header, keys.go, E15 push rows updated. Endpoint, catalog fold, codec, fixtures untouched. Verification: gofmt/vet clean; -race green for wal/sizecatalog/store(+proto+contract)/maintain/api/cmd-walhub; full go test -short ./... green (one e2e setup-shell failure was a missing web/dist in the scratch worktree only — green after building/copying dist, unrelated to this change); coverage wal 95.4% / sizecatalog 97.8% / api 95.1% / store 95.0%+; no new Go/npm deps. No merge, no new PR — awaiting re-review.

crueber commented

2026-09-09 22:05:47 +00:00

Author

Owner

PASS items (file:line on new tip cd41b39):

Truly parallel +0 sequential: internal/wal/publish.go:384-396 — sidecar goroutine (PutOverwrite meta/stats.json) starts before casManifest (L395) and joins at L396; derivation at L366-370 is pure arithmetic, no I/O. Parallel window confirmed; no sequential-after ordering.
Best-effort WARN-only: L407-415 — statsErr only logWarnf on committed path; push answers ok. On casErr path (L397-406) statsErr is discarded and the outcome follows the CAS only. Failed PUT can never fail the push. (Note: the join at L396 means a slow sidecar adds tail latency max(CAS,PUT) before reply/retry — inherent to the durability claim 'returned push carries its sidecar'; bounded by ctx, no leak. Acceptable, not a blocker.)
Blind overwrite + sweep convergence SOUND: CAS serializes winners; loser on 412-retries (L398-402 continue) recomputes statsBody from fresh base and overwrites, so last-writer == CAS-winner. Crash-between-PUT-and-CAS leaves an ahead-sidecar; next pack-changing publish overwrites, else sweep PUT-if-changed (internal/sizecatalog/sizecatalog.go:403) converges. Sidecar stays a rebuildable hint; manifest CAS the only commit point (law 4). Future note for #247 (not blocking): blind overwrite will clobber activity fields on the same file unless the publish write merges/preserves them — handle when #247 lands.
Ref-only/settings skip correct: batchChangesLiveSet (publish.go:453-463) true only on PUSH/COMPACT; runBatch kinds (L306-321) map Settings->SETTINGS, Compact->COMPACT, Pack->PUSH, default->REF_UPDATE; AnnotatePack (L1059-1110) bypasses runBatch (manifest-only CAS, flags explicitly OUT of size semantic). No stale size left (live set untouched); head_seq lags until next pack-write/sweep (sweep treats head mismatch as changed, sizecatalog.go:403) — bounded, self-healing.
Failed-PUT test genuine: size248_durable_test.go TestPublishSidecarBestEffortOnWriteFailure — failStatsStore fails only *stats.json PUTs, delegates rest; WARN line observed, res.Seq!=0, manifest head==res.Seq, sidecar absent. Push commits despite sidecar failure.
Durability test genuine: TestPublishSidecarDurablePushThenCompact asserts sidecar bytes via direct GET after Publish and PublishCompact with NO sweep run (push->present, compact->superseded pack leaves sum, cross-checked vs statsSizeOf + sizecatalog.ManifestSize). TestPublishSidecarSkippedForRefOnly asserts no bytes after ref-only+settings. TestStatsHelpersAgreeWithSizecatalog pins law-8 mirror (statsSizeOf==SizeOf, saturation, encode round-trip via DecodeStats).
Law 8 respected: publish.go duplicates the sum locally with comment (L465-470) instead of importing sizecatalog; test cross-checks agreement.
Budgets: 15_testing.md push ops <=6 (+1 total, +0 sequential, R1 B1) justified; E16 cold 11/warm 10 measured matches fence run below (E15-era 10/9 +1). push_budget_test second sanctioned touch bounded (statsPuts<=4, put-only, never a read — isSizeSidecarWrite). Sequential budgets untouched (warm refs/cold refs/checkpoint rows unchanged). fault_test.go:1069 '<=5' is FaultStore stats unit counting, not the publish path — correctly untouched.
412-ahead-write note (publish.go:373-383) sound per walk above.
Doc corrections accurate: 02/10/14 now state publish-path primary + sweep backfill with 2026-09-09 correction notes; keys.go documents overwrite+backfill; E16 corrected with measured numbers.

Verification (scratch worktree /tmp/pr258b @ cd41b39, since removed... [see below]):

go test -race ./internal/wal/... ./internal/store/... : ok (wal 5.9s, rw, store, fault, proto all ok)
./internal/sizecatalog + ./internal/wal sidecar tests: PASS
cmd/walhub TestPushFastPathZeroCollabRoundTrips (with web/dist copied from main worktree read-only): PASS — cold 11 / warm 10, 3 collab touches per push
coverage: wal 95.5%, wal/rw 100%, store 95.0%, fault 100%, proto 98.5%, sizecatalog 97.8% — >=95% gate holds
go vet clean; gofmt clean.

MERGE RECOMMENDATION: ready to merge.

Re-review PR #258 delta 1bf1c07..cd41b39 (blocking item: push-path durable sidecar write). File list confirmed in-scope (11 files: internal/wal/publish.go, internal/wal/size248_durable_test.go, internal/sizecatalog/sizecatalog.go, internal/store/keys.go, cmd/walhub/push_budget_test.go + docs 02/05/10/14/15/EVIDENCE). Nothing else changed unexpectedly. PASS items (file:line on new tip cd41b39): - Truly parallel +0 sequential: internal/wal/publish.go:384-396 — sidecar goroutine (PutOverwrite meta/stats.json) starts before casManifest (L395) and joins at L396; derivation at L366-370 is pure arithmetic, no I/O. Parallel window confirmed; no sequential-after ordering. - Best-effort WARN-only: L407-415 — statsErr only logWarnf on committed path; push answers ok. On casErr path (L397-406) statsErr is discarded and the outcome follows the CAS only. Failed PUT can never fail the push. (Note: the join at L396 means a slow sidecar adds tail latency max(CAS,PUT) before reply/retry — inherent to the durability claim 'returned push carries its sidecar'; bounded by ctx, no leak. Acceptable, not a blocker.) - Blind overwrite + sweep convergence SOUND: CAS serializes winners; loser on 412-retries (L398-402 continue) recomputes statsBody from fresh base and overwrites, so last-writer == CAS-winner. Crash-between-PUT-and-CAS leaves an ahead-sidecar; next pack-changing publish overwrites, else sweep PUT-if-changed (internal/sizecatalog/sizecatalog.go:403) converges. Sidecar stays a rebuildable hint; manifest CAS the only commit point (law 4). Future note for #247 (not blocking): blind overwrite will clobber activity fields on the same file unless the publish write merges/preserves them — handle when #247 lands. - Ref-only/settings skip correct: batchChangesLiveSet (publish.go:453-463) true only on PUSH/COMPACT; runBatch kinds (L306-321) map Settings->SETTINGS, Compact->COMPACT, Pack->PUSH, default->REF_UPDATE; AnnotatePack (L1059-1110) bypasses runBatch (manifest-only CAS, flags explicitly OUT of size semantic). No stale size left (live set untouched); head_seq lags until next pack-write/sweep (sweep treats head mismatch as changed, sizecatalog.go:403) — bounded, self-healing. - Failed-PUT test genuine: size248_durable_test.go TestPublishSidecarBestEffortOnWriteFailure — failStatsStore fails only *stats.json PUTs, delegates rest; WARN line observed, res.Seq!=0, manifest head==res.Seq, sidecar absent. Push commits despite sidecar failure. - Durability test genuine: TestPublishSidecarDurablePushThenCompact asserts sidecar bytes via direct GET after Publish and PublishCompact with NO sweep run (push->present, compact->superseded pack leaves sum, cross-checked vs statsSizeOf + sizecatalog.ManifestSize). TestPublishSidecarSkippedForRefOnly asserts no bytes after ref-only+settings. TestStatsHelpersAgreeWithSizecatalog pins law-8 mirror (statsSizeOf==SizeOf, saturation, encode round-trip via DecodeStats). - Law 8 respected: publish.go duplicates the sum locally with comment (L465-470) instead of importing sizecatalog; test cross-checks agreement. - Budgets: 15_testing.md push ops <=6 (+1 total, +0 sequential, R1 B1) justified; E16 cold 11/warm 10 measured matches fence run below (E15-era 10/9 +1). push_budget_test second sanctioned touch bounded (statsPuts<=4, put-only, never a read — isSizeSidecarWrite). Sequential budgets untouched (warm refs/cold refs/checkpoint rows unchanged). fault_test.go:1069 '<=5' is FaultStore stats unit counting, not the publish path — correctly untouched. - 412-ahead-write note (publish.go:373-383) sound per walk above. - Doc corrections accurate: 02/10/14 now state publish-path primary + sweep backfill with 2026-09-09 correction notes; keys.go documents overwrite+backfill; E16 corrected with measured numbers. Verification (scratch worktree /tmp/pr258b @ cd41b39, since removed... [see below]): - go test -race ./internal/wal/... ./internal/store/... : ok (wal 5.9s, rw, store, fault, proto all ok) - ./internal/sizecatalog + ./internal/wal sidecar tests: PASS - cmd/walhub TestPushFastPathZeroCollabRoundTrips (with web/dist copied from main worktree read-only): PASS — cold 11 / warm 10, 3 collab touches per push - coverage: wal 95.5%, wal/rw 100%, store 95.0%, fault 100%, proto 98.5%, sizecatalog 97.8% — >=95% gate holds - go vet clean; gofmt clean. MERGE RECOMMENDATION: ready to merge.

crueber closed this issue

2026-09-09 22:06:16 +00:00

crueber commented

2026-09-09 22:06:16 +00:00

Author

Owner

Implemented in PR #258 incl. push-path durable sidecar rework (parallel +0 sequential, blind-overwrite convergence verified; all gates green), merged. Closing.

crueber referenced this issue from a commit

2026-09-09 22:06:16 +00:00

Merge pull request 'Feature #248: server-side repo size tracking' (#258) from feat/issue-248 into main

crueber referenced this issue

2026-09-09 23:06:14 +00:00

Feature #247: server-side last-commit tracking #260

crueber referenced this issue

2026-09-09 23:06:21 +00:00

Track last commit (sha + date) per repo server-side; use it to order the explore page by most recent commit #247

crueber referenced this issue

2026-09-09 23:22:44 +00:00

Track last commit (sha + date) per repo server-side; use it to order the explore page by most recent commit #247