Skip to content

feat(store): add an opt-in in-store reader change contract - #275

Merged
hannesrudolph merged 4 commits into
mainfrom
feat/reader-change-contract
Oct 28, 2026
Merged

hannesrudolph merged 4 commits into
mainfrom
feat/reader-change-contract

Conversation

@hannesrudolph

@hannesrudolph hannesrudolph commented Oct 28, 2026 •

Copy link
Copy Markdown
Member

Summary

Add an opt-in, in-store reader change contract (gitcrawl change-contract status|enable|bump|disable). SQLite AFTER triggers on the tables the research reader and history captures read journal every insert, update (including ON CONFLICT DO UPDATE) and delete (including FK cascades) in the writer's own transaction, from every writer binary. Row-mode tables log declared primary keys; table-mode tables advance only a high-water sequence. The log is bounded by a prune trigger; a generation advances when a writable open re-arms tables whose triggers a rebuild removed or on explicit bump. Portable prune and cloud snapshots strip the contract. docs/change-contract.md specifies the reader algorithm (cursor = epoch, generation, schema_version, seq).

No GitHub provider behavior, schema user_version or existing reader contract changes; the contract is absent until enabled.

Fixes after local review

Contract management opens an existing archive without migration and refuses a different schema version. Portable prune and cloud snapshots strip the contract in one transaction. Any surviving partial contract is replaced with a new epoch, keeping its retention. Lowering retention prunes the existing log immediately.

Validation

  • Local ClawSweeper committed-range review of f1c40bc..e3f6234 (final head): complete, no findings, security cleared.
  • go test ./..., go vet ./..., gofmt on tracked files (re-run on the final head), deadcode, make docs, go mod tidy check.
  • Benchmarks: +6.8 µs per row (rows mode), +2.4 µs (table mode), inside one transaction.

Shipped-CLI trace (disposable fixture archive, built binary)

$ gitcrawl change-contract enable   # fixture archive created by store.Open
{'created': True, 'generation': 1, 'armed': True}
$ sqlite3 (a writer unaware of the contract): inserts, hash-conflict upsert, cascade delete
1 repositories op=1 key=1
2 threads op=1 key=1
3 blobs op=1 key=1
4 blobs op=2 key=1
5 thread_revisions op=1 key=7
6 thread_revisions op=3 key=7
7 threads op=3 key=1
$ gitcrawl change-contract status
{'sequence': 7, 'families': {'history.blobs': 4, 'history.pull_request_review_threads': 0, 'history.thread_revisions': 6, 'reader.clusters': 7, 'reader.code': 1, 'reader.coverage': 7}}
$ sqlite3: simulated legacy table rebuild of blobs
{'armed': False, 'generation': 1}
$ gitcrawl change-contract enable (re-arm)
{'rearmed': ['blobs'], 'generation': 2, 'armed': True}
$ gitcrawl portable prune --no-publish on a copy
exit 0; contract objects left: 0
$ gitcrawl change-contract enable on a missing archive
stat sqlite db: stat <fixture>/missing.db: no such file or directory
$ gitcrawl change-contract disable
{'removed': True, 'enabled': False}

Follow-up

Test-only commit e3f6234 covers contract refusal and write-failure paths so the CI coverage floor (85%) holds.

Journal every insert, update and delete of the tables the research reader
and history captures read, with declared row keys, through in-store triggers
that run in the writer's own transaction and from every writer binary.
A bounded log, per-table high-water sequences, families, a generation for
rewrites the triggers cannot observe, and a documented reader algorithm let
a consumer skip unchanged families and rebuild changed ones incrementally.
Writable opens re-arm tables whose triggers a rebuild removed. Portable and
cloud snapshots strip the contract.
…tention at once

Portable prune and cloud snapshots remove contract objects in one
transaction. A partial contract is replaced by a new epoch on enable or
writable open. Lowering retention prunes the existing log and advances
pruned_through in the same transaction.
…hout migrating

