Skip to content

Rename sync-incremental to sync, fix sync elapsed time, TUI and build polish#2

Merged
wesm merged 10 commits into
mainfrom
sync-bugfix
Feb 2, 2026
Merged

Rename sync-incremental to sync, fix sync elapsed time, TUI and build polish#2
wesm merged 10 commits into
mainfrom
sync-bugfix

Conversation

@wesm
Copy link
Copy Markdown
Member

@wesm wesm commented Feb 2, 2026

Summary

  • Rename sync-incremental command to sync for brevity (sync-incremental kept as hidden alias)
  • Fix sync elapsed time, TUI cache rebuild, and release build polish

Test plan

  • msgvault sync you@gmail.com works
  • msgvault sync-incremental you@gmail.com still works (alias)
  • msgvault --help shows sync not sync-incremental

🤖 Generated with Claude Code

wesm and others added 2 commits February 2, 2026 10:27
- Fix CLIProgress showing ~2562047h elapsed during incremental sync
  because startTime was never initialized (OnStart not called)
- Always do full cache rebuild on TUI launch when changes detected,
  since incremental rebuilds can produce incorrect results
- Quote $GITHUB_SHA in release workflow
- Hide "unknown" version in TUI title bar

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
The shorter name is easier to type for the most common operation.
sync-incremental is preserved as a hidden alias for backward compatibility.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@wesm wesm changed the title Fix sync elapsed time, TUI cache rebuild, and release build polish Rename sync-incremental to sync, fix sync elapsed time, TUI and build polish Feb 2, 2026
wesm and others added 8 commits February 2, 2026 10:34
- Rename file to match the new command name
- OnStart unconditionally resets startTime for reuse scenarios
- OnLatestDate initializes startTime when called before OnStart
- Add CLIProgress unit tests for callback ordering edge cases

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
When OnStart is never called (incremental sync path), lastPrint was
zero so the throttle check always passed, and elapsed was near-zero
producing absurd rates like 120000000.0/s. Initialize lastPrint
alongside startTime in OnProgress/OnLatestDate fallbacks, and guard
rate calculation against sub-second elapsed time.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Runs make test and make lint on macOS (matching the primary dev
environment and CGO/DuckDB build requirements) for all PRs and
pushes to main.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Newer versions enable stricter checks by default, causing spurious
lint failures.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Brew doesn't support versioned formulae for golangci-lint.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
The pre-built binary was compiled with Go 1.24 which refuses to lint
Go 1.25 code. Building from source with go install uses the CI's
Go 1.25 toolchain.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Check error returns flagged by errcheck
- Remove unused headerStyle and hasPrerelease
- Suppress intentionally ignored errors with _ =

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@wesm wesm merged commit 048f163 into main Feb 2, 2026
1 check passed
@wesm wesm deleted the sync-bugfix branch February 2, 2026 17:11
jesserobbins added a commit to jesserobbins/msgvault-1 that referenced this pull request Apr 23, 2026
…ilure

Two resume-path corrections:

- A fbmessengerCheckpoint saved mid-way through the first thread has
  ThreadIndex == 0, which the prior resume guard (`if prior.ThreadIndex > 0`)
  treated as "no progress yet" and ignored. Functionally this didn't lose
  data (source_message_id dedup made retry safe and the outer loop already
  starts at threadIdx=0) but summary.WasResumed stayed false, the "resuming"
  log didn't fire, and cumulative counters from the prior run didn't carry
  forward — so the summary undercounted and the CLI didn't tell the user a
  resume was happening. Gate removed; any well-formed checkpoint with a
  matching RootDir is now resumable. Added
  TestImportDYI_ResumeFromFirstThreadCheckpoint.

- The outer thread loop previously advanced the per-thread checkpoint to
  threadIdx+1 even after a hard error on that thread, so a resumed run
  would skip it. A transient failure (DB lock, I/O blip) should retry on
  the next run; source_message_id dedup keeps retry safe for any messages
  already written. On hard error we now log, flag HardErrors=true, and
  `continue` without advancing the checkpoint. A persistent error will
  surface via HardErrors=true across runs and the user can rerun with
  --no-resume if they want to force past it.

Addresses roborev combined review (6a28ac0) Medium findings kenn-io#2 and kenn-io#3.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
sternryan referenced this pull request in sternryan/msgvault May 6, 2026
- TestSourcesDB_OpenAndLookup: fresh URL scores 1.0, ingested URL scores 0.0
- TestSourcesDB_ReadOnly: INSERT against opened handle errors out

Confirms criterion #2 (URL gold) end-to-end with mattn/go-sqlite3 mode=ro
on a real file-backed sources.db. The five lexical scorers (urlgold,
curiosity, recurrence, decision, expert) were implemented in Task 1
scorer.go alongside Composite — Task 2b only needs the sources.db
integration test (per plan §14-02-02b).
webgress added a commit to webgress/msgvault that referenced this pull request May 22, 2026
External review caught three SQL shapes in internal/query/sqlite.go
(which backs both SQLite and PostgreSQL via the QueryDialect interface)
that PG rejects at parse time, breaking TUI startup and API aggregate /
deletion-staging endpoints when the store is PG.

HIGH kenn-io#1 — buildAggregateSQL emitted `FROM ( <inner SELECT> )` with
no alias on the outer derived table. PG requires every derived table
in FROM to be aliased ("syntax error at or near ')'"). Added `AS agg`;
SQLite tolerates the explicit alias unchanged.

HIGH kenn-io#2 — GetGmailIDsByFilter used SELECT DISTINCT … ORDER BY
m.sent_at DESC, m.id DESC. PG rejects DISTINCT when the ORDER BY
expressions are not in the SELECT list, and the multiplying joins to
message_recipients / message_labels were why DISTINCT was there in
the first place. Converted every multiplicative filter (sender,
sender name, recipient, recipient name, domain, label) into an
EXISTS subquery — per the SQL guidance in CLAUDE.md — so the join
graph no longer multiplies, DISTINCT is unnecessary, and the ORDER
BY stands on its own. The sources scope (1:1 with messages) stays as
a JOIN.

MEDIUM kenn-io#3 — ListMessages's ORDER BY referenced `m.size_estimate` and
`m.subject` raw while the SELECT exposed them through
`COALESCE(m.size_estimate, 0)` and `COALESCE(m.subject, '')`. Under
SELECT DISTINCT, PG insists the ORDER BY expressions textually match
something in the SELECT list. Switched the size and subject sorts to
the COALESCE expressions; the date sort already matched.

Test
- internal/query/pg_compat_test.go exercises Aggregate (ViewSenders),
  GetGmailIDsByFilter with a Label filter (the previously
  multiplying join, now EXISTS) and verifies no duplicate IDs come
  back, and ListMessages with every supported sort field. Uses
  testutil.NewTestStore, so MSGVAULT_TEST_DB=postgres://… runs the
  same suite against PG and catches regressions of these three bug
  shapes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant