Skip to content

fix(portable): preserve schema in current-state exports - #149

Merged
obviyus merged 1 commit into
mainfrom
fix/portable-invariant-schema
Aug 10, 2026
Merged

fix(portable): preserve schema in current-state exports#149
obviyus merged 1 commit into
mainfrom
fix/portable-invariant-schema

Conversation

@obviyus

@obviyus obviyus commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

Problem

The exporter deleted empty tables to save space, but gitcrawl status still queries one of those tables. Read-only clients cannot recreate it, so status failed with no such table: cluster_groups.

Fix

  • keep the current-state SQLite schema intact
  • clear excluded rows instead of dropping their tables
  • remove the obsolete table-drop path

Regression proof

The new read-only status test fails on origin/main:

read exported database status: count clusters: SQL logic error: no such table: cluster_groups (1)

The same test passes on this branch.

Real behavior proof

Built the current client from 28d2aed1e73e, then generated a fresh current-state export from a 55,808-thread archive. Export validation reported quick_check: ok, integrity_check: ok, zero foreign-key violations, and 93,102,080 bytes against the 99,999,999-byte budget.

The artifact was made filesystem read-only before either status command. Paths are redacted; JSON is narrowed to status fields.

$ stat -c 'mode=%a bytes=%s' ARTIFACT/gitcrawl.db
mode=444 bytes=93102080
$ sha256sum ARTIFACT/gitcrawl.db
33635324e628b007db3b85e1a266156327367bf04a19fa776f05ecb48fa78edf  ARTIFACT/gitcrawl.db

$ gitcrawl-v0.8.4 --version
0.8.4
$ gitcrawl-v0.8.4 status --json | jq '{state,summary,database_bytes,counts}'
{
  "state": "current",
  "summary": "55808 threads across 1 repositories",
  "database_bytes": 93102080,
  "counts": [
    {"id":"repositories","label":"Repositories","value":1},
    {"id":"threads","label":"Threads","value":55808},
    {"id":"open_threads","label":"Open threads","value":18012},
    {"id":"clusters","label":"Clusters","value":0}
  ]
}

$ gitcrawl-current status --json | jq '{state,summary,database_bytes,counts}'
{
  "state": "current",
  "summary": "55808 threads across 1 repositories",
  "database_bytes": 93102080,
  "counts": [
    {"id":"repositories","label":"Repositories","value":1},
    {"id":"threads","label":"Threads","value":55808},
    {"id":"open_threads","label":"Open threads","value":18012},
    {"id":"clusters","label":"Clusters","value":0}
  ]
}

$ sha256sum ARTIFACT/gitcrawl.db
33635324e628b007db3b85e1a266156327367bf04a19fa776f05ecb48fa78edf  ARTIFACT/gitcrawl.db

The unchanged hash proves both clients opened the fresh export without migration.

Validation

  • GOWORK=off go test ./internal/portable ./internal/store -count=1
  • GOWORK=off go vet ./internal/portable
  • git diff --check

The full suite reaches two pre-existing stale-index-lock failures; both reproduce on untouched origin/main.

@clawsweeper

clawsweeper Bot commented Aug 10, 2026

Copy link
Copy Markdown

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

@clawsweeper clawsweeper Bot added merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. 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 Aug 10, 2026
@clawsweeper

clawsweeper Bot commented Aug 10, 2026

Copy link
Copy Markdown

Codex review: needs maintainer review before merge. Reviewed August 10, 2026, 8:37 AM ET / 12:37 UTC.

ClawSweeper review

What this changes

The PR preserves empty SQLite table schemas in current-state portable exports while clearing their excluded rows, so read-only clients can report status.

Regression provenance

Possible regression — probable (reproduction; reviewed change). No predecessor PR is attributed.

Merge readiness

⚠️ Ready for maintainer review - 1 item remains

Keep this PR open for normal merge review: it fixes a current-main portable-export defect, resolves the prior changelog finding, and now includes strong read-only real-behavior proof.

Priority: P2
Reviewed head: 28d2aed1e73e4fcc125108ad431251f59e7e3fdd

Review scores

Measure Result What it means
Overall readiness 🐚 platinum hermit (4/6) A focused, well-covered compatibility repair with convincing real export evidence.
Proof confidence 🦞 diamond lobster (5/6) Sufficient (terminal): The PR body provides redacted terminal evidence from a large fresh read-only export, including successful after-fix status from a released and current client plus unchanged artifact hashes.
Patch quality 🐚 platinum hermit (4/6) No actionable review findings were identified.

Verification

Check Result Evidence
Real behavior Verified Sufficient (terminal): The PR body provides redacted terminal evidence from a large fresh read-only export, including successful after-fix status from a released and current client plus unchanged artifact hashes.
Evidence reviewed 4 items Current-main failure mechanism: The current profile drops cluster_groups, while status always counts that table through the generated query layer; a read-only exported database therefore cannot recreate the missing schema.
Narrow repair and regression coverage: The branch clears the static excluded-table list rather than dropping it, and adds a regression test that opens the export read-only and successfully calls status.
Real after-fix behavior proof: The PR body records a fresh 55,808-thread export made read-only, successful status output from both v0.8.4 and the current client, integrity checks, and an unchanged SHA-256 before and after both reads.
Findings None None.
Security None None.

How this fits together

