feat(cli): real progress bars for indexing - #54
Merged
Conversation
Uses the existing rich dependency rather than adding tqdm. Parsing previously used console.status(), an indeterminate spinner. It now reports a bar with count, elapsed and ETA alongside the current file name. The bigger gap was embeddings. build_embeddings() ran behind a single 'Building embeddings...' spinner with no output until it finished. On a first run that also downloads the model this is many minutes of apparent hang -- I had to query Qdrant directly to confirm the process was alive. It now takes an optional progress_callback(done, total), invoked after each batch, and the CLI renders it as a bar. Default is None, so the API is unchanged for existing callers. The callback also fires once with (0, 0) when there is nothing embeddable, so callers always get a terminal update instead of a bar that never resolves. Bars are transient, so they clear on completion and leave the existing summary lines as the only residue. Verified by re-indexing a 105-file Spring Boot project end to end. Suite: 3 failed, 178 passed (baseline on main: 3 failed, 174 passed) -- same three pre-existing failures, fixed separately in #51.
Owner
|
Good pr |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #5
Uses the existing
richdependency rather than addingtqdm.Parsing
Was
console.status()— an indeterminate spinner. Now a bar with count, elapsed and ETA, plus the current file name, which is what the issue asks for.Embeddings — the phase that actually needed this
build_embeddings()ran behind a single"Building embeddings..."spinner and emitted nothing until it finished. On a first run, which also downloads the model, that's many minutes of apparent hang. When I hit this I ended up querying Qdrant directly to work out whether the process was alive or wedged.It now accepts an optional
progress_callback(done, total), invoked after each batch, and the CLI renders it as a bar:Default
None, so existing callers are unaffected — this is additive.One deliberate detail: the callback also fires once with
(0, 0)when nothing is embeddable, so a caller driving a UI always gets a terminal update rather than a bar that never resolves.Bars are
transient=True, so they clear on completion and the existing summary lines remain the only residue.Tests
4 new tests in
tests/test_indexing_progress.py:(total, total), andtotaldoesn't shift mid-run_index_progress()actually carries a count column and an ETA (the issue explicitly asks for a file count)The rendering itself isn't asserted — rich suppresses live output when not attached to a TTY, so a test would be asserting nothing. Instead the tests pin the mechanism the bar is driven by. I verified the visible behaviour by re-indexing a 105-file Spring Boot project end to end.
Verification
Baseline on
mainis3 failed, 174 passed— same three pre-existing failures (fixed in #51), plus the 4 new tests.Independent of #50, #51, #52 and #53; branches from
main. CI here will be red at theLint with ruffstep until #51 lands — that's the pre-existing breakage, not this change.