Repository navigation
feat(*): show how far the import is into the source it is on - #598
Conversation
00768cc to
202aeea
Compare
|
Self-review round, three clean contexts (backend contract, web client, test strength). One blocker, three kept, four let go. Fixed in 202aeea. Blocker: the source the pass is on was counted twice at every source boundary. The window is every gap between a source settling and the next source's first batch, which is a file read wide, and it gets worse as the total gets smaller: a two-source run reads 100 percent halfway through. Fix: the gateway listens to the run's progress events and stops naming a settled source. Same sequence after the fix is monotonic 0 to 100 with no early 100. Also taken:
Named and let go:
|
gloryfromca
left a comment
There was a problem hiding this comment.
No blockers; this can merge as far as I am concerned.
Reviewed the complete PR diff and the follow-up fix, plus the surrounding importer, RPC status lifecycle, generated clients, web row calculation, callers, and relevant history. The source-boundary fix clears current only after the state file has settled that source, so status readers see exactly one share before, during, and after the transition. Cancellation, retry/failure, empty-source, phase-transition, and stopped/restarted-gateway behavior remain coherent.
Also checked AGENTS.md/CLAUDE.md, CONTEXT-MAP.md and the Runtime/Web UI architecture terms, backward compatibility of the additive optional wire field, commit/history requirements, and that the tests were strengthened rather than weakened.
Verification:
uv run pytest tests/test_rpc_schema_match.py tests/test_rpc_import_sync.py tests/test_importer_orchestrator.py tests/test_importer_phases.py -q- 472 passednpm test -- src/features/importSync/store.test.ts- 22 passednpx vitest run scripts/gates/fixture-shape.test.mjs- 5 passednpm run type-check- passednpm run gen:check- generated client matches the 193-method contractnpx eslint src/features/importSync/store.ts src/features/importSync/store.test.ts- passed
The rail's share counted settled sources over the total, so it stood still for the whole of a large source: a 287-message memory directory is 29 batches and ten minutes at 42 percent, which reads as a hang. The orchestrator now reports, per source, how many of its messages have landed after each batch; import.status carries it as a nullable current object beside phase, and the row adds that source's share to its count. A gateway restart clears it and the row falls back to the per-source share. Co-authored-by: Claude (claude-fable-5-1) <noreply@anthropic.com>
The gateway kept the last batch report of a source after run_import had marked it submitted, so a reader adding that source's share to the settled count drew the same source twice: the row reached 100 percent with a whole source still to send, then fell back when the next one started. Measured on a three-source pass, the sequence a poller saw was 33, 67, 33, 47, 60, 67, 100, 67 -- the window is every gap between a source settling and the next one's first batch, which is a file read wide. The run's progress events say when a source is settled, so the gateway now listens to them and stops naming it. The row ignores a current source while no run is on, for the same reason. The tests read the reports out of a real pass rather than a set global: the counts follow the source the pass is on, a retried batch adds its messages once and only after the attempt that lands, and a source given up on never reports its last batch as landed. Co-authored-by: Claude (claude-opus-5) <noreply@anthropic.com>
…ounts Three findings from a second review round, all on the per-source share this branch added. A source that failed is sent again by the next run while its entry still says failed, so the counts hold it for that whole pass; naming it as well had a reader add its share on top of a count that already held it. Measured on a three-source resume the row read 94 percent and fell back to 67 when the source landed. import.status no longer names a source the counts already hold. Rounding carried the last source over 99.5 before it was done: 17 of 18 settled with a 3000-message source 270 messages from the end read 100 percent, nine minutes early. The same 100-percent-then-wait is already written down in the profile mirror's progress line. The row now stops at 99 until every source is settled. The share one source buys the percentage is a fraction of a point in a run of many -- a 287-message source inside nineteen moves the number five times in ten minutes -- so the row carries that source's own counts beside it, the way it already carries the phase's, and they move with every batch. The tests read the reports out of a real pass: a failed source being sent again, a source given up on, a skipped source, a run that a stop left half way, a crash after a source was named, a source with nothing to send, and the whole status payload validated against the result model. Co-authored-by: Claude (claude-opus-5) <noreply@anthropic.com>
5298f45 to
40d43b8
Compare
gloryfromca
left a comment
There was a problem hiding this comment.
Blocking: keep batch progress visible while retrying a failed source.
The new 99-percent cap and the source-count display work for first attempts, but the retry path suppresses the data that display needs. Reviewed the revision delta, the importer/state/RPC callers and history, the web rendering path, the wire contract and backward compatibility, the project architecture/rules, and the added tests; no other blocker survived verification.
Verification:
uv run pytest tests/test_rpc_schema_match.py tests/test_rpc_import_sync.py tests/test_importer_orchestrator.py tests/test_importer_phases.py -q- 477 passednpm test -- src/features/importSync/store.test.ts src/features/importSync/ImportSyncPage.test.tsx- 34 passednpm run type-check- passednpm run gen:check- generated client matches the 193-method contract- ESLint on the five touched importSync files - passed
|
Second self-review round, three fresh clean contexts (the fix itself, the row's user-visible states, test strength). Three findings taken, four named and let go. Fixed in 40d43b8. The same double count, through the Rounding reached 100 percent before the pass was done. 17 of 18 settled with a 3000-message source in flight reads 100 percent from The number still barely moved in the case the branch exists for. One source's whole share is Tests. The second round mutated fourteen things the first round had not; nine survived. The ones that mattered: Named and let go:
One correction to the review above: |
…ut of sight Dropping the name of a source the count already held removed the double count but also removed its progress: a failed source keeps its failed entry for the whole of the run that sends it again, so the row stood still and count-less for all ten minutes of a large retry. A source is counted or named, never both, and the in-flight one is the one to leave out of the count: the row now falls back by that source's share when the retry starts, which is the work that is really left, then climbs with its batches. Co-authored-by: Claude (claude-opus-5) <noreply@anthropic.com>
gloryfromca
left a comment
There was a problem hiding this comment.
No blockers; suggestions only, and they are marked inline.
This head rebases the reviewed import change onto the updated target without changing its import-related tree. Rechecked the full target diff, relevant callers/history, wire compatibility, tests, and project architecture/rules; no new issue survived verification.
Named follow-up: a failed source being retried is omitted from import.status.current, so that retry does not show per-batch counts. The behavior remains real but is nonblocking at this fourth round because the row still reports the run and the operator can wait for completion. The prior thread has been replied to and resolved.
Verification:
uv run pytest tests/test_rpc_schema_match.py tests/test_rpc_import_sync.py tests/test_importer_orchestrator.py tests/test_importer_phases.py -q- 477 passednpm test -- src/features/importSync/store.test.ts src/features/importSync/ImportSyncPage.test.tsx scripts/gates/fixture-shape.test.mjs- 39 passednpm run type-check- passednpm run gen:check- generated client matches the 193-method contract- ESLint on the five touched importSync files - passed
gloryfromca
left a comment
There was a problem hiding this comment.
No blockers; this can merge as far as I am concerned.
Reviewed the new retry-count delta on the actual GitHub head. It fixes the named follow-up without changing the wire shape: the in-flight source remains visible through current and is temporarily omitted from settled counts, so the row neither double-counts nor freezes. Checked the state transitions, per-platform totals, cancellation/settlement behavior, caller compatibility, project rules, and the strengthened regression test.
Verification on an isolated worktree at b4cf9751c: uv run pytest tests/test_rpc_schema_match.py tests/test_rpc_import_sync.py tests/test_importer_orchestrator.py tests/test_importer_phases.py -q - 477 passed.
The author reply was confirmed; the originating thread was already resolved and remains settled.
## Summary
The rail's import row counted settled sources over the total, so its
share stood still for the whole of a large source. On the live run a
287-message memory directory is 29 batches of 10 and about ten minutes
at 42 percent, which the maintainer read as a hang.
- The orchestrator reports, per source, how many of its messages have
landed after each batch (`on_batch(platform, source_key, sent, total)`),
the way it already reports per-source outcomes.
- `import.status` carries it as a nullable `current {platform,
source_key, sent, total}` beside `phase`, set while the message pass is
on and null otherwise; additive and optional in the contract, both
generated clients regenerated.
- The row adds that source's share to its count: `(settled + sent/total)
/ total`. A gateway restart clears the field and the row falls back to
the per-source share, which is what it showed before.
- A source is counted or named, never both. The gateway stops naming a
source the moment the run's progress event settles it, and leaves the
one a retry is sending out of the settled count instead of out of sight.
Without the first, a poller on a three-source pass saw 33, 67, 33, 47,
60, 67, 100, 67 percent, reaching 100 with a whole source still to send;
without the second, a failed source being sent again was held by the
count and named at the same time. The row also ignores a `current` while
no run is on.
- The row stops at 99 percent until every source is settled: rounding
carried the last source over 99.5 well before it was done, which is the
100-percent-then-wait the profile mirror's progress line already warns
about.
- The row carries the current source's own counts beside the percentage
(`41% - 120/287`), the way it already carries the phase's `1/3`. One
source's whole share is `1/N` of the bar, so in a run of many the
percentage alone moves a few times an hour; the counts move with every
batch.
## Type
- [ ] Fix
- [x] Feature
- [ ] Docs
- [ ] CI / tooling
- [ ] Refactor
- [ ] Other
## Verification
- `uv run pytest tests/test_rpc_schema_match.py
tests/test_rpc_import_sync.py tests/test_importer_orchestrator.py
tests/test_importer_phases.py -q`: 472 passed.
`tests/integration/test_import_e2e.py` green on the first round of this
branch.
- The reports are read out of a real pass rather than a set global: a
two-source run over a fake backend that polls `import.status` from
inside `store` sees `[(k1,0,12), (k1,10,12), (k2,0,12), (k2,10,12)]`,
and a poll taken in the window between one source settling and the next
one's first batch sees `current` null both times.
- ui-web: 194 tests across the import feature and the gates pass; `npm
run type-check` clean, `npm run gen:check` in sync, eslint clean on the
touched feature.
- Mutation checks, each confirmed red: report the batch before
`backend.store` rather than after it lands; report once per store
attempt instead of once per landed batch; name every report after the
first source of the run; drop `on_progress` from the gateway's
`run_import` call; drop `on_batch` from it; drop the `running` guard,
the clamp, or the zero guard from the row's share.
- Real host: a gateway built from this branch, its own RAVEN_HOME and a
synthetic Claude Code home of three memory sources (5, 68 and 9
messages), driven through the wizard's data-sync step in a browser,
storing into a real EverOS. The row, sampled from the DOM:
```
run 0% - 0/5 (first source)
run 33% - 0/68 (second source begins)
run 38% - 10/68
run 43% - 20/68
run 48% - 30/68
run 53% - 40/68
run 58% - 50/68
run 63% - 60/68
run 67% - 0/9 (third source begins)
done 100%
```
The number moves with every batch inside the 68-message source, the bar
width follows it, no step goes backwards, and 100 percent arrives only
with the finished row. One apparent inversion appeared while two
samplers were reading the page at once and did not reproduce with a
single sampler; the row's poll is a fixed interval and does not
serialise its reads, so an out-of-order answer could still show one.
That is older than this branch and corrects itself on the next read.
- [x] Relevant tests pass locally
- [x] Relevant lint / type checks pass locally
- [ ] User-facing docs or screenshots are updated when needed
## Risk
User-visible: the row's percentage now moves during a large source
instead of stepping once per source. `import.status` gains one optional
nullable field; older clients ignore it.
Rollback: revert the squash commit; no data migration is involved.
- [x] Security impact considered (no new inputs, credentials or
endpoints)
- [x] Backward compatibility considered
- [x] Rollback path is clear for risky changes
## Related Issues
N/A
---------
Co-authored-by: gloryfromca <23442919+gloryfromca@users.noreply.github.com>
Co-authored-by: Claude (claude-fable-5-1) <noreply@anthropic.com>
Summary
The rail's import row counted settled sources over the total, so its share stood still for the whole of a large source. On the live run a 287-message memory directory is 29 batches of 10 and about ten minutes at 42 percent, which the maintainer read as a hang.
on_batch(platform, source_key, sent, total)), the way it already reports per-source outcomes.import.statuscarries it as a nullablecurrent {platform, source_key, sent, total}besidephase, set while the message pass is on and null otherwise; additive and optional in the contract, both generated clients regenerated.(settled + sent/total) / total. A gateway restart clears the field and the row falls back to the per-source share, which is what it showed before.currentwhile no run is on.41% - 120/287), the way it already carries the phase's1/3. One source's whole share is1/Nof the bar, so in a run of many the percentage alone moves a few times an hour; the counts move with every batch.Type
Verification
uv run pytest tests/test_rpc_schema_match.py tests/test_rpc_import_sync.py tests/test_importer_orchestrator.py tests/test_importer_phases.py -q: 472 passed.tests/integration/test_import_e2e.pygreen on the first round of this branch.import.statusfrom insidestoresees[(k1,0,12), (k1,10,12), (k2,0,12), (k2,10,12)], and a poll taken in the window between one source settling and the next one's first batch seescurrentnull both times.npm run type-checkclean,npm run gen:checkin sync, eslint clean on the touched feature.backend.storerather than after it lands; report once per store attempt instead of once per landed batch; name every report after the first source of the run; dropon_progressfrom the gateway'srun_importcall; dropon_batchfrom it; drop therunningguard, the clamp, or the zero guard from the row's share.The number moves with every batch inside the 68-message source, the bar width follows it, no step goes backwards, and 100 percent arrives only with the finished row. One apparent inversion appeared while two samplers were reading the page at once and did not reproduce with a single sampler; the row's poll is a fixed interval and does not serialise its reads, so an out-of-order answer could still show one. That is older than this branch and corrects itself on the next read.
Risk
User-visible: the row's percentage now moves during a large source instead of stepping once per source.
import.statusgains one optional nullable field; older clients ignore it.Rollback: revert the squash commit; no data migration is involved.
Related Issues
N/A