fix(portable): avoid archive churn from hydration checkpoints - #197
vincentkoc wants to merge 2 commits into
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: blocked before merge. Reviewed September 13, 2026, 10:11 AM ET / 14:11 UTC. ClawSweeper reviewWhat this changesExclude producer hydration checkpoints from portable archive identity calculations while retaining their rows in exported SQLite files, with regression tests and documentation. Regression provenancePossible regression — suspected (reviewed change). No predecessor PR is attributed. Merge readiness⛔ Blocked before merge - 4 items remain The change remains useful and is absent from main, but changing a shipped identity profile introduces a cross-version archive validation defect. This member-authored PR also requires explicit maintainer handling. Priority: P2 Review scores
Verification
How this fits togetherGitcrawl exports runtime SQLite data into portable archives with an exact file checksum and a semantic identity used to avoid redundant publication. Subscribers recompute the declared identity when validating archives before adoption. flowchart LR
A[Runtime SQLite database] --> B[Portable export]
B --> C[Archive retaining checkpoints]
C --> D[Disposable normalized copy]
D --> E[Semantic identity]
C --> F[Exact checksum]
E --> G[Subscriber validation]
F --> G
Decision needed
Why: Versioning the changed digest preserves an unambiguous contract, but choosing when publishers may emit it requires knowledge of deployed consumers. Before merge
Findings
Agent review detailsSecurityNone. Review metrics
Merge-risk optionsMaintainer options:
Technical reviewBest possible solution: Preserve validation of existing v1 archives and introduce checkpoint-insensitive identity through an explicitly versioned, compatibility-tested producer/subscriber transition. Do we have a high-confidence way to reproduce the issue? Yes, source establishes the trigger: export a database containing the checkpoint table with the old identity policy, then validate its unchanged manifest using this branch. The digests differ and subscriber validation rejects it; this review did not execute that scenario. Is this the best way to solve the issue? No, the exact-table exclusion is appropriately narrow, but changing the meaning of a shipped profile without a compatibility transition is incomplete. Full review comments:
Overall correctness: patch is incorrect AGENTS.md: not found in the target repository. Codex review notes: model internal, reasoning medium; reviewed against 223f23dd3b75. LabelsLabel changes:
Label justifications:
EvidenceWhat I checked:
Likely related people:
Rank-up movesOptional improvements that raise the rating; they are not merge blockers.
Rating scale
Overall follows the weaker of proof and patch quality. Workflow
|
|
Closing as an abandoned approach, not as fixed by #200. The replacement store design derives hydration progress from native database facts and checkpoints, so it no longer needs The native partial-sync repair is merged in #200. The corresponding store integration remains under qualification in https://github.com/openclaw/gitcrawl-store/pull/32; this closure does not claim that integration is merged or deployed. |
What Problem This Solves
Fixes unnecessary portable archive updates when gitcrawl-store persists hydration cursors in its runtime database without changing source facts.
User Impact
Portable exports retain numeric hydration checkpoints for recovery, while cursor-only changes preserve the existing semantic artifact identity. This is a prerequisite for the separate gitcrawl-store checkpoint repair; it does not deploy a producer binary, migrate a live database, or publish a cloud archive.
Why This Change Was Made
Add only
gitcrawl_store_hydration_progressto the existing exact local-bookkeeping exclusion. Unknown tables, source content, exact file hashes, and schema versions retain their existing behavior.The runtime cache remains the durable cursor owner. A publisher may skip a cursor-only export, so the published checkpoint can lag the runtime cache.
Evidence
1749b9632ca25f18dda94cfea1fdaf36dfe5dd0d: no findings.223f23dd3b75d4c5c56bc0d85a9d685acd60bfc0identity implementation, then passed with the repair.go test ./internal/portable -run 'TestSemanticArtifactIdentity|TestCurrentStateSemanticPolicy|TestExportSemanticIdentity|TestExport.*(Identity|Stable)|TestArtifactIdentity' -count=1.git diff --checkpassed. Production delta: +3 lines (one policy entry and its comment); tests: +65 lines. No dependency changes.