Skip to content

feat: collect repository metrics in an independent store - #206

Open
hannesrudolph wants to merge 6 commits into
mainfrom
feat/metrics-collection
Open

hannesrudolph wants to merge 6 commits into
mainfrom
feat/metrics-collection

Conversation

@hannesrudolph

@hannesrudolph hannesrudolph commented Sep 15, 2026

Copy link
Copy Markdown
Member

What Problem This Solves

Repository headline history currently requires a separate collector instead of a discoverable Gitcrawl command.

User Impact

Adds gitcrawl metrics collect|import|status --config metrics.json for stars, forks, actual subscribers, open PRs/issues, optional completed-day clones, and stable releases. Operators can retain OpenClaw repository metrics in a separate private SQLite metrics database, using native GitHub authentication and JSON output.

Why This Change Was Made

The metrics store preserves unknown values, zeroes, decreases, original import IDs, and daily corrections. Imports are scoped and atomic; archive/wrong-owner databases are rejected before a writable open. The commands never invoke archive refresh, embeddings, models, or schedules. Help, control metadata, and source documentation expose the full workflow.

Evidence

  • Collector/store and native CLI fixtures pass, including actual watcher semantics, missing/incomplete PR counts, optional clone 403/404, release pagination beyond five pages, cancellation, rollback of late invalid imports, daily revisions, and archive ownership/path protection.
  • Full Go suite and focused race checks pass; coverage exceeds the repository's 85% gate.
  • Vet, vulnerability/dead-code scans, module tidiness, formatting, docs build/tests, and release-script tests pass. One existing formatting-only indentation issue was corrected to satisfy the formatting gate.
  • A locally built CLI passed fixture-only collect/import/status acceptance: two repositories, idempotent releases/imports, daily deduplication, and preserved NULLs. No live archives, API credentials, jobs, or services were changed.
  • Local source builds require no signing credentials. Cross-platform GoReleaser snapshot builds were not run locally (GoReleaser is unavailable); the existing CI matrix covers them. Official signing/notarization remains the release workflow's responsibility; no release was published.

@clawsweeper

clawsweeper Bot commented Sep 15, 2026

Copy link
Copy Markdown

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

ClawSweeper review complete

ClawSweeper finished reviewing this revision. The review result is being finalized.

View the workflow run.

@clawsweeper clawsweeper Bot added P2 Normal priority bug or improvement with limited blast radius. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. labels Sep 15, 2026
@clawsweeper

clawsweeper Bot commented Sep 15, 2026

Copy link
Copy Markdown

Codex review: needs changes before merge. Reviewed September 17, 2026, 1:34 PM ET / 17:34 UTC (Revision 7).

ClawSweeper review

What this changes

Adds commands to collect GitHub repository counters and releases, import historical observations, and inspect an independent SQLite metrics database.

Merge readiness

Needs changes before merge - 3 items remain

Keep open: this remains a distinct contribution absent from main, and both previously reported correctness blockers remain in the pinned head.

Priority: P2
Reviewed head: 6b27f8d797be9b2977ad915287c02afabb17bb09

Review scores

Measure Result What it means
Overall readiness 🦐 gold shrimp (3/6) The feature has substantial focused coverage, but two reproducible source-level defects remain unresolved.
Proof confidence 🌊 off-meta tidepool Not applicable: The MEMBER-authored PR is exempt from the ordinary contributor proof gate; its captured body reports fixture-only native CLI acceptance for collection, import, and status, which is supplemental validation.
Patch quality 🦐 gold shrimp (3/6) 2 actionable review findings remain.

Verification

Check Result Evidence
Real behavior Not applicable Not applicable: The MEMBER-authored PR is exempt from the ordinary contributor proof gate; its captured body reports fixture-only native CLI acceptance for collection, import, and status, which is supplemental validation.
Evidence reviewed 8 items Timestamp ordering blocker remains: Validation accepts RFC3339 timestamps with offsets and variable fractional precision, and insertion preserves their text. Status selects max(observed_at), which does not reliably select the latest instant.
Initialization recovery blocker remains: Open creates the final database file before opening SQLite and inserting ownership metadata. Errors return without removing the newly created incomplete database; subsequent opens reject empty files or databases missing the ownership marker.
Storage dependency boundary: The new storage implementation directly imports CrawlKit's store package and calls store.Open; go.mod pins CrawlKit v0.16.1. Its initialization error behavior is therefore relevant to the finding.
Findings 2 actionable findings [P2] Compare observation timestamps chronologically
[P2] Recover safely from failed first-time database initialization
Security None None.

How this fits together

Gitcrawl normally maintains searchable GitHub thread archives. The new metrics subsystem reads repository statistics or imported history into a separate database and returns collection results or read-only status.

flowchart LR
  A[Metrics configuration] --> B[Metrics commands]
  C[GitHub statistics] --> B
  D[Imported history] --> B
  B --> E[Writer lock and validation]
  E --> F[Independent metrics database]
  F --> G[Read-only status]
  B --> H[Collection or import result]
Loading

Before merge

  • Compare observation timestamps chronologically (P2) - This previously reported blocker remains. Imports preserve RFC3339 offsets, but max(observed_at) compares TEXT: 2026-09-15T10:00:00+02:00 sorts after 2026-09-15T09:00:00Z despite being an hour older. Variable fractional precision also misorders instants within a second. Select the latest parsed instant, including rows already stored, and add regression coverage for both cases.
  • Recover safely from failed first-time database initialization (P2) - This previously reported blocker remains. The final database file is created before SQLite initialization and ownership insertion. A canceled context or initialization error leaves an empty or unowned file; subsequent collect/import attempts reject it permanently, even after the original error clears. Make failed creation retryable without deleting or accepting pre-existing foreign databases, and cover failure followed by a successful retry.
  • Complete next step (P2) - Repair chronological status selection and failed first-initialization recovery, with regression coverage for existing imported rows and safe database reuse.

Findings

  • [P2] Compare observation timestamps chronologically — internal/headlinemetrics/metrics.go:352
  • [P2] Recover safely from failed first-time database initialization — internal/headlinemetrics/metrics.go:200-207
Agent review details

Security

None.

Review metrics

Metric Value Why it matters
Production and test growth Go production net +820 lines; tests +910 lines Production growth implements the explicitly requested independent collector, storage, CLI, and writer locking.

Technical review

Best possible solution:

Retain the independent store design, select timestamps by parsed instant without rewriting imported history, and make initial database creation safely retryable while preserving foreign-database rejection.

Do we have a high-confidence way to reproduce the issue?

Yes, source establishes both branch-level failure paths: mixed timestamp representations misorder status, and cancellation during initial SQLite opening leaves a rejected file. Neither path was executed during this read-only review.

Is this the best way to solve the issue?

Yes, the separate store is a coherent implementation boundary, but chronological status selection and retryable initialization must be repaired before landing.

Full review comments:

  • [P2] Compare observation timestamps chronologically — internal/headlinemetrics/metrics.go:352
    This previously reported blocker remains. Imports preserve RFC3339 offsets, but max(observed_at) compares TEXT: 2026-09-15T10:00:00+02:00 sorts after 2026-09-15T09:00:00Z despite being an hour older. Variable fractional precision also misorders instants within a second. Select the latest parsed instant, including rows already stored, and add regression coverage for both cases.
    Confidence: 0.99
  • [P2] Recover safely from failed first-time database initialization — internal/headlinemetrics/metrics.go:200-207
    This previously reported blocker remains. The final database file is created before SQLite initialization and ownership insertion. A canceled context or initialization error leaves an empty or unowned file; subsequent collect/import attempts reject it permanently, even after the original error clears. Make failed creation retryable without deleting or accepting pre-existing foreign databases, and cover failure followed by a successful retry.
    Confidence: 0.98

Overall correctness: patch is incorrect
Overall confidence: 0.98

AGENTS.md: not found in the target repository.

Codex review notes: model internal, reasoning medium; reviewed against d99af2ed7b3b.

Labels

Label justifications:

  • P2: This is an optional metrics improvement with two bounded correctness defects and no demonstrated urgent impact on existing archive workflows.
  • rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🌊 off-meta tidepool and patch quality is 🦐 gold shrimp.
  • status: ⏳ waiting on author: ClawSweeper has contributor-facing work open and is waiting for author action. Not applicable: The MEMBER-authored PR is exempt from the ordinary contributor proof gate; its captured body reports fixture-only native CLI acceptance for collection, import, and status, which is supplemental validation.

