Skip to content

fix(analytics): read the discovery receipt beside the resolved config - #231

Merged
steipete merged 1 commit into
openclaw:mainfrom
SebTardif:fix/gitcrawl-f004
Oct 7, 2026
Merged

steipete merged 1 commit into
openclaw:mainfrom
SebTardif:fix/gitcrawl-f004

Conversation

@SebTardif

@SebTardif SebTardif commented Oct 4, 2026 •

Copy link
Copy Markdown
Contributor

analytics --once and analytics --watch looked for the historical discovery receipt in the working directory when the config came from GITCRAWL_CONFIG. Resolve the config path using the same rules as the runtime, then read status.json beside that file.

The synthetic regression fails on unchanged main and passes with this fix, including an unrelated receipt in the working directory. Existing empty-baseline and repository-binding tests also pass. README documents where the receipt belongs, and the Unreleased changelog credits @SebTardif.

Full make check passed on Hetzner with the pinned toolchain, 85.6% coverage, and six snapshot targets. Independent Codex P0–P3 review found no actionable issues. Exact-head cross-platform CI, Docker, and secret scanning passed at 87118b92521b439a87a47574065bffa07bf40544.

Thanks @SebTardif for the report, original fix, and regression test.

@clawsweeper

clawsweeper Bot commented Oct 4, 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: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. labels Oct 4, 2026
@clawsweeper

clawsweeper Bot commented Oct 4, 2026 •

Copy link
Copy Markdown

Codex review: needs maintainer review before merge. Reviewed October 6, 2026, 9:44 PM ET / October 7, 2026, 01:44 UTC (Revision 4).

ClawSweeper review

What this changes

Analytics reads historical discovery totals beside the resolved configuration file, with a regression test and documentation for running from another directory.

Merge readiness

✅ Ready for maintainer review

This PR remains useful: current main and v0.15.0 still use the unresolved config path. No actionable patch defect was found, and the supplied runtime evidence adequately demonstrates receipt selection.

Priority: P2
Reviewed head: 87118b92521b439a87a47574065bffa07bf40544

Review scores

Measure Result What it means
Overall readiness 🐚 platinum hermit (4/6) A focused repair with relevant real runtime evidence and a regression test has no identified merge-blocking defect.
Proof confidence 🐚 platinum hermit (4/6) Sufficient (terminal): The macOS --once and --status transcript exercises the production receipt reader with a real SQLite archive and conflicting config-directory and working-directory receipts, observing the intended 4/9 totals. GitHub source inspection confirms the earlier proof revision used the current resolved lookup. Successful GitHub collection was outside that scenario; no stored-data contract changes require migration.
Patch quality 🐚 platinum hermit (4/6) No actionable review findings were identified.

Verification

Check Result Evidence
Real behavior Verified Sufficient (terminal): The macOS --once and --status transcript exercises the production receipt reader with a real SQLite archive and conflicting config-directory and working-directory receipts, observing the intended 4/9 totals. GitHub source inspection confirms the earlier proof revision used the current resolved lookup. Successful GitHub collection was outside that scenario; no stored-data contract changes require migration.
Evidence reviewed 9 items Verified introduced change: The pinned main-to-head delta changes one production expression, adds a 59-line regression test, and adds README and changelog entries. The reader now uses the same resolver as configuration loading.
Current main still needs the fix: Current main reads status.json beside a.configPath without resolving it. The CLI stores the explicit config argument there, while LoadRuntime resolves environment-selected and default configuration paths separately.
Release check: The supplied latest release, v0.15.0, also contains the unresolved receipt lookup; the requested fix is not already present there.
Findings None None.
Security None None.

How this fits together

Gitcrawl analytics maintains GitHub collection coverage in a local SQLite archive. A historical discovery receipt supplies its initial totals before ongoing collection begins.

flowchart TD
  A[Config flag or environment] --> B[Resolve selected config]
  B --> C[Read adjacent discovery receipt]
  C --> D[Validate repository and totals]
  D --> E[Initialize missing archive coverage]
  E --> F[Collect GitHub updates]
Loading

Before merge

None.

Agent review details

Security

None.

Review metrics

Metric Value Why it matters
Production and test delta production +1/-1; tests +59/-0 The focused regression adds coverage without growing the production implementation.

Technical review

Best possible solution:

Use the existing config resolver consistently for receipt lookup while preserving repository validation and already-established coverage.

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

Yes: main uses the raw config argument for receipt lookup even though configuration loading resolves GITCRAWL_CONFIG separately. No reviewer-side execution was performed; the supplied patched runtime trace demonstrates the corrected selection.

Is this the best way to solve the issue?

Yes: reusing the existing resolver is a narrow repair that aligns receipt lookup with configuration loading without adding another setting or implementation.

AGENTS.md: found but not applied because it conflicted with ClawSweeper's review contract.

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

Labels

Label changes:

No label changes.

Label justifications:

  • P2: This repairs analytics initialization for environment-selected or default configuration paths with a limited blast radius.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit and patch quality is 🐚 platinum hermit.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (terminal): The macOS --once and --status transcript exercises the production receipt reader with a real SQLite archive and conflicting config-directory and working-directory receipts, observing the intended 4/9 totals. GitHub source inspection confirms the earlier proof revision used the current resolved lookup. Successful GitHub collection was outside that scenario; no stored-data contract changes require migration.
  • proof: sufficient: Contributor real behavior proof is sufficient. The macOS --once and --status transcript exercises the production receipt reader with a real SQLite archive and conflicting config-directory and working-directory receipts, observing the intended 4/9 totals. GitHub source inspection confirms the earlier proof revision used the current resolved lookup. Successful GitHub collection was outside that scenario; no stored-data contract changes require migration.

Evidence

What I checked:

  • Verified introduced change: The pinned main-to-head delta changes one production expression, adds a 59-line regression test, and adds README and changelog entries. The reader now uses the same resolver as configuration loading. (internal/cli/analytics.go:153, 87118b92521b)
  • Current main still needs the fix: Current main reads status.json beside a.configPath without resolving it. The CLI stores the explicit config argument there, while LoadRuntime resolves environment-selected and default configuration paths separately. (internal/cli/analytics.go:153, 8e02671294b2)
  • Release check: The supplied latest release, v0.15.0, also contains the unresolved receipt lookup; the requested fix is not already present there. (internal/cli/analytics.go:153, e8448c176044)
  • Existing resolver contract: Load and Save already call ResolvePath, and TestResolvePathUsesEnv defines the GITCRAWL_CONFIG behavior. The resolver delegates to crawlkit/config; this PR reuses that existing contract without changing dependency versions or its API. (internal/config/config.go:104, 87118b92521b)
  • Real behavior proof and revision continuity: The captured PR body reports a macOS binary at f5b4582 running --once against a real SQLite archive, followed by --status showing 4 issues and 9 pull requests from the selected config's receipt rather than the working-directory totals of 111 and 222. GitHub's original file endpoint confirms that revision used the same resolved lookup as the current patch. The subsequent partial_response failure does not negate the observed initialization result or prove successful GitHub collection. (internal/cli/analytics.go:153, f5b458260b88)
  • Validation and compatibility boundary: Receipt repository binding, completion checks, totals, and timestamps remain unchanged. Initialization still runs only when the repository has no coverage row; no persisted schema or format changes. The added test exercises environment-selected configuration from an unrelated working directory. Tests were inspected, not executed, under the read-only review contract. (internal/cli/analytics_config_path_test.go:19, 87118b92521b)

Likely related people:

  • Hannes Rudolph: Raw commit 84ed753 adds internal/cli/analytics.go:165 relative to its recorded parents. This identifies author metadata, not feature responsibility or a PR merger. (role: source-line author; confidence: high; commits: 84ed753d157f; files: internal/cli/analytics.go)
  • steipete: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)

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-10-04T02:47:21.237Z sha c6c5533 :: needs real behavior proof before merge. :: none
  • reviewed 2026-10-04T02:58:23.573Z sha f5b4582 :: needs real behavior proof before merge. :: none
  • reviewed 2026-10-04T16:58:55.812Z sha f5b4582 :: needs maintainer review before merge. :: none

@clawsweeper clawsweeper Bot added proof: sufficient Contributor real behavior proof is sufficient. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. and removed status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. labels Oct 4, 2026
Read status.json beside the same resolved config used by the analytics runtime,
including GITCRAWL_CONFIG. Cover running from an unrelated working directory
with its own receipt, and document the selected-config ownership boundary.

Co-authored-by: Sebastien Tardif <sebtardif@ncf.ca>
@steipete
steipete merged commit c731775 into openclaw:main Oct 7, 2026
7 checks passed
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. proof: sufficient Contributor real behavior proof is sufficient. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants