Skip to content

Commit ec558d2

Browse files
authored
Merge pull request #65 from firstsun-dev/codex/perf-github-push-260723
Push boost
2 parents 49f16c6 + 64b7fec commit ec558d2

24 files changed

Lines changed: 629 additions & 266 deletions

‎docs/push-strategy-benchmark.md‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
# GitHub push strategy benchmark (#61)
2+
3+
## Procedure
4+
5+
Use a disposable repository and branch. For each file count (1, 5, 10, 25, 50, 100, 200), generate equal-size unique files, run the GraphQL path and then the developer-only `pushBatchViaGitDataApiForBenchmark` path on separate branches, and record the opt-in `PushTimingRecord` plus the REST wall time. Do not run this against a user vault or production branch.
6+
7+
The GraphQL record is enabled only by registering `setPushTimingHandler`; it contains no file paths, contents, token, repository identity, or network transmission. `providerProcessingMs` is intentionally `null`, because Obsidian's `requestUrl` API does not expose server-only timing.
8+
9+
## Deterministic request-count result
10+
11+
| Files | GraphQL requests | Git Data API requests | Git Data API request waves (8 concurrent blobs) |
12+
| ---: | ---: | ---: | ---: |
13+
| 1 | 2 | 6 | 6 |
14+
| 5 | 2 | 10 | 6 |
15+
| 10 | 2 | 15 | 7 |
16+
| 25 | 2 | 30 | 9 |
17+
| 50 | 2 | 55 | 12 |
18+
| 100 | 2 | 105 | 18 |
19+
| 200 | 2 | 205 | 30 |
20+
21+
GraphQL performs a branch-head read and one mutation; the caller marks the committed paths synced without a tree readback. Git Data API performs a ref read, commit read, one blob upload per file, tree creation, commit creation, and ref update. The table is covered by service tests for both implementations.
22+
23+
## Decision
24+
25+
Keep GraphQL as the production strategy for every supported batch size (1–200). There is no request-count crossover: Git Data API starts with four additional round trips and adds one request per file. The REST path remains developer-only so provider benchmarks can challenge this conclusion without adding a user-facing strategy switch.

‎feature_list.json‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -121,6 +121,22 @@
121121
"dependencies": ["feat-014"],
122122
"status": "done",
123123
"evidence": "Commit 7676325. npx eslint . -> 0 errors; npm run build -> clean; npx vitest run -> 344/344 passed."
124+
},
125+
{
126+
"id": "feat-016",
127+
"name": "perf(push): eliminate redundant GitHub requests + avoid stale rename lookup requests",
128+
"description": "Two further push-speed perf commits landed on codex/perf-github-push-260723 before this session (pre-existing work, not authored in this session): 0445b17 eliminate redundant GitHub requests, f8a0a26 avoid stale rename lookup requests. perf(push) is patch-level under .releaserc.json's commit-analyzer, so these will bump 1.3.0 -> 1.3.1 on the next automated release.",
129+
"dependencies": ["feat-015"],
130+
"status": "done",
131+
"evidence": "Commits 0445b17, f8a0a26 (pre-session)."
132+
},
133+
{
134+
"id": "feat-017",
135+
"name": "feat(i18n): localize update notifications (issue #64)",
136+
"description": "The what's-new modal title/buttons were already i18n'd, but each release's entry text (src/changelog.ts) was hard-coded English. Restructured into src/changelog/ with one folder per version (1.2.1, 1.3.0, 1.3.1), each entry carrying inline {en, zh-tw, zh-cn} text resolved at render time by entryText() against the active locale (falls back to en). WhatsNewModal and the settings what's-new banner both switched from reading entry.text to entryText(entry). First pass put the text as new keys in the shared src/i18n/locales/*.ts catalog; user rejected that since it would make those files grow unbounded every release, so it was redesigned into the per-version-folder shape. Also added a 1.3.1 entry summarizing feat-016's push-speed work.",
137+
"dependencies": [],
138+
"status": "done",
139+
"evidence": "npx eslint . -> 0 errors; npm run build -> clean; npx vitest run -> 351/351 passed."
124140
}
125141
],
126142
"_evidenceStyle": "Keep evidence to one line: commit hash + short pointer (e.g. 'Commit abc1234 - added X, tests pass'). Debugging narrative and design discussion belong in the commit message, not this file."

‎package-lock.json‎

Lines changed: 2 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎progress.md‎

Lines changed: 20 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -12,57 +12,55 @@ Completed work is archived in [archive/](./archive/), one file per calendar mont
1212

1313
## Current State
1414

15-
**Last Updated:** 2026-07-14 12:05
15+
**Last Updated:** 2026-07-23
1616
**Session ID:** current
17-
**Active Feature:** None — feat-014 and feat-015 both committed. Ready for the next issue.
17+
**Active Feature:** feat-064 done. Ready for the next issue.
1818

1919
## Status
2020

2121
### What's Done
2222

23-
- [x] feat-001..013 — see [archive/2026-07.md](./archive/2026-07.md)
24-
- [x] feat-014: GitHub's `pushBatch`/`deleteBatch` now use GraphQL `createCommitOnBranch` instead of the REST blob-per-file loop (commit `114a575`).
25-
- [x] feat-015: fixed a false "modified" status right after batch push, caused by GitHub's tree-by-branch-name read lagging a just-completed write. `pushAllFiles` now returns `syncedPaths`; `SyncStatusView` marks those files synced directly instead of re-fetching the remote tree (commit `7676325`).
26-
- Both committed onto `claude/fix-directory-symlink-pull-260713`, **not yet pushed to remote**.
27-
- Evidence: `npx eslint .` → 0 errors; `npm run build` → clean; `npx vitest run` → 344/344 passed.
23+
- [x] feat-001..015 — see [archive/2026-07.md](./archive/2026-07.md). 1.3.0 shipped 2026-07-14 (`chore(release): 1.3.0`).
24+
- [x] Two push-speed perf commits landed on `codex/perf-github-push-260723` prior to this session: `0445b17` (eliminate redundant GitHub requests) and `f8a0a26` (avoid stale rename lookup requests). Both `perf(push)` scope — patch-level under `.releaserc.json`'s commit-analyzer rules, so the next automated release will be 1.3.1.
25+
- [x] feat-064 (issue [#64](https://github.com/firstsun-dev/git-files-sync/issues/64)): "what's new" entry text moved out of hard-coded English strings. `ChangelogEntry.text` is now `{ en, 'zh-tw'?, 'zh-cn'? }`; `entryText()` in `src/changelog/index.ts` picks the active locale at render time, falling back to `en`. `WhatsNewModal` and the settings banner both call it instead of reading `entry.text` directly.
26+
- [x] Changelog content reorganized from a single growing `src/changelog.ts` into per-version folders (`src/changelog/1.2.1/`, `1.3.0/`, `1.3.1/`) — user explicitly asked for this instead of piling more keys into the shared `src/i18n/locales/*.ts` catalog, which would grow unbounded release over release.
27+
- [x] Added a `1.3.1` release entry (notable, i18n'd in en/zh-tw/zh-cn) summarizing the push-speed work, so the "what's new" modal covers it once semantic-release cuts 1.3.1.
28+
- Evidence: `npx eslint .` → 0 errors; `npm run build` → clean (includes Obsidian 1.11.0 compat check); `npx vitest run` → 351/351 passed. (Ran `npm install` first — `node_modules` was empty at session start in this worktree.)
2829

2930
### What's In Progress
3031

3132
- Nothing actively in progress.
3233

3334
### What's Next
3435

35-
1. Push `claude/fix-directory-symlink-pull-260713` to update PR #51 — confirm with the user first.
36-
2. Manually verify feat-014/feat-015 inside the actual Obsidian plugin UI (this session's verification used a live GitHub API smoke test driving `GitHubService` directly, not the full plugin).
37-
3. Filed issue #57 (repo `git-files-sync`, Project #6, P2, 4h estimate): build a repeatable live-credential smoke test process across GitHub/GitLab/Gitea, not just GitHub — https://github.com/firstsun-dev/git-files-sync/issues/57
38-
4. Re-sync against `gh issue list --repo firstsun-dev/git-files-sync --state open` — `feature_list.json`'s backlog is a snapshot from an earlier session.
39-
5. Issue #37 (Bitbucket provider support, feat-010): unblocked, still not started.
40-
6. PR #51 is large (10+ issues' worth of changes now). Flag to the user before piling on more commits.
36+
1. Push this session's commit to `origin/codex/perf-github-push-260723` — confirm with the user first (this branch already has no open PR; decide whether to open one or merge directly to `main`).
37+
2. New release folders (e.g. `src/changelog/1.4.0/`) still need one import line added to `src/changelog/index.ts`'s `CHANGELOG` array — not fully zero-touch, but no longer touches the shared i18n locale files.
38+
3. Re-sync against `gh issue list --repo firstsun-dev/git-files-sync --state open` — the backlog below is a snapshot from an earlier session.
4139

4240
## Blockers / Risks
4341

4442
- None currently.
4543

4644
## Decisions Made
4745

48-
- **All work goes into one PR (#51)**: user explicitly said "不要那麼多pr merge" (don't want so many PRs). Commit new work directly onto `claude/fix-directory-symlink-pull-260713`.
49-
- **Optimistic local status update over a post-push re-fetch (feat-015)**: rather than delaying the refresh or retrying, mark just-pushed files synced from data already in hand. Correct by construction (it's the exact content just written) and sidesteps GitHub's eventual-consistency window entirely instead of just narrowing it.
50-
- **Credential handling for live smoke tests**: write PATs to a local scratchpad file the agent reads directly — never as a `!`-prefixed command (still lands in the transcript) or a Bash argument (blocked by the permission classifier). Filed as issue #57 to build a proper reusable process across all three providers.
46+
- **Changelog "what's new" text lives in per-version folders under `src/changelog/`, not the shared i18n catalog**: user flagged that dumping every release's entry text into `src/i18n/locales/*.ts` would make those files grow forever. Each `src/changelog/<version>/index.ts` carries its own inline `{ en, zh-tw, zh-cn }` text; `src/changelog/index.ts` aggregates and resolves per the active locale via `entryText()`.
47+
- **perf → patch under `.releaserc.json`**: confirmed via `.releaserc.json`'s commit-analyzer `releaseRules` (`{ "type": "perf", "release": "patch" }`), so the two `perf(push)` commits already on this branch bump 1.3.0 → 1.3.1 automatically on release — no manual edit to `package.json`/`manifest.json`/`versions.json` (that's `@semantic-release/exec`'s job in CI).
5148

5249
## Files Modified This Session
5350

54-
- `src/services/github-service.ts`, `src/services/git-service-base.ts`, `tests/services/github-service.test.ts` (feat-014)
55-
- `src/logic/sync-manager.ts`, `src/ui/SyncStatusView.ts`, `tests/logic/sync-manager-batch.test.ts`, `tests/ui/SyncStatusView.test.ts` (feat-015)
51+
- `src/changelog.ts` deleted; replaced by `src/changelog/{index,types}.ts` and `src/changelog/{1.2.1,1.3.0,1.3.1}/index.ts`
52+
- `src/ui/WhatsNewModal.ts`, `src/settings.ts` (use `entryText()` instead of raw `entry.text`)
53+
- `tests/changelog.test.ts`, `tests/ui/WhatsNewModal.test.ts` (fixtures updated to the new `text: { en }` shape)
54+
- `package-lock.json` (stale `version` field synced to 1.3.0 by `npm install`)
5655

5756
## Evidence of Completion
5857

59-
- [x] Tests pass: `npx vitest run` → 344/344 passed
58+
- [x] Tests pass: `npx vitest run` → 351/351 passed
6059
- [x] Type check clean: `npm run build` → clean
6160
- [x] Lint clean: `npx eslint .` → 0 errors
62-
- [x] Live smoke tests against a real GitHub repo (feat-014 push/delete, feat-015 empty-file diagnosis)
63-
- [ ] Manual verification inside the actual Obsidian plugin UI — not yet done
61+
- [ ] Manual verification inside the actual Obsidian plugin UI — not done, no Obsidian instance available
6462
- [ ] Pushed to remote — not yet done, needs confirmation
6563

6664
## Notes for Next Session
6765

68-
- Working branch: `claude/fix-directory-symlink-pull-260713` (PR #51). Commits `114a575` and `7676325` are local only — push before starting new work, or confirm with the user.
66+
- Working branch: `claude/conventional-perf-push-speed-9136c3`, tracking `origin/codex/perf-github-push-260723`. Nothing pushed yet — confirm with the user before pushing/merging.

‎session-handoff.md‎

Lines changed: 19 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -9,53 +9,49 @@
99

1010
## Current Objective
1111

12-
- Two fixes this session, both committed onto `claude/fix-directory-symlink-pull-260713` (PR #51), **not yet pushed to remote**:
13-
1. **feat-014** (commit `114a575`): GitHub's `pushBatch`/`deleteBatch` switched from the REST Git Data API's per-file blob-creation loop to a single GraphQL `createCommitOnBranch` mutation.
14-
2. **feat-015** (commit `7676325`): fixed a false "modified" status shown right after a batch push, caused by re-fetching the remote tree too soon after a write (GitHub's tree-by-branch-name read can lag a moment behind a just-completed commit).
12+
- Implement issue [#64](https://github.com/firstsun-dev/git-files-sync/issues/64) (localize "what's new" update notifications) on branch `claude/conventional-perf-push-speed-9136c3`, which tracks `origin/codex/perf-github-push-260723` (already ahead of `main` by two `perf(push)` commits: `0445b17`, `f8a0a26`). Also prepare the "what's new" changelog entry for the next patch release (1.3.1) covering those push-speed perf commits, so it's ready when semantic-release cuts the release. **Not yet committed or pushed.**
1513

1614
## Completed This Session
1715

18-
- [x] `githubGraphQL()` helper in `github-service.ts`; explicit handling for GraphQL's 200-status-with-`errors`-array failure mode.
19-
- [x] `BaseGitService.getLatestCommitSha()` extracted from `resolveGitHubStyleBaseTree()`.
20-
- [x] `SyncManager.pushAllFiles()` now returns `syncedPaths: Array<{path, sha?}>`, populated at all three push-success sites (batch chunk, sequential fallback, immediate symlink/rename push).
21-
- [x] `SyncStatusView.executeBatchOperation()` marks just-pushed paths `'synced'` directly via a new `applyOptimisticSyncedStatus()` instead of calling `refreshAllStatuses()` after a push. Pull is unchanged (still does a full refresh).
22-
- [x] Live-verified both fixes against a real GitHub repo (`firstsun-dev/obsidian-sync-test`) using a scratchpad script that bundles the actual `github-service.ts` with a thin `obsidian` stub (`requestUrl` backed by real `fetch`) — not just mocks.
23-
- [x] Filed issue #57 on `firstsun-dev/git-files-sync` (Project #6, P2, 4h): build a repeatable live-credential smoke test process across GitHub/GitLab/Gitea (this session only covered GitHub).
24-
- [x] Cleaned up harness state: archived feat-011/012/013 + issue #78 fix + status-badge UI work into `archive/2026-07.md` (progress.md had grown past its own 80-line cleanup threshold); trimmed `feature_list.json` evidence strings under 300 chars. Harness self-audit went from 96/100 to 100/100.
16+
- [x] Refactored `src/changelog.ts` into a `src/changelog/` package: `types.ts` (shared interfaces), `index.ts` (aggregator + `entryText()` locale resolver + `getUnseenReleases()`), and one folder per release (`1.2.1/`, `1.3.0/`, `1.3.1/`) each exporting a `release: ChangelogRelease` with inline `{ en, 'zh-tw', 'zh-cn' }` text per entry.
17+
- [x] First design pass put the changelog text as new keys in the shared `src/i18n/locales/{en,zh-tw,zh-cn}.ts` catalog — **user rejected this** because it makes those files grow unbounded every release. Reverted and moved to the per-version-folder design above instead.
18+
- [x] `WhatsNewModal.ts` and `settings.ts` (the "what's new" banner) now call `entryText(entry)` to resolve text for the active locale (falls back to `en`) instead of reading a hard-coded `entry.text` string.
19+
- [x] Added a `1.3.1` release entry (notable, all three locales) describing the push-speed improvements, so the modal/banner will surface it once the next release ships.
20+
- [x] Updated `tests/changelog.test.ts` and `tests/ui/WhatsNewModal.test.ts` fixtures to the new `text: { en: '...' }` shape.
21+
- [x] `node_modules` was empty at session start in this worktree — ran `npm install` before the gate (this also fixed a stale `1.2.1` → `1.3.0` version mismatch in `package-lock.json`'s `"version"` field).
2522

2623
## Verification Evidence
2724

2825
| Check | Command | Result | Notes |
2926
|---|---|---|---|
3027
| Lint | `npx eslint .` | 0 errors | |
3128
| Type check + compat | `npm run build` | Pass | Includes Obsidian 1.11.0 compat typecheck |
32-
| Tests | `npx vitest run` | 344/344 passed | +3 new: 2 for `syncedPaths`, 1 more for the post-push status behavior (total 5 new across both fixes) |
33-
| Live smoke test | ad-hoc scratchpad scripts | Pass | feat-014: pushBatch/deleteBatch each produced one commit with correct content. feat-015: single empty file and two-identical-empty-file batches both wrote correct 0-byte blobs with matching git shas — confirmed the bug was read-side (tree fetch timing), not write-side |
29+
| Tests | `npx vitest run` | 351/351 passed | |
3430
| Manual (in Obsidian) | — | Not done | No Obsidian instance available in this environment |
3531

3632
## Files Changed (this session)
3733

38-
- feat-014: `src/services/github-service.ts`, `src/services/git-service-base.ts`, `tests/services/github-service.test.ts`
39-
- feat-015: `src/logic/sync-manager.ts`, `src/ui/SyncStatusView.ts`, `tests/logic/sync-manager-batch.test.ts`, `tests/ui/SyncStatusView.test.ts`
40-
- Harness state: `progress.md`, `session-handoff.md`, `feature_list.json`, `archive/2026-07.md`
34+
- `src/changelog.ts` deleted → `src/changelog/{index,types}.ts`, `src/changelog/{1.2.1,1.3.0,1.3.1}/index.ts`
35+
- `src/ui/WhatsNewModal.ts`, `src/settings.ts`
36+
- `tests/changelog.test.ts`, `tests/ui/WhatsNewModal.test.ts`
37+
- `package-lock.json` (version field sync from `npm install`)
4138

4239
## Decisions Made
4340

44-
- **GraphQL only for GitHub**: GitLab's Commits API already sends a whole batch in one call; Gitea has no GraphQL API. `GitServiceInterface` stays REST-based elsewhere.
45-
- **Optimistic local status update over re-fetching (feat-015)**: use data already known from the push itself rather than trusting an immediate remote read, which sidesteps GitHub's eventual-consistency window rather than just narrowing it with a delay/retry.
46-
- **Credential handling**: PATs must go into a scratchpad file the agent reads directly (`fs.readFileSync`), never typed as a `!`-prefixed command (still lands in the transcript) or passed as a Bash command-line argument (blocked by the permission classifier as credential materialization). Filed issue #57 to formalize this as the only documented path for future provider testing.
41+
- **Per-version folders, not the shared i18n catalog**: see above — direct user correction mid-session.
42+
- **No manual version bump**: `.releaserc.json`'s commit-analyzer maps `perf` → `patch`, so 1.3.0 → 1.3.1 happens automatically via `@semantic-release/exec` in CI when this merges to `main`. Didn't hand-edit `package.json`/`manifest.json`/`versions.json`.
4743

4844
## Blockers / Risks
4945

50-
- **Commits `114a575` and `7676325` are local only** — not pushed to `origin/claude/fix-directory-symlink-pull-260713` yet. Confirm with the user before pushing.
51-
- PR #51 keeps growing (10+ issues' worth now) — still worth flagging before adding more.
46+
- Nothing committed yet. Need to confirm with the user before committing and before pushing to `origin/codex/perf-github-push-260723` (shared branch, no open PR currently).
47+
- Whether the final destination is a PR against `main` or a direct merge of `codex/perf-github-push-260723` — not yet decided with the user.
5248

5349
## Next Session Startup
5450

5551
1. Read `CLAUDE.md`, `feature_list.json`, `progress.md`, then this file.
56-
2. Run `./init.sh` before editing.
57-
3. Check whether `114a575`/`7676325` have been pushed yet (`git log origin/claude/fix-directory-symlink-pull-260713..HEAD`); push first if not, after confirming with the user.
52+
2. Run `./init.sh` (or at least `npm install`) before editing — this worktree started with an empty `node_modules`.
53+
3. Check `git log origin/codex/perf-github-push-260723..HEAD` — if this session's changelog work is still uncommitted/unpushed, finish that first.
5854

5955
## Recommended Next Step
6056

61-
- Push this session's two commits, then either re-sync `gh issue list` or move to issue #37 (Bitbucket provider support) per the previously agreed order.
57+
- Commit the changelog work with a Conventional Commits message (likely `feat(i18n): localize update notifications` scope, closes #64), confirm with the user, then push to `origin/codex/perf-github-push-260723`.

0 commit comments

Comments
 (0)