sync --include-comments downloads comments again for every open thread, even when the thread has not changed

[vlsi]

Summary

I keep a local archive of pgjdbc/pgjdbc (489 open issues and pull requests) and want to refresh it every week or month. Each gitcrawl sync owner/repo --include-comments run downloads the comments of every open thread again, including threads whose updated_at has not changed since the previous run. With --with pr-details the same happens to files, commits, checks, and review threads of every open pull request. A periodic refresh therefore costs about as many API requests as the first backfill.

Reproduction

gitcrawl 0.9.6, a fresh database, two runs back to back:

export GITCRAWL_CONFIG=$PWD/gc/config.toml GITCRAWL_DB_PATH=$PWD/gc/gc.db
gitcrawl init --db $PWD/gc/gc.db --json
gitcrawl sync openclaw/gitcrawl --include-comments --progress-file $PWD/gc/p1.json --json
gitcrawl sync openclaw/gitcrawl --include-comments --progress-file $PWD/gc/p2.json --json

Progress file after the second run:

{"schema":"gitcrawl.sync-progress.v1","repository":"openclaw/gitcrawl","state":"succeeded","stage":"finalizing","issues_received":0,"pull_requests_received":1,"comments_received":2,"observed_at":"2026-09-13T15:03:25.977678Z"}

The one open thread is #197. Its updated_at did not change between the runs, and the database already held its comments at that updated_at before the second run started:

$ sqlite3 gc/gc.db "select t.number, t.updated_at_gh, r.family, r.source_updated_at from threads t join thread_child_observation_reservations r on r.thread_id = t.id where t.state = 'open'"
197|2026-09-13T14:11:55Z|comments|2026-09-13T14:11:55Z

On this repository the second run fetches 2 comments. On a repository with a few hundred open threads, it fetches the comments of all of them. I have not measured the request count on pgjdbc/pgjdbc itself.

Expected

When a thread's updated_at in the list response equals the source_updated_at recorded for a child family in thread_child_observation_reservations, sync does not request that family for the thread. In the reproduction above, the second run reports "comments_received": 0.

A way to force a full re-download (for example, a flag) stays available for data that can change without moving updated_at.

Actual

Syncer.Sync requests comments, review threads, and pull request details for every row in the list response (internal/syncer/syncer.go, the loop over rows before persist). The comparison with thread_child_observation_reservations happens only afterwards, in ReserveThreadChildObservation, when the data is written. Unchanged data is discarded after it has been downloaded.

Possible approach

This part is a suggestion; the report above stands without it.

1. Before the download loop, read ThreadChildObservation for each thread and family, and skip the request when source_updated_at equals the row's updated_at.
2. Check runs and workflow runs probably need a different key, such as the head SHA. As far as I know, a new CI run does not change a pull request's updated_at; I have not verified this.
3. Optionally, fetch the open list with since taken from a watermark, as the closed sweep already does with closed_sweep_through. After fix: avoid stalled embedding batches and stale open threads #175, threads missing from a response are not treated as closed or deleted.

Workaround

--state all --since <date of the previous sync> limits the list to threads updated after that date, so only those are hydrated. The caller has to track the date, and a run with --since does not advance closed_sweep_through.

Related

I found no existing issue or pull request about this (searched open and closed items for since, incremental, watermark, and comments hydration). #175 fixed missed closures after an offline period, but it does not change which threads are hydrated.

[clawsweeper]

Codex review: keeping this open for maintainer follow-up; there is still a little grit to resolve. Reviewed September 13, 2026, 11:10 AM ET / 15:10 UTC.

Summary
The repeated downloads are source-confirmed and remain on current main. This is a useful optimization request, but the proposed timestamp shortcut changes freshness semantics: stored observations order writes rather than certify that child data is unchanged. Neither related PR implements download reuse.

Reproducibility: yes. the repeated-fetch behavior is source-reproducible: two syncs returning the same thread invoke the selected child endpoints both times. The reporter supplied a v0.9.6 transcript; no sync or tests were executed during this read-only review.

Maintainer decision needed

Question	Recommendation
Should automatic child-download reuse change default sync freshness, or begin as an explicit opt-in mode?	Start with opt-in reuse: Introduce a bounded reuse mode with family-specific validation while preserving existing default hydration and an explicit full-refresh path.

Why: Current hydration promises fresh child observations, while the requested shortcut trades that guarantee for fewer API requests; existing timestamps do not settle the safe invalidation policy.

Next step
Choose the freshness and force-refresh contract before implementation; this changes supported hydration semantics and is not a narrow existing-behavior repair.

Review details

Best possible solution:

Provide conservative, family-specific download reuse with explicit full refresh, preserving observation ordering and refusing to mark unvalidated cached evidence as freshly hydrated.

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

Yes, the repeated-fetch behavior is source-reproducible: two syncs returning the same thread invoke the selected child endpoints both times. The reporter supplied a v0.9.6 transcript; no sync or tests were executed during this read-only review.

Is this the best way to solve the issue?

No, comparing only parent updated_at is not yet a safe general solution. Reservations enforce write ordering, so download reuse needs independent freshness validation and explicit handling of skipped families.

AGENTS.md: not found in the target repository.

Remaining risk / open question:

• A parent timestamp or unchanged head SHA alone does not establish that every child collection is current; the requested reuse policy needs family-specific invalidation guarantees.
• Skipping fetches without changing persistence bookkeeping could treat unfetched collections as empty or incorrectly certify complete revision evidence.

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

Label changes

Label changes:

• add P2: Repeated hydration increases periodic archive refresh cost, with a supported time-window workaround and no demonstrated outage.
• add impact:other: The concrete impact is unnecessary GitHub API traffic and refresh latency.
• add issue-rating: 🦞 diamond lobster: Current issue advisory state selects this label.
• add clawsweeper:source-repro: Current issue advisory state selects this label.
• add clawsweeper:no-new-fix-pr: Current issue advisory state selects this label.
• add clawsweeper:needs-maintainer-review: Current issue advisory state selects this label.
• add clawsweeper:needs-product-decision: Current issue advisory state selects this label.

Label justifications:

• P2: Repeated hydration increases periodic archive refresh cost, with a supported time-window workaround and no demonstrated outage.
• impact:other: The concrete impact is unnecessary GitHub API traffic and refresh latency.

Evidence reviewed

What I checked:

• Current fetch path confirms repeated retrieval: Every returned thread enters the hydration loop. IncludeComments directly calls fetchCommentRows; IncludePRDetails fetches review threads and full PR details. There is no stored-observation check before these requests. (internal/syncer/syncer.go:205, 223f23dd3b75)
• Comments reach the real HTTP request boundary: Comment and review methods paginate their GitHub endpoints. The HTTP request path dispatches without an ETag or If-Modified-Since cache check, so the repeated sync calls are not absorbed by an existing conditional-request cache. (internal/github/client.go:313, 223f23dd3b75)
• Observation reservations are ordering fences: ReserveThreadChildObservation compares source timestamps and observation sequences. Equal source timestamps with a newer sequence can be accepted; the report's claim that unchanged timestamps cause downloaded data to be discarded is therefore not generally correct. (internal/store/observation_order.go:53, 223f23dd3b75)
• Tests explicitly accept newer same-timestamp observations: The child-family reservation test accepts PR details at sequence 4 after sequence 2 with the same source timestamp. This establishes that timestamp equality alone is not the current freshness contract. (internal/store/observation_order_test.go:227, 223f23dd3b75)
• Existing controls and evidence contract: The sync documentation supports --since for bounded listing, --numbers for exact refresh, and selective hydration. It also requires complete discussion and PR detail evidence for revisions and tracks workflow/check transitions. The reporter's --state all --since workaround is supported, but automatic child-download reuse is not documented. (docs/sync.md:33, 223f23dd3b75)
• Reported release and main retain the same sync implementation: GitHub reports v0.9.6 as the latest release. Its internal/syncer/syncer.go blob is c0512d140a7fee9771b8b8603f5ac4a31a8d472b, matching git ls-tree HEAD. A live branch read also confirms the supplied main SHA. (internal/syncer/syncer.go:205, 223f23dd3b75)

Likely related people:

• vincentkoc: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)
• steipete: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)

How this review workflow works

• 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.

[steipete]

Fixed in #203, squash-merged as efaeb1f.

Unchanged general-discussion comments on issues and PRs are now reused when fresh parent timestamps, comment counts, exact completed membership, and retained payloads agree. Missing, pruned, changed, or failed observations download again. sync --force and refresh --force bypass reuse; selected PR review and detail data remain live.

Built-artifact proof against ten unchanged open PRs in the real public pgjdbc/pgjdbc repository: total requests fell from 32 to 22 and issue-comment requests from 10 to 0. --force restored 32 / 10. A repeat sync of this issue made no comment request and reported comments_received: 0.

Regression coverage includes third-sync reuse, exact live membership, equivalent timestamps, failed-force recovery, concurrent replacement, and portable-prune restoration. Both the dropped-membership and timestamp-string-equality mutants are rejected. All local gates passed with 85.5% coverage; the CLI suite needed a longer timeout on the loaded local host. Exact-head CI passed on 20e3148, and independent reviews were clean through P2.

Thanks @vlsi for the report, reproduction, and mutation testing. The change remains in Unreleased; no patch release was published.