Treat any surviving change_contract object as present so partial contracts
are replaced with a new epoch, keeping a valid stored retention. Contract
management opens an existing archive without schema migration or
convergence work and refuses a different schema version.
@clawsweeper

clawsweeper Bot commented Oct 28, 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 clawsweeper:intake:feature Preliminary submission classification; full source review pending. P2 Normal priority bug or improvement with limited blast radius. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. and removed clawsweeper:intake:feature Preliminary submission classification; full source review pending. labels Oct 28, 2026
@clawsweeper

clawsweeper Bot commented Oct 28, 2026

Copy link
Copy Markdown

Codex review: blocked before merge.

What this changes

This PR adds an opt-in SQLite change journal that lets archive readers identify changed tables and rows.

Example: An enabled archive receives a hash-conflict update to blobs row 1.

  • Before: A reader sees database activity without a supported row-level change cursor.
  • After: The journal records blobs row 1 with update operation 2 and advances its sequence.

Review scores

Measure Result What it means
Overall readiness 🦐 gold shrimp (3/6) Useful, coherent implementation with real CLI evidence and no discrete correctness finding, pending persisted-state compatibility evidence and CI classification.
Proof confidence 🦐 gold shrimp (3/6) Real behavior proof is not required for maintainer- or bot-authored pull requests.
Patch quality 🐚 platinum hermit (4/6) No actionable review findings were identified.

Product

Kind: Feature · Worth it: Yes · Fix scope: Complete
User problem: Archive readers rebuild derived state after unrelated database commits because they lack a supported table-level or row-level change cursor.
Reason: The opt-in contract addresses a concrete reader cost, supports unaware writers, and has owner direction through maintainer authorship.

Merge readiness

⛔ Blocked before merge - 1 item remains

Keep open: this maintainer-authored PR adds a useful capability absent from main. No concrete introduced bug was found; the failed coverage checks remain unattributed.

Priority: P2
Reviewed head: 3b603838fb8e23638784b769eee74547df2b56aa

Before merge

  • Add data-model compatibility proof - The review found that existing stored data may not work after upgrade. Show that existing data still loads and works with this change.

Findings

None.

Tests

  • Missing end-to-end proof: Supply a redacted v0.17.0-to-head CLI upgrade trace showing existing settings and archive data intact through enable, writer activity, and disable.
Agent review details

How this fits together

Gitcrawl stores GitHub archive data in SQLite; database triggers turn committed writer mutations into bounded change records for read-only consumers, while portable and cloud publication remove the local contract.

flowchart LR
  A[Gitcrawl and SQLite writers] --> B[Archive tables]
  B --> C[Transactional change triggers]
  C --> D[Bounded journal and cursor]
  D --> E[Read-only consumers]
  B --> F[Portable and cloud copies]
  F --> G[Contract removed before publication]
Loading

Technical review

Best possible solution:

Land the opt-in contract with existing archive contents preserved and contract state excluded from published copies.

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

This is a feature request implemented by a patch; the supplied CLI trace demonstrates its core behavior on a disposable fixture.

Is this the best way to solve the issue?

Database triggers are an appropriate owner for changes made by unaware writers, and the implementation keeps management, repair, and stripping within existing storage and publication boundaries.

AGENTS.md: found and applied where relevant.

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

