Repository navigation
fix(analytics): read the discovery receipt beside the resolved config - #231
Conversation
|
🦞👀 Pull request received. I will update this pull request when review starts. ClawSweeper review completeClawSweeper finished reviewing this revision. The review result is being finalized. |
|
Codex review: needs maintainer review before merge. Reviewed October 6, 2026, 9:44 PM ET / October 7, 2026, 01:44 UTC (Revision 4). ClawSweeper reviewWhat this changesAnalytics 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 Review scores
Verification
How this fits togetherGitcrawl 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]
Before mergeNone. Agent review detailsSecurityNone. Review metrics
Technical reviewBest 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. LabelsLabel changes: No label changes. Label justifications:
EvidenceWhat I checked:
Likely related people:
Rating scale
Overall follows the weaker of proof and patch quality. Workflow
HistoryReview history (3 earlier review cycles) |
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>
f5b4582 to
87118b9
Compare
analytics --onceandanalytics --watchlooked for the historical discovery receipt in the working directory when the config came fromGITCRAWL_CONFIG. Resolve the config path using the same rules as the runtime, then readstatus.jsonbeside 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 checkpassed 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 at87118b92521b439a87a47574065bffa07bf40544.Thanks @SebTardif for the report, original fix, and regression test.