Portable export turns a live Gitcrawl SQLite archive into a compact, shareable current-state database. The resulting database is later opened by read-only Gitcrawl clients, including the status command.

flowchart LR
  A[Live archive database] --> B[Portable exporter]
  B --> C[Current-state profile]
  C --> D[Clear excluded history rows]
  D --> E[Validated compact export]
  E --> F[Read-only status command]
Loading

Before merge

  • Resolve merge risk (P1) - Artifacts generated before this fix remain missing the schema and must be regenerated to make read-only status work.
Agent review details

Security

None.

Review metrics

Metric Value Why it matters
Patch scope 1 production file and 2 test files; 48 added, 63 removed The branch is focused on one portable-export policy change and its regression coverage.

Merge-risk options

Maintainer options:

  1. Decide the mitigation before merge
    Land the schema-preserving export repair and regenerate affected current-state portable artifacts.
  2. Pause or close
    Do not merge this PR until maintainers decide whether the risk is worth taking.

Technical review

Best possible solution:

Land the schema-preserving export repair and regenerate affected current-state portable artifacts.

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

Yes: current main drops cluster_groups, while read-only status unconditionally counts it; the branch’s focused regression test reproduces that exact path.

Is this the best way to solve the issue?

Yes: retaining the established schema while deleting excluded rows preserves the compact-data policy without requiring read-only clients to migrate the export.

AGENTS.md: not found in the target repository.

Codex review notes: model internal, reasoning high; reviewed against ff6b2b41861d.

Labels

Label changes:

  • add proof: sufficient: Contributor real behavior proof is sufficient. The PR body provides redacted terminal evidence from a large fresh read-only export, including successful after-fix status from a released and current client plus unchanged artifact hashes.
  • add rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🦞 diamond lobster and patch quality is 🐚 platinum hermit.
  • add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (terminal): The PR body provides redacted terminal evidence from a large fresh read-only export, including successful after-fix status from a released and current client plus unchanged artifact hashes.
  • remove status: 📣 needs proof: Current PR status label is status: 👀 ready for maintainer look.
  • remove rating: 🦪 silver shellfish: Current PR rating is rating: 🐚 platinum hermit, so this older rating label is no longer current.
  • remove merge-risk: 🚨 compatibility: Current PR review selected no merge-risk labels.

Label justifications:

  • P2: Current-state exports on main can make the normal read-only status workflow fail, but the defect has a bounded compatibility repair.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🦞 diamond lobster 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 PR body provides redacted terminal evidence from a large fresh read-only export, including successful after-fix status from a released and current client plus unchanged artifact hashes.
  • proof: sufficient: Contributor real behavior proof is sufficient. The PR body provides redacted terminal evidence from a large fresh read-only export, including successful after-fix status from a released and current client plus unchanged artifact hashes.

Evidence

What I checked:

  • Current-main failure mechanism: The current profile drops cluster_groups, while status always counts that table through the generated query layer; a read-only exported database therefore cannot recreate the missing schema. (internal/portable/export.go:51, ff6b2b41861d)
  • Narrow repair and regression coverage: The branch clears the static excluded-table list rather than dropping it, and adds a regression test that opens the export read-only and successfully calls status. (internal/portable/export.go:323, 28d2aed1e73e)
  • Real after-fix behavior proof: The PR body records a fresh 55,808-thread export made read-only, successful status output from both v0.8.4 and the current client, integrity checks, and an unchanged SHA-256 before and after both reads. (28d2aed1e73e)
  • Feature provenance: Current-state export, including the table-drop policy, entered main in the portable-export feature commit; later main commits also maintained this export path. (internal/portable/export.go:51, e5e424200c3d)

Likely related people:

  • Peter Steinberger: Introduced the portable current-state export and authored the two subsequent main-branch artifact-identity fixes in the same subsystem. (role: introduced current-state export and recent area contributor; confidence: high; commits: e5e424200c3d, 584420b71e66, ff6b2b41861d; files: internal/portable/export.go)

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 (1 earlier review cycle)
  • reviewed 2026-08-10T12:30:20.408Z sha cd8f1d0 :: needs real behavior proof before merge. :: [P3] Remove the release-owned changelog entry

@obviyus
obviyus force-pushed the fix/portable-invariant-schema branch from cd8f1d0 to 28d2aed Compare August 10, 2026 12:33
@obviyus

obviyus commented Aug 10, 2026

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Aug 10, 2026

Copy link
Copy Markdown

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: when the review finishes, ClawSweeper will create the durable review comment if needed or update the existing comment in place.

Re-review progress:

@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. merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. labels Aug 10, 2026
@obviyus
obviyus merged commit e61e438 into main Aug 10, 2026
18 checks passed
@obviyus
obviyus deleted the fix/portable-invariant-schema branch August 10, 2026 12:50
@obviyus

obviyus commented Aug 10, 2026

Copy link
Copy Markdown
Contributor Author

Landed via squash onto main.

  • Scoped tests: GOWORK=off go test ./internal/portable ./internal/store -count=1; GOWORK=off go vet ./internal/portable; git diff --check
  • Changelog: skipped; release-owned
  • Land commit: 28d2aed1e73e4fcc125108ad431251f59e7e3fdd
  • Merge commit: e61e438b6db0e7d7b60193931dec962290c55609

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.

1 participant