Skip to content

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

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

hannesrudolph wants to merge 5 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 15, 2026, 6:44 PM ET / 22:44 UTC (Revision 4).

ClawSweeper review

What this changes

Adds commands to collect GitHub repository counters and releases, import historical observations, and inspect a separate private SQLite database.

Merge readiness

Needs changes before merge - 3 items remain

The feature remains useful and absent from the pinned main branch; both previously reported correctness blockers remain unchanged.

Priority: P2
Reviewed head: ef72049e24791e7bcb24bf0f325c3830b2a919a3

Review scores

Measure Result What it means
Overall readiness 🦐 gold shrimp (3/6) The implementation has substantial focused coverage, but two previously identified correctness defects still block readiness.
Proof confidence 🌊 off-meta tidepool Not applicable: The MEMBER-authored PR is exempt from ordinary contributor proof; its body reports fixture-based CLI collect/import/status checks, not live GitHub or LaunchAgent observations.
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 ordinary contributor proof; its body reports fixture-based CLI collect/import/status checks, not live GitHub or LaunchAgent observations.
Evidence reviewed 9 items Timestamp ordering defect: Validation accepts RFC3339 offsets and fractional precision, while status selects max(observed_at) from a TEXT column; insertion preserves the original strings.
Initialization failure is not retryable: Open creates the destination before initializing SQLite and writing ownership metadata. Error exits leave that file behind, while subsequent opens reject empty files or missing ownership metadata.
Earlier findings remain applicable: The comparison against the preceding reviewed revision returned no changes to the store implementation or its tests. The supplied completed-review projection records both findings and their regression-coverage requests.
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 takes repository targets and GitHub responses or imported history, then stores observations independently of those archives.

flowchart TD
  A[Metrics configuration] --> B[Metrics commands]
  C[GitHub counters and releases] --> D[Collector]
  B --> D
  E[Imported history] --> F[Scope and row validation]
  D --> F
  F --> G[Independent SQLite store]
  G --> H[Read-only status]
Loading

Before merge

  • Compare observation timestamps chronologically (P2) - This previously reported blocker remains. Imports accept and 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 produces incorrect ordering. Compare parsed instants, including existing imported rows, and cover both cases.
  • Recover safely from failed first-time database initialization (P2) - This previously reported blocker remains. After creating the destination file, a canceled context or SQLite initialization error returns without removing or completing the new store. The next invocation rejects the leftover empty file or missing ownership metadata, so a transient first-run failure permanently blocks collection at that path. Make initialization retryable while retaining rejection of pre-existing empty or foreign databases.
  • Complete next step (P2) - Repair chronological status selection and failed first-time initialization, with regression coverage for existing imported rows and database preservation.

Findings

  • [P2] Compare observation timestamps chronologically — internal/headlinemetrics/metrics.go:351-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 Production +816 net lines; tests +910 lines Production growth implements the stated independent collector, store, CLI, and cross-platform writer lock.

Technical review

Best possible solution:

Keep the independent store design, compare timestamp instants accurately, and make first-time initialization recoverable without relaxing protection of existing databases.

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

Yes, source establishes both patch defects: import offset-bearing timestamps before reading status, or cancel first-time initialization after file creation and retry. Neither scenario was executed here; the metrics feature is absent from pinned main.

Is this the best way to solve the issue?

Yes, an independent store fits the requested isolation, but chronological comparisons and recoverable initialization are necessary for reliable operation.

Full review comments:

  • [P2] Compare observation timestamps chronologically — internal/headlinemetrics/metrics.go:351-352
    This previously reported blocker remains. Imports accept and 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 produces incorrect ordering. Compare parsed instants, including existing imported rows, and cover 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. After creating the destination file, a canceled context or SQLite initialization error returns without removing or completing the new store. The next invocation rejects the leftover empty file or missing ownership metadata, so a transient first-run failure permanently blocks collection at that path. Make initialization retryable while retaining rejection of pre-existing empty or foreign databases.
    Confidence: 0.99

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 0bb13412e4c8.

Labels

Label justifications:

  • P2: This bounded optional metrics feature has two store-correctness defects, with no demonstrated 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 ordinary contributor proof; its body reports fixture-based CLI collect/import/status checks, not live GitHub or LaunchAgent observations.

Evidence

Acceptance criteria:

  • [P1] GOWORK=off go test -count=1 ./internal/headlinemetrics ./internal/cli.
  • [P1] GOWORK=off go test -race -count=1 ./internal/headlinemetrics.
  • [P1] GOWORK=off go vet ./internal/headlinemetrics ./internal/cli.

What I checked:

  • Timestamp ordering defect: Validation accepts RFC3339 offsets and fractional precision, while status selects max(observed_at) from a TEXT column; insertion preserves the original strings. (internal/headlinemetrics/metrics.go:352, ef72049e2479)
  • Initialization failure is not retryable: Open creates the destination before initializing SQLite and writing ownership metadata. Error exits leave that file behind, while subsequent opens reject empty files or missing ownership metadata. (internal/headlinemetrics/metrics.go:192, ef72049e2479)
  • Earlier findings remain applicable: The comparison against the preceding reviewed revision returned no changes to the store implementation or its tests. The supplied completed-review projection records both findings and their regression-coverage requests. (internal/headlinemetrics/metrics.go:169, ef72049e2479)
  • Storage dependency boundary: The new implementation imports CrawlKit's store package and calls store.Open with its application-owned schema; go.mod pins CrawlKit v0.16.1. This establishes the dependency contract relevant to initialization failure. (internal/headlinemetrics/metrics.go:200, ef72049e2479)
  • Dependency error handling: The tagged store implementation closes its connection after ping or schema failure but does not remove the destination file. The v0.16.1 tag resolves to this commit; its full root policy was also read. (store/store.go:40, 0bb18e9865a2)
  • Main and release necessity check: The pinned main command dispatcher has no metrics command, and tree checks found neither the metrics package nor its CLI file on main or v0.10.0. GitHub canonical searches were rejected by the limited-mode endpoint, so no broader duplicate relationship is asserted. (internal/cli/app.go:127, 0bb13412e4c8)

Likely related people:

  • unknown: The claimed source-line change could not be verified from bounded local history. (role: source history unknown; confidence: low)
  • Peter Steinberger: 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 existing 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 (3 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

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