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

Closed
opened 2026-09-09 17:45:51 +00:00 by crueber · 5 comments
Owner

What's requested

Keep track of each repository's last commit — which commit (sha) and when — as queryable server-side state, and use it on the explore page (/explore) to order repositories by most recent commit instead of today's name-based proxy.

Current state (code evidence — this is a documented gap)

  • Client-side stamp exists but costs one GET per repo. <ActivityStamp> (web/src/components/ActivityStamp.jsx, issue #142) renders "active " per repo row by fetching GET …/commits?n=1 per repo (shared activity:{o}/{r} cache, 30 s TTL, #117 caps bound it at ~500 GETs cold). The component header explicitly documents the rejected alternative: "the summary (GET …/api, §9.1) carries NO date field at all … true reverse-chronological order wants a server-side shape, not client logic."
  • Ordering is a documented honest proxy, not real ordering. web/src/lib/owners.js:17-33 newestFirst() is reverse-lexicographic on the repo NAME — its own doc comment says: "the listing path exposes NO creation timestamp … if the backend ever carries creation times, this is the single function to replace." The explore page (web/src/pages/Owners.jsx) therefore does NOT order by recent commit today, despite rendering per-row stamps that look like it does.
  • The per-repo data needed already exists in the manifest — it just isn't aggregated anywhere queryable:
    • Manifest.Packs (internal/store/proto/types.go:110-122) carries every live pack's Seq and the WAL LogEntry.CreatedAt (:132) gives wall-clock time per entry.
    • The last commit's sha is derivable from the manifest's HEAD ref snapshot (RefSnapshot/Ref in the WAL state), and its date from the head commit object.
    • Manifest.UpdatedAt is manifest-write time (push time, not commit time — too coarse and semantically wrong per the #142 decision).
  • Listing path is manifest-gated already. repoRegistry.Owners/Repos (cmd/walhub/serve.go:498-547) already reads manifest.pb per repo (parallel, limit 8) just to decide existence — so per-manifest metadata rides the same reads; the missing piece is a durable, sorted index, not more scans.
  • RepoCatalog exists but is name-only. proto.RepoCatalog (meta/repos.pb, internal/store/keys.go:23, types.go:254-258) holds Repos []string — no timestamps, and it's "optional, not required for correctness".

Proposed design

  1. Record last-commit state at publish time (the moment the WAL entry with commits lands): extend the per-repo manifest (or a small sidecar, e.g. repos/<o>/<r>/meta/activity.json following the access.json sidecar precedent) with last_commit_sha, last_commit_time (commit date, matching #142's semantic: commit_date first, author_date fallback), and last_push_at (push time — cheap and useful even when a push adds no commits; #142 documents that a branch-delete/tag-only push moves nothing). Update is O(1) per push — no scan.
    • Semantic rule from #142 carries over: "last commit" = tip of HEAD (default branch), not any ref.
  2. Aggregate into a queryable index. Extend RepoCatalog (or add meta/activity.pb) to a per-repo {owner, name, last_commit_sha, last_commit_time, last_push_at} row, updated on the same publish path and rebuilt/swept by the maintainer loop (which already walks repos). This is the object-store-analog of an index: one GET serves the full sorted list for the explore page.
    • Keep the catalog's existing contract: optional, rebuildable from manifests, not required for correctness — a missing/stale row falls back to the per-repo value or "unknown".
  3. API. GET /api/v1/owners/{owner}/repos (and a new GET /api/v1/repos?sort=activity or explore-shaped endpoint) returns rows with the activity fields (never plain string lists — this is a wire-shape change to the §8 endpoints; template the change through all three route twins per the NonRepo pattern in internal/api/routes.go:46-51). Include Cache-Control: SWR per the existing conventions.
  4. Frontend. Replace newestFirst() (lib/owners.js:30) with ordering on the server-provided last_commit_time (its own doc comment names it "the single function to replace"). Explore page rows then sort truly newest-first, ActivityStamp reads the same field instead of a per-row commits?n=1 GET (drops the ~500-GET cold-cache worst case), and Repos.jsx (/:owner) gets the same ordering for free.
  5. Backfill. Existing repos get their activity rows on the maintainer's next pass (read manifest → head sha → commit date) rather than requiring a migration; until then the API reports null and the UI keeps today's behavior.

Acceptance criteria

  • A push that changes HEAD updates the repo's last_commit_sha/last_commit_time (commit-date semantics per #142: commit_date preferred, author_date fallback); a tag-only/branch-delete push updates last_push_at but not the commit fields.
  • The aggregate listing endpoint returns activity fields and supports ordering by them; explore page (/explore) and /:owner order repos by most recent commit, newest first.
  • <ActivityStamp> sources the stamp from the listing data (no per-row commits?n=1 fetch on the explore/owner pages); empty repos show "no commits yet".
  • No scans on hot paths: the per-push update is O(1); listing reads one aggregate object; the maintainer backfill is bounded and resumable.
  • Catalog stays optional/rebuildable: deleting it doesn't break correctness; stale rows are detected and rebuilt by the maintainer sweep.
  • Wire change goes through all three route twins (template/api-browser/services) per the law-12 lane rule.
  • Headless unit tests for the ordering function (ties → deterministic secondary key) and a publish-path test proving the activity update rides the existing WAL commit (no second store round trip beyond the index write).
## What's requested Keep track of each repository's last commit — which commit (sha) and when — as queryable server-side state, and use it on the explore page (`/explore`) to order repositories by most recent commit instead of today's name-based proxy. ## Current state (code evidence — this is a documented gap) - **Client-side stamp exists but costs one GET per repo.** `<ActivityStamp>` (`web/src/components/ActivityStamp.jsx`, issue #142) renders "active <date>" per repo row by fetching `GET …/commits?n=1` per repo (shared `activity:{o}/{r}` cache, 30 s TTL, #117 caps bound it at ~500 GETs cold). The component header explicitly documents the rejected alternative: *"the summary (`GET …/api`, §9.1) carries NO date field at all … true reverse-chronological order wants a server-side shape, not client logic."* - **Ordering is a documented honest proxy, not real ordering.** `web/src/lib/owners.js:17-33` `newestFirst()` is reverse-lexicographic on the repo NAME — its own doc comment says: *"the listing path exposes NO creation timestamp … if the backend ever carries creation times, this is the single function to replace."* The explore page (`web/src/pages/Owners.jsx`) therefore does NOT order by recent commit today, despite rendering per-row stamps that look like it does. - **The per-repo data needed already exists in the manifest** — it just isn't aggregated anywhere queryable: - `Manifest.Packs` (`internal/store/proto/types.go:110-122`) carries every live pack's `Seq` and the WAL `LogEntry.CreatedAt` (:132) gives wall-clock time per entry. - The last commit's sha is derivable from the manifest's HEAD ref snapshot (`RefSnapshot`/`Ref` in the WAL state), and its date from the head commit object. - `Manifest.UpdatedAt` is manifest-write time (push time, not commit time — too coarse and semantically wrong per the #142 decision). - **Listing path is manifest-gated already.** `repoRegistry.Owners/Repos` (`cmd/walhub/serve.go:498-547`) already reads `manifest.pb` per repo (parallel, limit 8) just to decide existence — so per-manifest metadata rides the same reads; the missing piece is a durable, sorted index, not more scans. - **`RepoCatalog` exists but is name-only.** `proto.RepoCatalog` (`meta/repos.pb`, `internal/store/keys.go:23`, types.go:254-258) holds `Repos []string` — no timestamps, and it's "optional, not required for correctness". ## Proposed design 1. **Record last-commit state at publish time** (the moment the WAL entry with commits lands): extend the per-repo manifest (or a small sidecar, e.g. `repos/<o>/<r>/meta/activity.json` following the `access.json` sidecar precedent) with `last_commit_sha`, `last_commit_time` (commit date, matching #142's semantic: `commit_date` first, `author_date` fallback), and `last_push_at` (push time — cheap and useful even when a push adds no commits; #142 documents that a branch-delete/tag-only push moves nothing). Update is O(1) per push — no scan. - Semantic rule from #142 carries over: "last commit" = tip of HEAD (default branch), not any ref. 2. **Aggregate into a queryable index.** Extend `RepoCatalog` (or add `meta/activity.pb`) to a per-repo `{owner, name, last_commit_sha, last_commit_time, last_push_at}` row, updated on the same publish path and rebuilt/swept by the maintainer loop (which already walks repos). This is the object-store-analog of an index: one GET serves the full sorted list for the explore page. - Keep the catalog's existing contract: optional, rebuildable from manifests, not required for correctness — a missing/stale row falls back to the per-repo value or "unknown". 3. **API.** `GET /api/v1/owners/{owner}/repos` (and a new `GET /api/v1/repos?sort=activity` or explore-shaped endpoint) returns rows with the activity fields (never plain string lists — this is a wire-shape change to the §8 endpoints; template the change through all three route twins per the NonRepo pattern in `internal/api/routes.go:46-51`). Include `Cache-Control: SWR` per the existing conventions. 4. **Frontend.** Replace `newestFirst()` (`lib/owners.js:30`) with ordering on the server-provided `last_commit_time` (its own doc comment names it "the single function to replace"). Explore page rows then sort truly newest-first, `ActivityStamp` reads the same field instead of a per-row `commits?n=1` GET (drops the ~500-GET cold-cache worst case), and `Repos.jsx` (`/:owner`) gets the same ordering for free. 5. **Backfill.** Existing repos get their activity rows on the maintainer's next pass (read manifest → head sha → commit date) rather than requiring a migration; until then the API reports null and the UI keeps today's behavior. ## Acceptance criteria - [ ] A push that changes HEAD updates the repo's `last_commit_sha`/`last_commit_time` (commit-date semantics per #142: `commit_date` preferred, `author_date` fallback); a tag-only/branch-delete push updates `last_push_at` but not the commit fields. - [ ] The aggregate listing endpoint returns activity fields and supports ordering by them; explore page (`/explore`) and `/:owner` order repos by most recent commit, newest first. - [ ] `<ActivityStamp>` sources the stamp from the listing data (no per-row `commits?n=1` fetch on the explore/owner pages); empty repos show "no commits yet". - [ ] No scans on hot paths: the per-push update is O(1); listing reads one aggregate object; the maintainer backfill is bounded and resumable. - [ ] Catalog stays optional/rebuildable: deleting it doesn't break correctness; stale rows are detected and rebuilt by the maintainer sweep. - [ ] Wire change goes through all three route twins (template/api-browser/services) per the law-12 lane rule. - [ ] Headless unit tests for the ordering function (ties → deterministic secondary key) and a publish-path test proving the activity update rides the existing WAL commit (no second store round trip beyond the index write).
Author
Owner

Review: #247 server-side last-commit tracking (REVIEW ONLY, no code)

Verdict: proceed-with-fixes — the direction is sound and most code evidence checks out, but 5 blocking items must be resolved before implementation (hot-path contention, breaking wire change, unspecified derivation call site, missing frozen-list amendment, inaccurate manifest claim).

What I verified against code (correct)

  • ActivityStamp cost claim: CONFIRMED. web/src/components/ActivityStamp.jsx:1-33 documents one GET commits?n=1 per row, shared activity key, 30 s TTL, #117 caps.
  • newestFirst proxy claim: CONFIRMED. web/src/lib/owners.js:17-33 is reverse-lexicographic on name with an honest doc comment naming itself the single function to replace.
  • Manifest.UpdatedAt is push time: CONFIRMED (internal/wal/publish.go:706 sets UpdatedAt=now in buildNextManifest; bind_wal.go:554-557 surfaces it as Manifest.LastPush). Right to reject it as commit-time.
  • LogEntry.CreatedAt wall-clock: CONFIRMED (publish.go:294-304, monotonic guard at 260-268).
  • Listing gate reads manifest per repo, parallel limit 8: CONFIRMED (cmd/walhub/serve.go:559-587 liveRepos).
  • RepoCatalog name-only + optional: CONFIRMED (types.go:248-252, codec.go:2617-2691 fields 1+2 only). Decoder skips unknown fields (codec.go:2681-2684), so additive field 3+ is wire-compatible. No writer or reader of the catalog exists anywhere in Go code — it is a dead shape today; this plan revives it.
  • Overview heavyweight/no-store: CONFIRMED (internal/api/overview.go:11,61; bind_wal.go:542-553).
  • Three route twins: CONFIRMED (internal/api/routes.go:49-51: /api/v1 + /api-browser/v1 + /services/api). The "three twins" phrasing is accurate.
  • Roles empty = all (config.go:61), so zero-config runs the maintainer — backfill via maintainer works on first boot. Multi-role fleets must be covered in the doc.

BLOCKING

B1 — Synchronous bucket-root catalog CAS on the publish hot path is a cross-repo contention funnel and breaks law 6.
§2 has the catalog "updated on the same publish path". The catalog is ONE bucket-root key; every push to every repo would read-modify-write it. Concurrent pushes to unrelated repos then 412 against each other, and the casUpdate ladder (02 §2.7: immediate re-read, counted retries, ErrRetriesExhausted) turns cross-repo concurrency into spurious contention with a failure backstop designed for same-key races, not global ones. It also adds ≥1 sequential store round trip to the push ≤5 budget (law 6) — the acceptance criterion "no second store round trip beyond the index write" already concedes the regression instead of eliminating it.
Required fix: the push path writes ONLY the per-repo sidecar (repo-scoped key, no cross-repo contention), and the aggregate catalog is refreshed OFF the hot path (maintainer sweep / background fold; staleness = documented pass interval). Alternatively per-owner sharded catalogs. Either way §2 needs a rewrite and the sim budget assertion (15_testing.md) must show push +0.

B2 — Changing ownerRepos from ["a","b"] to rows of objects is a retyping breaking change, not an additive field.
internal/api/discovery.go:131-142 returns a plain string list today. 14.12 allows new OPTIONAL fields on existing JSON objects; array-of-string → array-of-object is a retype and "requires a NEW prefix served alongside v1; v1 is never edited in place". The plan must name the new endpoint (new path or v2), add it to discovery endpoints[] (pick a rule — recent feature waves chose no-discovery entries, document which rule this follows), triple-twins, and SDK. Same for the proposed top-level GET /api/v1/repos — 14.3 requires a spec note for new top-level families.

B3 — Derivation call site and failure semantics are unspecified, and the git read is hand-waved.
Commit date is NOT derivable from manifest bytes: manifest.pb carries NO refs (see B5). The exact call site in publish.go runBatch must be named (recommendation: derive once, outside the CAS attempt loop, or best-effort after commitLocal success), with the rule: derivation failure NEVER fails the push — degrade to null and let the maintainer heal it (law 4: the push ACK covers git data; the rollup is explicitly optional/rebuildable, so ACK-before-rollup is fine — but say so). A git subprocess per push also needs its exact argv added to 04_git.md per law 2 (precedent exists: 07_api.md:529,563 %aI/%cI formats — name which one the publish path uses).

B4 — Missing 14 §14.11 rule-2 frozen-list amendment + proto-compat work.
The sidecar family (repos///meta/activity.json or chosen name) and the aggregate (extended RepoCatalog field 3+ as a new RepoActivity message, or new bucket-root meta/activity.pb key — bucket-root keys are also frozen layout) MUST join the frozen overwritable list in the same change, with codec + golden fixtures (TestGoldenWireCompat). Also rule out "manifest-inline" explicitly: Manifest is the linearization point every replica parses; keep rollups out of it — sidecar + catalog only.

B5 — "Last commit sha derivable from the manifest HEAD ref snapshot" is inaccurate.
There is no ref snapshot in manifest.pb. The tip comes from the ref layer (post-sync view / checkpoint refs.pb / log txns) and the date from the commit object. Cold derivation therefore costs a refs sync (log segment GETs) plus a git read — the backfill unit must budget this per repo, and state its enumeration source (note: maintain engine Repos() in bind_wal.go:29 is the LOCAL in-memory list, not bucket enumeration — a fleet-wide backfill needs a bucket LIST, off-hot-path-legal but must be specified, bounded, and resumable).

Should-fix

  • S1: "Metadata rides the same reads" is wrong — liveRepos uses Head (existence only), not Get. Harvesting manifest content needs GETs; the catalog remains the right answer, fix the prose.
  • S2: Date semantics are otherwise good (commit_date-first per #142/activity.test.js, HEAD-tip-only, tag/branch-delete → last_push_at only, ties → deterministic secondary key). Add: client timestamps are untrusted (future/ancient) — sorting stays total via the secondary key; RFC 3339; null = unknown/unbackfilled (UI keeps current behavior) vs "no commits yet" for truly empty.
  • S3: Define stale-row detection concretely (catalog as_of seq vs sidecar/manifest revision), cache class (SWR per conventions), conditional-GET revalidation (law 4).
  • S4: Pagination interplay with #117 caps (50×10) — specify slice-after-server-sort.
  • S5: JSON conventions per 14.12: uint64s as strings, arrays [] never null.

Nit

  • N1: ownerRepos 200-[]-for-unknown-owner behavior (§8) must be preserved by the new endpoint.
# Review: #247 server-side last-commit tracking (REVIEW ONLY, no code) Verdict: **proceed-with-fixes** — the direction is sound and most code evidence checks out, but 5 blocking items must be resolved before implementation (hot-path contention, breaking wire change, unspecified derivation call site, missing frozen-list amendment, inaccurate manifest claim). ## What I verified against code (correct) - ActivityStamp cost claim: CONFIRMED. web/src/components/ActivityStamp.jsx:1-33 documents one GET commits?n=1 per row, shared activity key, 30 s TTL, #117 caps. - newestFirst proxy claim: CONFIRMED. web/src/lib/owners.js:17-33 is reverse-lexicographic on name with an honest doc comment naming itself the single function to replace. - Manifest.UpdatedAt is push time: CONFIRMED (internal/wal/publish.go:706 sets UpdatedAt=now in buildNextManifest; bind_wal.go:554-557 surfaces it as Manifest.LastPush). Right to reject it as commit-time. - LogEntry.CreatedAt wall-clock: CONFIRMED (publish.go:294-304, monotonic guard at 260-268). - Listing gate reads manifest per repo, parallel limit 8: CONFIRMED (cmd/walhub/serve.go:559-587 liveRepos). - RepoCatalog name-only + optional: CONFIRMED (types.go:248-252, codec.go:2617-2691 fields 1+2 only). Decoder skips unknown fields (codec.go:2681-2684), so additive field 3+ is wire-compatible. No writer or reader of the catalog exists anywhere in Go code — it is a dead shape today; this plan revives it. - Overview heavyweight/no-store: CONFIRMED (internal/api/overview.go:11,61; bind_wal.go:542-553). - Three route twins: CONFIRMED (internal/api/routes.go:49-51: /api/v1 + /api-browser/v1 + /services/api). The "three twins" phrasing is accurate. - Roles empty = all (config.go:61), so zero-config runs the maintainer — backfill via maintainer works on first boot. Multi-role fleets must be covered in the doc. ## BLOCKING **B1 — Synchronous bucket-root catalog CAS on the publish hot path is a cross-repo contention funnel and breaks law 6.** §2 has the catalog "updated on the same publish path". The catalog is ONE bucket-root key; every push to every repo would read-modify-write it. Concurrent pushes to unrelated repos then 412 against each other, and the casUpdate ladder (02 §2.7: immediate re-read, counted retries, ErrRetriesExhausted) turns cross-repo concurrency into spurious contention with a failure backstop designed for same-key races, not global ones. It also adds ≥1 sequential store round trip to the push ≤5 budget (law 6) — the acceptance criterion "no second store round trip beyond the index write" already concedes the regression instead of eliminating it. Required fix: the push path writes ONLY the per-repo sidecar (repo-scoped key, no cross-repo contention), and the aggregate catalog is refreshed OFF the hot path (maintainer sweep / background fold; staleness = documented pass interval). Alternatively per-owner sharded catalogs. Either way §2 needs a rewrite and the sim budget assertion (15_testing.md) must show push +0. **B2 — Changing ownerRepos from ["a","b"] to rows of objects is a retyping breaking change, not an additive field.** internal/api/discovery.go:131-142 returns a plain string list today. 14.12 allows new OPTIONAL fields on existing JSON objects; array-of-string → array-of-object is a retype and "requires a NEW prefix served alongside v1; v1 is never edited in place". The plan must name the new endpoint (new path or v2), add it to discovery endpoints[] (pick a rule — recent feature waves chose no-discovery entries, document which rule this follows), triple-twins, and SDK. Same for the proposed top-level GET /api/v1/repos — 14.3 requires a spec note for new top-level families. **B3 — Derivation call site and failure semantics are unspecified, and the git read is hand-waved.** Commit date is NOT derivable from manifest bytes: manifest.pb carries NO refs (see B5). The exact call site in publish.go runBatch must be named (recommendation: derive once, outside the CAS attempt loop, or best-effort after commitLocal success), with the rule: derivation failure NEVER fails the push — degrade to null and let the maintainer heal it (law 4: the push ACK covers git data; the rollup is explicitly optional/rebuildable, so ACK-before-rollup is fine — but say so). A git subprocess per push also needs its exact argv added to 04_git.md per law 2 (precedent exists: 07_api.md:529,563 %aI/%cI formats — name which one the publish path uses). **B4 — Missing 14 §14.11 rule-2 frozen-list amendment + proto-compat work.** The sidecar family (repos/<o>/<r>/meta/activity.json or chosen name) and the aggregate (extended RepoCatalog field 3+ as a new RepoActivity message, or new bucket-root meta/activity.pb key — bucket-root keys are also frozen layout) MUST join the frozen overwritable list in the same change, with codec + golden fixtures (TestGoldenWireCompat). Also rule out "manifest-inline" explicitly: Manifest is the linearization point every replica parses; keep rollups out of it — sidecar + catalog only. **B5 — "Last commit sha derivable from the manifest HEAD ref snapshot" is inaccurate.** There is no ref snapshot in manifest.pb. The tip comes from the ref layer (post-sync view / checkpoint refs.pb / log txns) and the date from the commit object. Cold derivation therefore costs a refs sync (log segment GETs) plus a git read — the backfill unit must budget this per repo, and state its enumeration source (note: maintain engine Repos() in bind_wal.go:29 is the LOCAL in-memory list, not bucket enumeration — a fleet-wide backfill needs a bucket LIST, off-hot-path-legal but must be specified, bounded, and resumable). ## Should-fix - S1: "Metadata rides the same reads" is wrong — liveRepos uses Head (existence only), not Get. Harvesting manifest content needs GETs; the catalog remains the right answer, fix the prose. - S2: Date semantics are otherwise good (commit_date-first per #142/activity.test.js, HEAD-tip-only, tag/branch-delete → last_push_at only, ties → deterministic secondary key). Add: client timestamps are untrusted (future/ancient) — sorting stays total via the secondary key; RFC 3339; null = unknown/unbackfilled (UI keeps current behavior) vs "no commits yet" for truly empty. - S3: Define stale-row detection concretely (catalog as_of seq vs sidecar/manifest revision), cache class (SWR per conventions), conditional-GET revalidation (law 4). - S4: Pagination interplay with #117 caps (50×10) — specify slice-after-server-sort. - S5: JSON conventions per 14.12: uint64s as strings, arrays [] never null. ## Nit - N1: ownerRepos 200-[]-for-unknown-owner behavior (§8) must be preserved by the new endpoint.
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 (repo-scoped, contention-free, +0 round trips); the aggregate catalog folds off-hot-path via maintainer sweep (staleness = documented pass interval). Sim budgets prove push unchanged.
  • B2 — no in-place retype. Owner/repo string lists stay; a NEW endpoint (with triple twins + discovery + SDK) serves object rows alongside v1.
  • B3 — commit-date derivation site named at implementation (derive once outside the CAS loop or best-effort after commitLocal; new argv in 04_git.md); derivation failure NEVER fails the push (null + maintainer heals).
  • B4 — frozen-list amendment for the sidecar family + aggregate fields in the same change (hand codec + golden fixtures); manifest-inline ruled out. Shared with #248 — ONE amendment, landed by whichever merges first (#248 establishes it).
  • B5 — no ref snapshot in manifest.pb. Cold derivation = refs sync + git read; backfill budgets this; enumeration via maintain engine (fleet-wide LIST only if bounded/resumable).

Should-fix adoptions

liveRepos Head-vs-Get accuracy; null-vs-0 semantics; tie-breaks; stale-row detection; SWR class; uint64-as-string; #117 cap interplay. Landing order: #248's rails first, activity second on the same shape.

# 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 (repo-scoped, contention-free, +0 round trips); the aggregate catalog folds off-hot-path via maintainer sweep (staleness = documented pass interval). Sim budgets prove push unchanged. - **B2 — no in-place retype.** Owner/repo string lists stay; a NEW endpoint (with triple twins + discovery + SDK) serves object rows alongside v1. - **B3 — commit-date derivation site named at implementation** (derive once outside the CAS loop or best-effort after commitLocal; new argv in 04_git.md); derivation failure NEVER fails the push (null + maintainer heals). - **B4 — frozen-list amendment** for the sidecar family + aggregate fields in the same change (hand codec + golden fixtures); manifest-inline ruled out. Shared with #248 — ONE amendment, landed by whichever merges first (#248 establishes it). - **B5 — no ref snapshot in manifest.pb.** Cold derivation = refs sync + git read; backfill budgets this; enumeration via maintain engine (fleet-wide LIST only if bounded/resumable). ## Should-fix adoptions liveRepos Head-vs-Get accuracy; null-vs-0 semantics; tie-breaks; stale-row detection; SWR class; uint64-as-string; #117 cap interplay. Landing order: #248's rails first, activity second on the same shape.
Author
Owner

Implementation ready for review: PR #260 (feat/issue-247, one commit on origin/main). Builds on the #248 rails per R1 — shared sidecar/catalog/sweep/endpoint extended, never duplicated. Production-verified full lifecycle (push hint, tag-push nulls, sweep heal folded=1/1/0, explore orders by commit date). All tiers green; browser render proof blocked by the shared-daemon network guard (documented in 12_web_ui.md, same guard a prior entry cites). Do-not-merge pending review.

Implementation ready for review: PR #260 (feat/issue-247, one commit on origin/main). Builds on the #248 rails per R1 — shared sidecar/catalog/sweep/endpoint extended, never duplicated. Production-verified full lifecycle (push hint, tag-push nulls, sweep heal folded=1/1/0, explore orders by commit date). All tiers green; browser render proof blocked by the shared-daemon network guard (documented in 12_web_ui.md, same guard a prior entry cites). Do-not-merge pending review.
Author
Owner

Review of PR #260 (feat/issue-247): READY TO MERGE (with one pushed nit fix; see below). Verified in scratch worktree /tmp/pr260 at 5028ab2 plus fixup ccd7e51; main worktree untouched and clean.

R1 COMPLIANCE (all five blocking rulings):

  • B1 (no bucket-root CAS on push): PASS. Push writes ONLY the repo-scoped meta/stats.json sidecar, issued in parallel with the manifest CAS (+1 total op, +0 sequential trips; internal/wal/publish.go:404-429). Bucket-root catalog folded off-hot-path by the maintainer sweep (1 CAS/pass). Push-budget gate green with totals IDENTICAL to E16 (cold 11 / warm 10).
  • B2 (no in-place retype): PASS by extension, acceptable. v1 string lists untouched; the #248-new /detailed surface (itself the R1-sanctioned new endpoint) gains additive optional fields + sort=activity. Triple twins + discovery + SDK all covered (internal/api/routes.go:52-54, repos_detailed_test.go:153 TestOwnerReposDetailedTwinsAndDiscovery). A second endpoint for the same rows would fork the surface for no isolation gain; rationale documented in 07_api.md and 14_extensibility.md.
  • B3 (derivation site + never-fail + argv): PASS. derivePushActivity (internal/server/bind_wal.go:208) runs before h.Publish, i.e. outside the CAS ladder; nil on ANY failure; exact argv git log -1 --format=%cI%x00%aI documented in 04_git.md 4.5 with real-git tests (internal/git/activity_test.go).
  • B4 (frozen amendment + codec + fixtures): PASS. 14 14.11 amended with no new bucket keys (shared #248 families); hand codec fields 6-8 with golden fixture catalog_activity + legacy-decode + empty-encodes-to-nothing tests; wal.proto + 02 2.1/2.2 updated in the same change.
  • B5 (cold derivation budget): PASS. resolveActivity budget stated in 10_maintenance.md 4 (refs-view reads + <=1 serve-sync + 1 git, ActivityParallel=2, fresh sidecars cost zero git via hook-skip). Enumeration is the engine local list, bounded 256/pass with cursor resume; fleet-wide LIST explicitly a V1 non-goal, documented.

MERGE CONTRACT (walked both directions): publish blind-write re-derives size arithmetically from the held manifest (no clobber of size); sweep foldOne never regresses activity it cannot refresh (TestFoldOneHookFailurePreserves, TestFoldOnePreservesActivityWithoutHook); catalog same-head monotonicity preserves known rows (TestSweepCatalogNeverRegressesSameHeadRow). HeadTarget symref fallback is sound: tip always comes from the WAL view, the file only supplies the target name when the view never recorded one, and every miss preserves + continues (tested incl. serve-sync rescue). One documented residual wart (non-blocking): a tag-only push blind-writes null commit fields until the next sweep heals them (bounded by the pass interval); forced by the no-sidecar-read law, disclosed in E17 + 05_wal_engine.md.

BUDGETS: [5,14]->[5,16] honest — each +1 is a documented blind sidecar PUT (#248 pack publish, #247 ref-txn push clock), flatness asserted separately (TestEvidenceImportFlat), push totals unchanged. Sim fence updated in-test with rationale, not weakened.

SHOULD-FIX: null-vs-0, unknowns-last-either-direction, tie-breaks, stale detection (head_seq + monotonicity), SWR class, #117 slice-after-sort all land tested. uint64-as-JSON-numbers deviates from the 14.12 as-strings convention but matches the LANDED #248 shape — consistency wins, not this PRs to fork. Frontend: explore + /:owner share repos:{owner} detailed(sort=activity&order=desc); ActivityStamp at/empty shortcut sound (undefined=fetch, null+empty=no-commits, null+nonempty=legacy fallback).

NIT FIXED + PUSHED (ccd7e51): orderByActivity tied on (name,owner) while Go FilterSort ties on (owner,name) and E17 claimed they match. Aligned JS to (owner,name) + added a cross-owner tie test. Zero behavioral change on listing pages (single-owner lists).

TESTS: go -race green on proto/git/sizecatalog/maintain/api/store/wal/repoimport/cmd-push-budget; server green except TestUIAssetConcepts (environmental: scratch dist lacks landing GIFs, needs full make web; PR touches only bind_wal.go + its test there). Coverage >=95% on every touched package (95.0-98.3). node --test: all PR-touched files pass (owners/sdk-surface/activity 24/24 incl. the new tie test); 7 unrelated files fail ONLY in scratch (node_modules is gitignored and absent there; no package.json changes). gofmt/vet clean. No new Go modules or npm deps. TestDiscoveryShape passes -count=3, no flake observed. No browser run per task scope (nothing browser-facing beyond data-driven rendering already covered headless).

MERGE RECOMMENDATION: ready to merge.

Review of PR #260 (feat/issue-247): READY TO MERGE (with one pushed nit fix; see below). Verified in scratch worktree /tmp/pr260 at 5028ab2 plus fixup ccd7e51; main worktree untouched and clean. R1 COMPLIANCE (all five blocking rulings): - B1 (no bucket-root CAS on push): PASS. Push writes ONLY the repo-scoped meta/stats.json sidecar, issued in parallel with the manifest CAS (+1 total op, +0 sequential trips; internal/wal/publish.go:404-429). Bucket-root catalog folded off-hot-path by the maintainer sweep (1 CAS/pass). Push-budget gate green with totals IDENTICAL to E16 (cold 11 / warm 10). - B2 (no in-place retype): PASS by extension, acceptable. v1 string lists untouched; the #248-new /detailed surface (itself the R1-sanctioned new endpoint) gains additive optional fields + sort=activity. Triple twins + discovery + SDK all covered (internal/api/routes.go:52-54, repos_detailed_test.go:153 TestOwnerReposDetailedTwinsAndDiscovery). A second endpoint for the same rows would fork the surface for no isolation gain; rationale documented in 07_api.md and 14_extensibility.md. - B3 (derivation site + never-fail + argv): PASS. derivePushActivity (internal/server/bind_wal.go:208) runs before h.Publish, i.e. outside the CAS ladder; nil on ANY failure; exact argv git log -1 --format=%cI%x00%aI <sha> documented in 04_git.md 4.5 with real-git tests (internal/git/activity_test.go). - B4 (frozen amendment + codec + fixtures): PASS. 14 14.11 amended with no new bucket keys (shared #248 families); hand codec fields 6-8 with golden fixture catalog_activity + legacy-decode + empty-encodes-to-nothing tests; wal.proto + 02 2.1/2.2 updated in the same change. - B5 (cold derivation budget): PASS. resolveActivity budget stated in 10_maintenance.md 4 (refs-view reads + <=1 serve-sync + 1 git, ActivityParallel=2, fresh sidecars cost zero git via hook-skip). Enumeration is the engine local list, bounded 256/pass with cursor resume; fleet-wide LIST explicitly a V1 non-goal, documented. MERGE CONTRACT (walked both directions): publish blind-write re-derives size arithmetically from the held manifest (no clobber of size); sweep foldOne never regresses activity it cannot refresh (TestFoldOneHookFailurePreserves, TestFoldOnePreservesActivityWithoutHook); catalog same-head monotonicity preserves known rows (TestSweepCatalogNeverRegressesSameHeadRow). HeadTarget symref fallback is sound: tip always comes from the WAL view, the file only supplies the target name when the view never recorded one, and every miss preserves + continues (tested incl. serve-sync rescue). One documented residual wart (non-blocking): a tag-only push blind-writes null commit fields until the next sweep heals them (bounded by the pass interval); forced by the no-sidecar-read law, disclosed in E17 + 05_wal_engine.md. BUDGETS: [5,14]->[5,16] honest — each +1 is a documented blind sidecar PUT (#248 pack publish, #247 ref-txn push clock), flatness asserted separately (TestEvidenceImportFlat), push totals unchanged. Sim fence updated in-test with rationale, not weakened. SHOULD-FIX: null-vs-0, unknowns-last-either-direction, tie-breaks, stale detection (head_seq + monotonicity), SWR class, #117 slice-after-sort all land tested. uint64-as-JSON-numbers deviates from the 14.12 as-strings convention but matches the LANDED #248 shape — consistency wins, not this PRs to fork. Frontend: explore + /:owner share repos:{owner} detailed(sort=activity&order=desc); ActivityStamp at/empty shortcut sound (undefined=fetch, null+empty=no-commits, null+nonempty=legacy fallback). NIT FIXED + PUSHED (ccd7e51): orderByActivity tied on (name,owner) while Go FilterSort ties on (owner,name) and E17 claimed they match. Aligned JS to (owner,name) + added a cross-owner tie test. Zero behavioral change on listing pages (single-owner lists). TESTS: go -race green on proto/git/sizecatalog/maintain/api/store/wal/repoimport/cmd-push-budget; server green except TestUIAssetConcepts (environmental: scratch dist lacks landing GIFs, needs full make web; PR touches only bind_wal.go + its test there). Coverage >=95% on every touched package (95.0-98.3). node --test: all PR-touched files pass (owners/sdk-surface/activity 24/24 incl. the new tie test); 7 unrelated files fail ONLY in scratch (node_modules is gitignored and absent there; no package.json changes). gofmt/vet clean. No new Go modules or npm deps. TestDiscoveryShape passes -count=3, no flake observed. No browser run per task scope (nothing browser-facing beyond data-driven rendering already covered headless). MERGE RECOMMENDATION: ready to merge.
Author
Owner

Implemented in PR #260 incl. review tie-break fix (all R1 rulings verified, merge contract walked both directions; all gates green), merged. Closing.

Implemented in PR #260 incl. review tie-break fix (all R1 rulings verified, merge contract walked both directions; all gates green), merged. Closing.
crueber added this to the v1 milestone 2026-09-10 22:27:10 +00:00
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
crueber/walhub#247
No description provided.