Provenance checked

  • Writable store opening keeps the original intent (3e1655f: The existing opener owns schema migration; the addition repairs enabled contracts afterward and leaves maintenance commands on a separate non-migrating open.)
  • Portable pruning and cloud sanitization keeps the original intent (fix(cloud): preserve full PR patch fidelity #131: Published archives preserve appropriate canonical content while excluding local diagnostics and state; stripping the local contract follows that boundary and retains owner-exclusion checks.)
  • CLI dispatch and help keeps the original intent (refactor: organize CLI commands by responsibility #188: Command implementations are organized by responsibility; the new command follows the existing dispatch and dedicated-file structure.)

Testing

Proof path: shipped entry point.

Security

None.

Evidence

What I checked:

  • Introduced implementation: The pinned introduced diff adds the contract, CLI, documentation, tests, writable-open repair, and publication stripping; main has no equivalent contract implementation. (internal/store/change_contract.go:253, 3b603838fb8e)
  • Owner direction and supplied behavior evidence: GitHub confirms MEMBER authorship. The complete body supplies a built-CLI fixture trace covering enable, an unaware SQLite writer, upsert and cascade tracking, re-arming, portable prune, missing-file rejection, and disable; neither discussion comment adds validation. (3b603838fb8e)
  • Compatibility boundary: The new maintenance opener checks the existing schema version without migration; enable creates separate contract tables, and publication removes them. Both v0.17.0 and the reviewed branch use archive schema version 16, but the supplied trace starts with a newly created fixture. (internal/store/store.go:85, 3b603838fb8e)
  • Unclassified CI failures: Both Go jobs failed at Test with coverage after vet, vulnerability scanning, and deadcode passed. Annotations contain only exit code 1; log retrieval redirects to a blocked storage host, so neither test failure nor coverage-floor failure can be attributed to this diff. See https://github.com/openclaw/gitcrawl/actions/runs/40712358576. (.github/workflows/ci.yml:74, 3b603838fb8e)
  • Repository policy and checkout custody: The complete root AGENTS.md was read; no nested AGENTS.md, maintainer notes directory, or pr-simplify skill was found. The explicit review instruction supersedes helper execution. No helper, second reviewer, tests, or target code ran; status remained clean and registered worktrees were inspected without changes. (AGENTS.md:1, 3b603838fb8e)
  • Feature history and related work: History identifies Vincent Koc's original store and portable pruning, Peter Steinberger's storage/publication refactors, and Hannes Rudolph's merged analytics work at Collect analytics evidence with durable review-state recovery #216. Searches for reader, change contract, data_version, and invalidation found no canonical replacement. Following cloud_sqlite.go encountered an unavailable historical object; available path history and supplied provenance cover the changed boundary. (internal/cli/cloud_sqlite.go:117, f1c40bc03f86)

Likely related people:

  • Vincent Koc: Raw commit 3e1655f adds internal/store/store.go:60 relative to its recorded parents. This identifies author metadata, not feature responsibility or a PR merger. (role: source-line author; confidence: high; commits: 3e1655ff5aca; files: internal/store/store.go)
  • Peter Steinberger: Raw commit 63e9556 adds internal/store/store.go:46 relative to its recorded parents. This identifies author metadata, not feature responsibility or a PR merger. (role: source-line author; confidence: high; commits: 63e95564d9f9; files: internal/store/store.go)
  • Hannes Rudolph: Raw commit 84ed753 adds internal/cli/cloud_sqlite.go:102 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/cloud_sqlite.go)

Labels

Label changes:

  • add P2: Useful optional archive-reader functionality with limited scope and no demonstrated urgent user regression.
  • add rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🦐 gold shrimp and patch quality is 🐚 platinum hermit.

Label justifications:

  • P2: Useful optional archive-reader functionality with limited scope and no demonstrated urgent user regression.
  • rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🦐 gold shrimp and patch quality is 🐚 platinum hermit.

Rank-up moves

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

  • Demonstrate the contract lifecycle on a copy of a v0.17.0 archive while verifying existing settings and data remain intact.

Rating scale

6/6 🦀 challenger crab · 5/6 🦞 diamond lobster · 4/6 🐚 platinum hermit · 3/6 🦐 gold shrimp · 2/6 🦪 silver shellfish · 1/6 🧂 unranked krab. Overall follows the weaker of proof and patch quality; ✨ marks media proof (a screenshot, video, or linked artifact) that directly shows the changed behavior.

Workflow

ClawSweeper edits this one comment on every review. Comment @clawsweeper re-review for a fresh review only. Repair and merge execution lanes are retired.

Reviewed October 28, 2026, 11:18 AM ET / 15:18 UTC.

@hannesrudolph
hannesrudolph merged commit 033ab00 into main Oct 28, 2026
13 checks passed
@hannesrudolph
hannesrudolph deleted the feat/reader-change-contract branch October 28, 2026 15:28
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.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant