fix(github): preserve large numeric IDs during sync

[vincentkoc]

What Problem This Solves

Fixes an issue where users synchronizing GitHub data could archive rounded numeric IDs, merge distinct comments under one identity, or fail sync with a duplicate workflow-run error when IDs exceed floating-point precision.

Why This Change Was Made

Preserve JSON numbers at the GitHub acquisition boundary before untyped response maps reach the existing sync mappers and SQLite storage. One private decoder covers REST objects, paginated lists, envelope collections, and inner GraphQL data; it retains existing error wrapping, response closure, pagination, typed rate-limit decoding, and trailing-data behavior.

No mapper, schema, dependency, configuration, or workflow changes. Production delta is +11/-4 (net +7) for the shared lossless decode boundary; tests add 529 lines.

User Impact

Newly fetched repository, issue/PR, comment, review, and workflow IDs retain their exact numeric values, including adjacent IDs above the floating-point precision limit.

Previously stored suspect IDs remain unknown and unrepaired. This change does not rewrite existing archives, decode opaque GraphQL IDs, delete rows, or make a blanket resync a historical-repair procedure.

Evidence

• Before the fix, literal 9007199254740993 decoded as 9007199254740992; real HTTP-client-to-SQLite sync failed with workflow snapshot contains duplicate run 9007199254740992.
• After the fix, literal boundary IDs 9007199254740991, 9007199254740992, 9007199254740993, and 9223372036854775807 remain exact across REST and GraphQL paths.
• Regression coverage includes nested data, second pages, GraphQL thread/comment pagination, opaque IDs, boolean/line/counter fields, malformed/null/trailing-data behavior, all three comment families, workflow IDs, exact raw JSON, and repeated-sync row identity/count stability.
• GOWORK=off GOPROXY=off go test ./internal/github ./internal/syncer -run '^TestProviderIDs' -count=1: PASS.
• GOWORK=off GOPROXY=off go test ./internal/github ./internal/syncer -count=1: PASS (GitHub 5.448s; syncer 4.006s).
• git diff --check and scoped gofmt -l: PASS.
• Independent exact-patch review: PASS before commit.

No release, runtime deployment, or production archive mutation is included.

[clawsweeper]

🦞👀
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]

Codex review: needs changes before merge. Reviewed September 9, 2026, 7:05 AM ET / 11:05 UTC.

ClawSweeper review

What this changes

Preserves exact GitHub numeric IDs during REST and GraphQL syncs and adds decoding and SQLite persistence regression coverage.

Merge readiness

⛔ Needs changes before merge - 1 item remains

The fix remains necessary: main and v0.9.5 still decode provider IDs through floating point. No blocking correctness defect was found in this focused patch.

Priority: P2
Reviewed head: a20319e892cae334c172747f2a61036e709690a9

Review scores

Measure	Result	What it means
Overall readiness	🐚 platinum hermit (4/6)	A focused repair with compatible existing mappers, extensive regression coverage, and no identified blocking defect.
Proof confidence	🌊 off-meta tidepool	Not applicable: The MEMBER-authored PR is exempt from ordinary contributor proof. Its reported passing tests exercise the production GitHub client and syncer against local HTTP fixtures and SQLite; they were inspected but not rerun.
Patch quality	🐚 platinum hermit (4/6)	No actionable review findings were identified.

Verification

Check	Result	Evidence
Real behavior	Not applicable	Not applicable: The MEMBER-authored PR is exempt from ordinary contributor proof. Its reported passing tests exercise the production GitHub client and syncer against local HTTP fixtures and SQLite; they were inspected but not rerun.
Evidence reviewed	9 items	Introduced production change: The pinned base-to-head diff adds UseNumber at four untyped decoding boundaries; it changes no schema, dependency, configuration, authentication, or workflow. Existing mapper compatibility: jsonID already preserves json.Number.String(), intValue accepts json.Number, and mustJSON marshals the response maps directly. Comment persistence uses the resulting string provider ID. Source-backed failure mechanism: Workflow snapshot validation keys rows by jsonID and rejects duplicates, so adjacent numeric IDs collapsed by floating-point decoding can reject an otherwise valid snapshot.
Findings	None	None.
Security	None	None.

How this fits together

Gitcrawl fetches GitHub repository, discussion, and workflow data into a local SQLite archive. Its JSON decoder feeds sync mappers that assign provider identities and preserve raw responses.

flowchart LR
  A[GitHub REST responses] --> C[Lossless JSON decoding]
  B[GitHub GraphQL responses] --> C
  C --> D[Sync field mapping]
  D --> E[SQLite archive]
  E --> F[Local search and exports]

Loading

Before merge

• Complete next step (P2) - Mark the draft ready for review when the author intends it to land.

Agent review details

Security

None.

Review metrics

Metric	Value	Why it matters
Production and test delta	Production +11/-4; tests +529; changelog +2	Seven net production lines centralize lossless decoding, with broad boundary and persistence coverage.

Technical review

Best possible solution:

Keep lossless decoding at the acquisition boundary and retain the explicit separation between preventing new corruption and repairing historical archives.

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

Yes, from source: numeric IDs 9007199254740992 and 9007199254740993 collapse during current-main decoding and then collide in workflow snapshot validation. This review did not execute the regression.

Is this the best way to solve the issue?

Yes. A shared UseNumber decoder prevents precision loss before the existing compatible mappers and storage receive data, without introducing a competing ID conversion path.

AGENTS.md: not found in the target repository.

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

Labels

Label changes:

• add P2: This bounded correctness fix prevents identity collisions and sync failures for numeric IDs above floating-point precision.
• add rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🌊 off-meta tidepool and patch quality is 🐚 platinum hermit.
• add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Not applicable: The MEMBER-authored PR is exempt from ordinary contributor proof. Its reported passing tests exercise the production GitHub client and syncer against local HTTP fixtures and SQLite; they were inspected but not rerun.

Label justifications:

• P2: This bounded correctness fix prevents identity collisions and sync failures for numeric IDs above floating-point precision.
• rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🌊 off-meta tidepool and patch quality is 🐚 platinum hermit.
• status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Not applicable: The MEMBER-authored PR is exempt from ordinary contributor proof. Its reported passing tests exercise the production GitHub client and syncer against local HTTP fixtures and SQLite; they were inspected but not rerun.

Evidence

What I checked:

• Introduced production change: The pinned base-to-head diff adds UseNumber at four untyped decoding boundaries; it changes no schema, dependency, configuration, authentication, or workflow. (internal/github/client.go:455, a20319e892ca)
• Existing mapper compatibility: jsonID already preserves json.Number.String(), intValue accepts json.Number, and mustJSON marshals the response maps directly. Comment persistence uses the resulting string provider ID. (internal/syncer/syncer.go:1088, a20319e892ca)
• Source-backed failure mechanism: Workflow snapshot validation keys rows by jsonID and rejects duplicates, so adjacent numeric IDs collapsed by floating-point decoding can reject an otherwise valid snapshot. (internal/syncer/pull_details_more.go:61, a20319e892ca)
• Regression coverage inspected: Both added test files were read completely. They exercise four boundary IDs, nested values, REST and GraphQL pagination, decoding errors, raw JSON persistence, and two complete HTTP-client-to-SQLite sync passes with stable identities and counts. Tests were not executed during this read-only review. (internal/syncer/provider_ids_test.go:18, a20319e892ca)
• Release still needs the fix: v0.9.5 resolves to the recorded release SHA and still uses ordinary JSON decoding for paginated maps. The live main ref remains the pinned base ad1b6f2. (internal/github/client.go:379, 32e0e4b826c9)
• Historical data scope: The captured PR body and changelog explicitly limit the fix to newly fetched data and exclude repair of previously rounded IDs. Existing string identity columns and storage formats remain unchanged. (CHANGELOG.md:5, a20319e892ca)

Likely related people:

• vincentkoc: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)
• SebTardif: 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.