Evidence

Acceptance criteria:

  • [P1] go test ./internal/headlinemetrics ./internal/cli.
  • [P1] go test -race ./internal/headlinemetrics.
  • [P1] go test ./...
  • [P1] git diff --check.

What I checked:

  • Timestamp ordering blocker remains: Validation accepts RFC3339 timestamps with offsets and variable fractional precision, and insertion preserves their text. Status selects max(observed_at), which does not reliably select the latest instant. (internal/headlinemetrics/metrics.go:352, 6b27f8d797be)
  • Initialization recovery blocker remains: Open creates the final database file before opening SQLite and inserting ownership metadata. Errors return without removing the newly created incomplete database; subsequent opens reject empty files or databases missing the ownership marker. (internal/headlinemetrics/metrics.go:192, 6b27f8d797be)
  • Storage dependency boundary: The new storage implementation directly imports CrawlKit's store package and calls store.Open; go.mod pins CrawlKit v0.16.1. Its initialization error behavior is therefore relevant to the finding. (internal/headlinemetrics/metrics.go:200, 6b27f8d797be)
  • Dependency confirms incomplete-file retention: At v0.16.1, store.Open closes SQLite on PingContext or schema errors but does not remove the database file. The annotated release tag resolves to this commit. (store/store.go:40, 0bb18e9865a2)
  • Current-main and release necessity check: Current main has no metrics command dispatch or independent metrics package. The supplied v0.10.0 release commit likewise contains neither metrics path. A bounded read of the 100 most recently updated PRs found only this proposal matching metrics/headline/counter terminology; no replacement was established. (internal/cli/app.go:129, d99af2ed7b3b)
  • Re-review continuity and validation: The supplied completed review already identifies both findings at this exact head. Existing tests cover fresh stores, repeated imports, rejected foreign databases, partial collection, and writer serialization, but do not cover chronological timestamp selection or failed first-initialization recovery. Tests were inspected, not executed. (internal/headlinemetrics/metrics_test.go:45, 6b27f8d797be)

Likely related people:

  • steipete: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)
  • vincentkoc: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)

Rank-up moves

Optional improvements that raise the rating; they are not merge blockers.

  • Add chronological status selection covering offsets and fractional precision in already-imported rows.
  • Make failed initialization retryable and verify fresh creation, existing-store reuse, and unchanged foreign-database rejection.

Rating scale

Score Internal tier Crab rank Meaning
6/6 S 🦀 challenger crab Exceptional readiness
5/6 A 🦞 diamond lobster Very strong readiness
4/6 B 🐚 platinum hermit Good normal PR; ordinary maintainer review
3/6 C 🦐 gold shrimp Useful, but confidence is limited
2/6 D 🦪 silver shellfish Proof or implementation needs work
1/6 F 🧂 unranked krab Not merge-ready
N/A NA 🌊 off-meta tidepool Rating does not apply

Overall follows the weaker of proof and patch quality.
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

Workflow

  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

History

Review history (6 earlier review cycles)
  • reviewed 2026-09-15T21:29:36.999Z sha 22b4cf5 :: needs changes before merge. :: [P2] Compare observation timestamps chronologically | [P2] Recover safely from failed first-time database initialization
  • reviewed 2026-09-15T21:52:39.595Z sha abc138c :: needs changes before merge. :: [P2] Compare observation timestamps chronologically | [P2] Recover safely from failed first-time database initialization
  • reviewed 2026-09-15T22:37:36.873Z sha a7befd3 :: needs changes before merge. :: [P2] Compare observation timestamps chronologically | [P2] Recover safely from failed first-time database initialization
  • reviewed 2026-09-15T22:44:20.260Z sha ef72049 :: needs changes before merge. :: [P2] Compare observation timestamps chronologically | [P2] Recover safely from failed first-time database initialization
  • reviewed 2026-09-15T22:59:17.567Z sha 6b27f8d :: needs changes before merge. :: [P2] Compare observation timestamps chronologically | [P2] Recover safely from failed first-time database initialization
  • reviewed 2026-09-16T22:54:04.192Z sha 6b27f8d :: needs changes before merge. :: [P2] Compare observation timestamps chronologically | [P2] Recover safely from failed first-time database initialization

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

P2 Normal priority bug or improvement with limited blast radius. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant