From 592745118f60974ba273cd0fdb9a7fab4bb47896 Mon Sep 17 00:00:00 2001 From: Dmitry Teryaev Date: Sun, 14 Jun 2026 23:15:23 +0300 Subject: [PATCH 01/11] add propose + plan for index output rework (gating spike passed) Propose (propose/active/INDEX-OUTPUT-REWORK-PROPOSE.md), plan (plans/active/PLAN-INDEX-OUTPUT-REWORK.md), and per-PR agent prompts (plans/active/AGENT-PROMPTS-INDEX-OUTPUT-REWORK.md) for unifying the index-build output across init/increment/install/reprocess/update and adding a real progress bar (rich + JCIRAG_PROGRESS protocol). Gating spike passed 2026-06-14: a print(file=sys.stderr) inside a CocoIndex flow function reaches the parent's captured stderr (130/130 lines relayed; pre-walk divergence 0 on the fixture). PR-1 unblocked. Planning only; no production code changed. Co-Authored-By: Claude --- .../AGENT-PROMPTS-INDEX-OUTPUT-REWORK.md | 494 ++++++++++++++++ plans/active/PLAN-INDEX-OUTPUT-REWORK.md | 525 ++++++++++++++++++ propose/active/INDEX-OUTPUT-REWORK-PROPOSE.md | 453 +++++++++++++++ 3 files changed, 1472 insertions(+) create mode 100644 plans/active/AGENT-PROMPTS-INDEX-OUTPUT-REWORK.md create mode 100644 plans/active/PLAN-INDEX-OUTPUT-REWORK.md create mode 100644 propose/active/INDEX-OUTPUT-REWORK-PROPOSE.md diff --git a/plans/active/AGENT-PROMPTS-INDEX-OUTPUT-REWORK.md b/plans/active/AGENT-PROMPTS-INDEX-OUTPUT-REWORK.md new file mode 100644 index 0000000..4774d83 --- /dev/null +++ b/plans/active/AGENT-PROMPTS-INDEX-OUTPUT-REWORK.md @@ -0,0 +1,494 @@ +# Agent task prompts — INDEX-OUTPUT-REWORK (Spike → PR-4) + +Status: **active**. Plan: +[`plans/active/PLAN-INDEX-OUTPUT-REWORK.md`](./PLAN-INDEX-OUTPUT-REWORK.md). Propose: +[`propose/active/INDEX-OUTPUT-REWORK-PROPOSE.md`](../../propose/active/INDEX-OUTPUT-REWORK-PROPOSE.md). + +One prompt per step. **Landing order:** Spike (gate) → PR-1 → PR-2 → PR-3 → PR-4. Do +not start the next step until the previous is merged to `master` (the Spike is not +merged — it is a throwaway go/no-go). + +**Universal rules:** + +- Use `.venv/bin/python` and `.venv/bin/ruff` only. Never system `python`/`pip`. +- Nothing reachable from MCP tool handlers may write to **stdout** (stderr is the + progress stream; stdout is the JSON/wizard payload). +- **No ontology bump.** `ontology_version` stays **17**. No schema/enrichment change. +- **No new CLI flags** (existing `--quiet`/`--verbose` are wired through; nothing added). +- If ambiguous versus the plan/propose, **stop and ask** — do not expand scope. +- Do not `git push` unless the user explicitly asked. +- No drive-by lint fixes outside deliverables. +- CocoIndex stays a **subprocess**. Do not switch to in-process `app.update()`/`watch()`. + +--- + +## Spike — validate CocoIndex flow-function stderr relay (gate, no PR) + +**Branch:** throwaway (e.g. `spike/flow-stderr`), **not merged**. +**Plan section:** `plans/active/PLAN-INDEX-OUTPUT-REWORK.md` § Spike. + +**Attach (`@-files`):** + +- `@plans/active/PLAN-INDEX-OUTPUT-REWORK.md` (Spike + Principles only) +- `@propose/active/INDEX-OUTPUT-REWORK-PROPOSE.md` (§Vectors phase, Risks) +- `@java_index_flow_lancedb.py` +- `@java_codebase_rag/pipeline.py` +- `@path_filtering.py` (for the pre-walk divergence step) + +**Prompt:** + +```` +You are running the gating spike for INDEX-OUTPUT-REWORK. This is a throwaway branch; +it is NOT merged. The whole plan is contingent on this. + +## Objective +Confirm CocoIndex relays stderr written from inside `@coco.fn` flow functions to the +parent, and size the vectors pre-walk divergence. + +## Steps +1. In `java_index_flow_lancedb.py`, at the top of `process_java_file` (before the + ignore check), emit one line: + `print("JCIRAG_PROGRESS kind=vectors phase=java done=0 total=0 status=running", file=sys.stderr, flush=True)`. +2. Run `cocoindex update java_index_flow_lancedb.py:JavaCodeIndexLance --full-reprocess` + against `tests/bank-chat-system` with stdout+stderr captured to pipes (mirror + `pipeline._popen_capturing_stderr`'s capture). Heavy: this loads torch — it is fine + for the spike. +3. Inspect captured stderr for the `JCIRAG_PROGRESS` line. +4. Pre-walk `tests/bank-chat-system` reproducing the matcher includes + `LayeredIgnore` + (`cocoindex_excluded_patterns()` + `is_ignored`); record the count vs. the actual + `done` at completion. + +## Done when +- A one-paragraph **go/no-go note** is written on the throwaway branch: + - GO: "stderr relays — proceed with PR-1" + the divergence number (e.g. "pre-walk + 42, done 40, gap 2 ignored/empty"). + - NO-GO: "stderr suppressed — halt, re-propose transport". +- Do NOT open a PR. Report the verdict back; the branch is discarded (or kept as a + reference if NO-GO). +```` + +--- + +## PR-1 — `rich` dep + `progress.py` (parser, renderer, non-TTY fallback, relay) + +**Branch:** `feat/index-progress-protocol` off `master` **only after the Spike is GO**. +**Base:** `master`. +**Plan section:** `plans/active/PLAN-INDEX-OUTPUT-REWORK.md` § PR-1. +**PR title:** `feat(cli): add rich + progress protocol/renderer skeleton (JCIRAG_PROGRESS)` + +**Attach (`@-files`):** + +- `@plans/active/PLAN-INDEX-OUTPUT-REWORK.md` (PR-1 + Resolved design decisions only) +- `@propose/active/INDEX-OUTPUT-REWORK-PROPOSE.md` (§ progress protocol, § renderer, Risks) +- `@pyproject.toml` +- `@java_codebase_rag/cli_format.py` (ANSI helpers reused by non-TTY fallback) +- `@java_codebase_rag/cli_progress.py` (existing `_AsyncLineFilter` / drain — reference only) +- `@java_codebase_rag/pipeline.py` (existing `_LineFilter` / drain — reference only) + +**Prompt:** + +```` +You are implementing PR-1 from `plans/active/PLAN-INDEX-OUTPUT-REWORK.md`. Read the +**PR-1** section and the **Resolved design decisions** table before coding. Plan wins +over this prompt. + +## Scope +1. Add `rich>=13.7,<14` to `pyproject.toml` `dependencies`. +2. Create `java_codebase_rag/progress.py` with exactly these symbols: + - `ProgressEvent` dataclass (`kind`, `phase`, `pass_`, `done`, `total`, `status`, + `elapsed_s`). + - `parse_progress_line(line: bytes) -> ProgressEvent | None` — `None` for any + non-`JCIRAG_PROGRESS` line; never raises. + - `IndexProgressRenderer` — `rich.progress.Progress` (TTY) / concise-line fallback + (non-TTY); one task per phase; task visible/`running` only after its first event; + `apply(ev)` clamps completed→total on `status=done`; marks red on `status=failed`; + non-TTY prints at most every ~5 s per phase + on terminal events. + - `ProgressRelay` — line-buffered `feed(chunk)`; parse-first; progress events → + `renderer.apply` and suppressed from relay; non-progress → noise matcher then + `console.print` while `Live` is up (or raw `buffer.write` when relaying verbatim). +3. Add the 13 PR-1 tests verbatim from the plan in `tests/test_progress.py` — all + light (no subprocess, no cocoindex, no torch). + +## Out of scope (do NOT touch) +- Any `JCIRAG_PROGRESS` emission in production files (`build_ast_graph.py`, + `java_index_flow_lancedb.py`, `lance_optimize.py`, `server.py`). PR-2/3 add those. +- Any command wiring (`cli.py`, `installer.py`). No production caller of `progress.py` + in this PR. +- `_popen_capturing_stderr` / `accumulate_and_relay_subprocess_streams` changes + (the `on_progress` plumbing is PR-2). +- `Spinner` / `emit_vectors_*` removal (PR-3). +- Ontology version, schema, any enrichment. + +If you need any of the above, **stop and ask**. + +## Deliverables +1. `rich` installed; `progress.py` with the four symbols; 13 tests in `test_progress.py`. +2. No production caller of `progress.py` yet. + +## Tests +```bash +.venv/bin/ruff check java_codebase_rag/progress.py tests/test_progress.py pyproject.toml +.venv/bin/python -m pytest tests/test_progress.py -v +``` +Before PR open: +```bash +.venv/bin/ruff check . +.venv/bin/python -m pytest tests -v +``` +Expected: all 13 PR-1 tests pass; full suite green; no new heavy gating. + +## Sentinel checks (`git diff master..HEAD`) +```bash +# No production emission yet: +git diff master..HEAD -- build_ast_graph.py java_index_flow_lancedb.py lance_optimize.py server.py | rg "JCIRAG_PROGRESS" && exit 1 || true +# No production caller of progress.py yet: +git diff master..HEAD -- java_codebase_rag/cli.py java_codebase_rag/installer.py java_codebase_rag/pipeline.py | rg "import.*progress|from java_codebase_rag.progress" && exit 1 || true +# rich added: +git diff master..HEAD -- pyproject.toml | rg '^\+.*"rich' || { echo "rich dep missing"; exit 1; } +``` + +## Manual evidence +```bash +.venv/bin/python -c "from java_codebase_rag.progress import parse_progress_line, ProgressEvent, IndexProgressRenderer, ProgressRelay; print('symbols ok')" +``` + +## Definition of Done +- [ ] All 13 PR-1 test names from the plan exist and pass. +- [ ] Sentinels pass (no production emission/caller; rich dep present). +- [ ] `.venv/bin/ruff check .` + `.venv/bin/python -m pytest tests -v` green. +- [ ] PR title: `feat(cli): add rich + progress protocol/renderer skeleton (JCIRAG_PROGRESS)` +- [ ] Branch: `feat/index-progress-protocol` +```` + +--- + +## PR-2 — graph-phase progress (operator commands) + +**Branch:** `feat/index-graph-progress` off `master` **after PR-1 merged**. +**Base:** `master` at PR-1 merge. +**Plan section:** `plans/active/PLAN-INDEX-OUTPUT-REWORK.md` § PR-2. +**PR title:** `feat(cli): graph-phase index progress (count-first pass1 + pass steps)` + +**Attach (`@-files`):** + +- `@plans/active/PLAN-INDEX-OUTPUT-REWORK.md` (PR-2 + Resolved design decisions) +- `@propose/active/INDEX-OUTPUT-REWORK-PROPOSE.md` (§ Graph phase, § progress protocol) +- `@java_codebase_rag/progress.py` (PR-1 symbols) +- `@java_codebase_rag/pipeline.py` +- `@java_codebase_rag/cli_progress.py` +- `@java_codebase_rag/cli.py` +- `@build_ast_graph.py` +- `@tests/test_ast_graph_build.py` +- `@tests/test_java_codebase_rag_cli.py` + +**Prompt:** + +```` +You are implementing PR-2 from `plans/active/PLAN-INDEX-OUTPUT-REWORK.md`. Read the +**PR-2** section + Resolved design decisions. Plan wins over this prompt. + +## Scope +1. `build_ast_graph.py`: add `_emit_progress(parts)` writing one + `JCIRAG_PROGRESS kind=graph …` line to stderr (flushed), gated by the existing + verbose flag. `pass1_parse`: count-first (one filtered `os.walk`, no parse) for the + exact total, then emit `pass=1 total=N` and a `done=k` tick every ~25 files + on + completion. `pass2_edges`…`pass6_match_edges`: emit `pass=N/6 status=running` on + entry, `status=done elapsed_s=…` on exit. Keep the existing heartbeat lines. +2. `pipeline.py` `_popen_capturing_stderr`: accept `on_progress` callback; replace the + inline `_LineFilter` drain with a `ProgressRelay` (parse-first; events → callback; + non-progress → existing noise/relay). `run_build_ast_graph` / `run_incremental_graph`: + thread `on_progress` from the caller; `--verbose` mode passes `on_progress=None` + (raw relay, no renderer). +3. `cli_progress.py` `accumulate_and_relay_subprocess_streams`: same `ProgressRelay` + + `on_progress` wiring. +4. `cli.py`: a renderer context around `_run_with_pipeline_progress`'s `work()` (TTY, + non-`--quiet`, non-`--verbose`) owning the **graph** task; mark it `running` only + after the builder spawns; route non-progress relay lines through `console.print`. + `_cmd_init`/`_cmd_increment`/`_cmd_reprocess` pass the graph `on_progress` through. + Vectors/Optimize tasks stay **pending** in this PR. +5. Add the 7 PR-2 tests verbatim from the plan. Tests 5–7 patch the pipeline helpers + (no cocoindex/torch) so they run in the default light suite. + +## Out of scope (do NOT touch) +- Vectors-phase emission (`java_index_flow_lancedb.py`) — PR-3. +- Optimize emission / `Spinner` / `emit_vectors_*` removal — PR-3. +- `installer.py` (`install`/`update`) — PR-4. +- `_cmd_update`/`_cmd_install` flag forwarding — PR-4. +- Ontology version, schema, enrichment. + +If you need any of the above, **stop and ask**. + +## Deliverables +1. `build_ast_graph.py` emits `kind=graph` progress (exact count-first total + pass + steps) on the non-quiet path. +2. Sync + async drains route progress events to a callback and suppress raw relay. +3. Operator commands render the graph task determinate in default TTY mode. + +## Tests +```bash +.venv/bin/ruff check build_ast_graph.py java_codebase_rag/pipeline.py java_codebase_rag/cli_progress.py java_codebase_rag/cli.py tests/ +.venv/bin/python -m pytest tests/test_ast_graph_build.py tests/test_java_codebase_rag_cli.py -v \ + -k "pass1_emits_per_file or pass1_total_is_exact or passes_2_to_6 or graph_quiet_emits_no_progress or graph_phase_progress_on_stderr or increment_graph_phase_progress or graph_progress_absent_when_quiet" +``` +Before PR open: +```bash +.venv/bin/ruff check . +.venv/bin/python -m pytest tests -v +``` + +## Sentinel checks (`git diff master..HEAD`) +```bash +# No vectors/optimize emission yet (PR-3): +git diff master..HEAD -- java_index_flow_lancedb.py | rg "JCIRAG_PROGRESS" && exit 1 || true +git diff master..HEAD -- java_index_flow_lancedb.py lance_optimize.py | rg "kind=vectors|kind=optimize" && exit 1 || true +# Spinner / emit_vectors_* untouched: +git diff master..HEAD -- java_codebase_rag/cli_format.py | rg "^-.*class Spinner" && exit 1 || true +git diff master..HEAD -- java_codebase_rag/cli_progress.py | rg "^-.*def emit_vectors" && exit 1 || true +# installer untouched: +git diff master..HEAD -- java_codebase_rag/installer.py | rg "JCIRAG_PROGRESS|IndexProgressRenderer|run_init_if_needed.*quiet" && exit 1 || true +# graph progress emitted: +git diff master..HEAD -- build_ast_graph.py | rg "JCIRAG_PROGRESS kind=graph" || { echo "missing graph progress"; exit 1; } +``` + +## Manual evidence +```bash +rm -rf /tmp/iograph && .venv/bin/python build_ast_graph.py \ + --source-root tests/bank-chat-system --ladybug-path /tmp/iograph/code_graph.lbug --verbose 2>&1 \ + | rg "JCIRAG_PROGRESS kind=graph" | head +``` + +## Definition of Done +- [ ] All 7 PR-2 test names pass. +- [ ] Sentinels pass. +- [ ] `install`/`update` unchanged (still `quiet=True`). +- [ ] PR title: `feat(cli): graph-phase index progress (count-first pass1 + pass steps)` +- [ ] Branch: `feat/index-graph-progress` +```` + +--- + +## PR-3 — vectors-phase progress + retire `Spinner`/`emit_vectors_*` + both optimize sites + +**Branch:** `feat/index-vectors-progress` off `master` **after PR-2 merged**. +**Base:** `master` at PR-2 merge. +**Plan section:** `plans/active/PLAN-INDEX-OUTPUT-REWORK.md` § PR-3. +**PR title:** `feat(cli): vectors-phase index progress; retire Spinner/emit_vectors; optimize phase` + +**Attach (`@-files`):** + +- `@plans/active/PLAN-INDEX-OUTPUT-REWORK.md` (PR-3 + Resolved design decisions) +- `@propose/active/INDEX-OUTPUT-REWORK-PROPOSE.md` (§ Vectors phase, § Risks) +- `@java_codebase_rag/progress.py` +- `@java_index_flow_lancedb.py` +- `@java_codebase_rag/pipeline.py` +- `@java_codebase_rag/cli_progress.py` +- `@java_codebase_rag/cli_format.py` +- `@java_codebase_rag/cli.py` +- `@server.py` +- `@java_codebase_rag/lance_optimize.py` +- `@tests/test_java_codebase_rag_cli.py` + +**Prompt:** + +```` +You are implementing PR-3 from `plans/active/PLAN-INDEX-OUTPUT-REWORK.md`. Read the +**PR-3** section + Resolved design decisions + propose § Vectors phase. Plan wins. + +## Scope +1. `java_index_flow_lancedb.py`: `_emit_vectors_progress(...)` writes + `JCIRAG_PROGRESS kind=vectors …` to stderr. In `app_main`, emit an approximate + `total=N status=running` from a pre-walk reproducing the matcher includes + + `LayeredIgnore` (`cocoindex_excluded_patterns()` + `is_ignored`). In each + `process_*_file`, increment an atomic counter and emit `done=k` every ~25 files + + `status=done elapsed_s=…` on the final file. (Incremental catch-up: `memo=True` + ⇒ function only called for changed files ⇒ no `total` event ⇒ parent renders + indeterminate.) +2. `pipeline.py` `_run_cocoindex_update_impl`: drop the `Spinner` and + `emit_vectors_start`/`_finish`; route vectors events through `on_progress`; mark + the vectors task `running` only after `cocoindex` `Popen` succeeds (not on the 127 + stub). +3. `server.py` `run_refresh_pipeline`: route async-drain vectors events into the + renderer; drop `emit_vectors_start`/`emit_vectors_finish`; the optimize block + (`server.py:359-372`) emits `kind=optimize status=running` / `status=done`. +4. `lance_optimize.py` `optimize_lance_tables`: emit `kind=optimize status=running` + on entry, `status=done elapsed_s=…` on exit — the **second** optimize call site + (called from `_maybe_run_serialized_optimize` in `pipeline.py:129`). +5. `cli_progress.py`: remove `emit_vectors_start` / `emit_vectors_finish`. +6. `cli_format.py`: remove the `Spinner` class. +7. `cli.py`: extend the renderer context to own **vectors** + **optimize** tasks for + the operator commands (phase order `Vectors → Optimize → Graph`). +8. Add the 8 PR-3 tests verbatim from the plan. The cocoindex-flow tests + (`test_flow_emits_vectors_progress_per_file`, `test_pre_walk_total_divergence_bounded`) + are **heavy-gated** (`JAVA_CODEBASE_RAG_RUN_HEAVY=1`); the rest stay light via + patched helpers / synthetic event streams. + +## Out of scope (do NOT touch) +- `installer.py` (`install`/`update`) — PR-4. +- `_cmd_update`/`_cmd_install` flag forwarding — PR-4. +- Switching to in-process `cocoindex` `app.update()`/`watch()`. +- Ontology version, schema, enrichment. +- Any new CLI flag. + +If you need any of the above, **stop and ask**. + +## Deliverables +1. `process_*_file` emit `kind=vectors` progress; approximate total from `app_main`; + bar clamps to 100% on completion. +2. Both optimize call sites emit `kind=optimize`. +3. `Spinner`, `emit_vectors_start`, `emit_vectors_finish` removed (no dangling imports). +4. Operator commands render `Vectors → Optimize → Graph`. + +## Tests +```bash +.venv/bin/ruff check java_index_flow_lancedb.py java_codebase_rag/pipeline.py java_codebase_rag/cli_progress.py java_codebase_rag/cli_format.py java_codebase_rag/cli.py server.py java_codebase_rag/lance_optimize.py tests/ +.venv/bin/python -m pytest tests -v -k "vectors_progress_clamps or vectors_progress_approximate or vectors_incremental_renders or cli_init_vectors_phase or reprocess_optimize_phase or spinner_removed or emit_vectors or flow_emits_vectors or pre_walk_total_divergence" +``` +Before PR open: +```bash +.venv/bin/ruff check . +.venv/bin/python -m pytest tests -v +``` +Expected: all 8 PR-3 tests pass (heavy-gated skip without the env var); full suite green. + +## Sentinel checks (`git diff master..HEAD`) +```bash +# Spinner + emit_vectors_* gone (import must fail): +.venv/bin/python -c "from java_codebase_rag.cli_format import Spinner" 2>/dev/null && { echo "Spinner still importable"; exit 1; } || true +.venv/bin/python -c "from java_codebase_rag.cli_progress import emit_vectors_start" 2>/dev/null && { echo "emit_vectors_start still importable"; exit 1; } || true +# No remaining callers/imports of removed symbols anywhere in the tree: +rg -n "emit_vectors_start|emit_vectors_finish|class Spinner|Spinner\(" java_codebase_rag/ server.py && exit 1 || true +# installer untouched: +git diff master..HEAD -- java_codebase_rag/installer.py | rg "JCIRAG_PROGRESS|IndexProgressRenderer" && exit 1 || true +# both optimize call sites emit kind=optimize: +git diff master..HEAD -- server.py | rg "kind=optimize" || { echo "server.py missing optimize progress"; exit 1; } +git diff master..HEAD -- java_codebase_rag/lance_optimize.py | rg "kind=optimize" || { echo "lance_optimize.py missing optimize progress"; exit 1; } +``` + +## Manual evidence (heavy) +```bash +JAVA_CODEBASE_RAG_RUN_HEAVY=1 rm -rf /tmp/iovec && JAVA_CODEBASE_RAG_INDEX_DIR=/tmp/iovec \ + cocoindex update java_index_flow_lancedb.py:JavaCodeIndexLance --full-reprocess 2>&1 \ + | rg "JCIRAG_PROGRESS kind=vectors" | head +``` + +## Definition of Done +- [ ] All 8 PR-3 test names pass (heavy-gated skip without env). +- [ ] Sentinels pass; `Spinner`/`emit_vectors_*` not importable. +- [ ] `ruff` + full `pytest tests -v` green. +- [ ] PR title: `feat(cli): vectors-phase index progress; retire Spinner/emit_vectors; optimize phase` +- [ ] Branch: `feat/index-vectors-progress` +```` + +--- + +## PR-4 — installer alignment (`install`/`update`) + verbosity wiring + docs + +**Branch:** `feat/index-installer-progress` off `master` **after PR-3 merged**. +**Base:** `master` at PR-3 merge. +**Plan section:** `plans/active/PLAN-INDEX-OUTPUT-REWORK.md` § PR-4. +**PR title:** `feat(cli): unified index progress for install/update; wire --quiet/--verbose` + +**Attach (`@-files`):** + +- `@plans/active/PLAN-INDEX-OUTPUT-REWORK.md` (PR-4 + Resolved design decisions) +- `@propose/active/INDEX-OUTPUT-REWORK-PROPOSE.md` (§ Per-command matrix, § Flags/TTY/failure) +- `@java_codebase_rag/progress.py` +- `@java_codebase_rag/installer.py` +- `@java_codebase_rag/cli.py` +- `@docs/JAVA-CODEBASE-RAG-CLI.md` +- `@README.md` +- `@tests/test_installer.py` +- `@tests/test_java_codebase_rag_cli.py` + +**Prompt:** + +```` +You are implementing PR-4 from `plans/active/PLAN-INDEX-OUTPUT-REWORK.md`. Read the +**PR-4** section + Resolved design decisions + propose § Per-command matrix. Plan wins. + +## Scope +1. `installer.py` `run_init_if_needed`: replace the stdout `print("Creating index…")` + / `print("Index created successfully.")` chatter around the indexing calls with the + renderer context (vectors + optimize + graph tasks); un-silence the + `run_cocoindex_update` / `run_build_ast_graph` calls (drop `quiet=True`-style + silence; pass the progress context). Keep all other wizard prompts/summaries on + stdout unchanged. Do NOT wrap `run_install` in `_run_with_pipeline_progress`. +2. `installer.py` `run_update`: drop `quiet=True` on `run_cocoindex_update` / + `run_incremental_graph`; wrap those calls in the renderer context (not + `_run_with_pipeline_progress`); accept and forward `quiet`/`verbose`. Move + stdout `print("\nUpdating index (Lance + graph)…")` / error prints that describe + indexing progress onto the stderr renderer framing. +3. `cli.py` `_cmd_update`: forward `quiet=bool(args.quiet)` and + `verbose=bool(args.verbose)` to `run_update` (both ignored today). `_cmd_install`: + forward `verbose=bool(args.verbose)` to `run_install` (only `quiet` wired today). +4. `docs/JAVA-CODEBASE-RAG-CLI.md`: document the unified progress output (header / + phase list / footer on stderr), determinate-vs-indeterminate per command, and + `--quiet`/`--verbose`/non-TTY behaviour; note the `install`/`update` stderr + behaviour change and that wizard stdout is otherwise unchanged. +5. `README.md`: one-line lifecycle note that indexing shows a progress bar; mention + the `rich` dependency. +6. Add the 7 PR-4 tests verbatim from the plan (patch the pipeline helpers — no + cocoindex/torch — so they run in the default light suite). + +## Out of scope (do NOT touch) +- `build_ast_graph.py` / `java_index_flow_lancedb.py` emission (PR-2/3). +- `progress.py` symbols. +- A stdout JSON payload for `install`/`update` (Open Q6 — recommended no). +- Ontology version, schema, enrichment. +- Any new CLI flag. + +If you need any of the above, **stop and ask**. + +## Deliverables +1. `install`/`update` render the unified phase list on stderr during indexing; wizard + stdout otherwise unchanged. +2. `update` no longer runs indexing with `quiet=True`. +3. `--quiet`/`--verbose` wired through `_cmd_update`/`run_update`; `--verbose` through + `install`. +4. Docs updated. + +## Tests +```bash +.venv/bin/ruff check java_codebase_rag/installer.py java_codebase_rag/cli.py docs/JAVA-CODEBASE-RAG-CLI.md README.md tests/ +.venv/bin/python -m pytest tests/test_installer.py tests/test_java_codebase_rag_cli.py -v \ + -k "install_emits_indexing_progress_on_stderr or update_emits_indexing_progress_on_stderr or update_runs_indexing_without_quiet_true or cmd_update_forwards_quiet or cmd_update_forwards_verbose or cmd_install_forwards_verbose or stdout_contract_preserved" +``` +Before PR open: +```bash +.venv/bin/ruff check . +.venv/bin/python -m pytest tests -v +``` + +## Sentinel checks (`git diff master..HEAD`) +```bash +# update no longer passes quiet=True to the indexing helpers: +git diff master..HEAD -- java_codebase_rag/installer.py | rg '^\-.*quiet=True' || { echo "expected quiet=True removal in installer"; exit 1; } +git diff master..HEAD -- java_codebase_rag/installer.py | rg 'run_cocoindex_update\(.*quiet=True|run_incremental_graph\(.*quiet=True' && exit 1 || true +# flags forwarded: +git diff master..HEAD -- java_codebase_rag/cli.py | rg "verbose=bool\(args.verbose\)" || { echo "verbose not forwarded"; exit 1; } +# stdout contract: no JCIRAG_PROGRESS / IndexProgressRenderer reaches stdout: +git diff master..HEAD | rg "print\(.*JCIRAG_PROGRESS|print\(.*IndexProgressRenderer" && exit 1 || true +``` + +## Manual evidence +```bash +# update is no longer silent (stderr shows progress framing): +.venv/bin/python -c "import inspect; from java_codebase_rag.installer import run_update; src=inspect.getsource(run_update); assert 'quiet=True' not in src, 'still quiet'; print('ok')" +``` + +## Definition of Done +- [ ] All 7 PR-4 test names pass. +- [ ] Sentinels pass. +- [ ] `install`/`update` wizard stdout shape unchanged. +- [ ] Docs + README updated. +- [ ] PR title: `feat(cli): unified index progress for install/update; wire --quiet/--verbose` +- [ ] Branch: `feat/index-installer-progress` +```` + +--- + +## After all PRs land + +- [ ] Move `propose/active/INDEX-OUTPUT-REWORK-PROPOSE.md` → `propose/completed/`. +- [ ] Move `plans/active/PLAN-INDEX-OUTPUT-REWORK.md` + this prompts file → `plans/completed/`. +- [ ] Confirm the whole-plan done definition in the plan is satisfied. diff --git a/plans/active/PLAN-INDEX-OUTPUT-REWORK.md b/plans/active/PLAN-INDEX-OUTPUT-REWORK.md new file mode 100644 index 0000000..9e3ffd3 --- /dev/null +++ b/plans/active/PLAN-INDEX-OUTPUT-REWORK.md @@ -0,0 +1,525 @@ +# Plan: unified progress-bearing index-build output + +Status: **active (planning)**. This plan implements +[`propose/active/INDEX-OUTPUT-REWORK-PROPOSE.md`](../../propose/active/INDEX-OUTPUT-REWORK-PROPOSE.md). +This file is plan-only and does not implement code. + +Depends on: a **throwaway spike** that validates CocoIndex relays flow-function +stderr to the parent (see Spike section). PR-1 does not start until the spike passes. + +## Goal + +- One shared, `rich`-based renderer draws the index-build output (header + per-phase + progress list + footer) on stderr for **all five** lifecycle commands (`init`, + `increment`, `install`, `reprocess`, `update`), replacing the per-command, + sometimes-silent output of today. +- A single `JCIRAG_PROGRESS` structured-line protocol carries real progress across + the subprocess boundary for both the vectors phase (CocoIndex) and the graph phase + (`build_ast_graph.py`), so both phases render a real bar (determinate where the + denominator is knowable; indeterminate otherwise), not just an elapsed spinner. +- `install`/`update` stop being silent (`update` drops `quiet=True`) and their + indexing progress moves off stdout onto the stderr renderer, while each command's + stdout contract is preserved. + +## Principles (do not relitigate in review) + +- **stdout = machine payload; stderr = human progress.** No command writes indexing + progress to stdout. `init`/`increment`/`reprocess` keep their JSON/pprint stdout; + `install`/`update` keep their human-readable wizard stdout. +- **CocoIndex stays a subprocess.** Do not switch to the in-process `app.update()`/ + `watch()` API — that re-introduces the native-thread shutdown crash the subprocess + isolation exists to avoid (`_console_script_main` → `os._exit`). +- **One renderer, one code path.** `install`/`update` are un-silenced to engage the + same renderer the operator commands use — they are **not** wrapped in + `_run_with_pipeline_progress` (that would put wizard prompts under the + header/footer framing). +- **Determinate where the denominator is knowable; indeterminate where it isn't.** + Graph pass 1 is exactly determinate (count-first, single-layer ignore). Vectors is + *approximately* determinate on full reprocess (bar clamps to 100% on completion) + and indeterminate on incremental catch-up (`memo=True`). +- **Single stderr writer while the `rich` `Live` region is up.** Non-progress lines + route through `console.print(...)`; raw `sys.stderr.buffer.write` relay happens + only in `--verbose` (no `Live` region). +- **A task is `running` only after its subprocess actually spawns.** The 126/127 + pre-spawn stubs emit `status=failed` and never mark a task `running`. +- **Three verbosity tiers preserved.** `--quiet` = no progress stderr; default = + `rich` display; `--verbose` = raw subprocess relay (no `Live` region). +- **No new CLI flags.** Existing `--quiet`/`--verbose` flags are wired through on + `update` (both ignored today) and `install` (`--verbose` ignored today). + +## PR breakdown - overview + +| PR | Scope | Ontology bump | Areas of concern | Test buckets | Independent of | +| --- | --- | --- | --- | --- | --- | +| Spike | Validate CocoIndex relays flow-function stderr; size pre-walk divergence | none | none (throwaway, no merge) | none (go/no-go note) | — | +| PR-1 | `rich` dep + `progress.py` (parser + renderer + non-TTY fallback + relay invariant); unit tests | none | parser vs existing `_LineFilter`/relay; rich `Live` single-writer; non-TTY fallback | progress unit (light) | Spike | +| PR-2 | Graph-phase `JCIRAG_PROGRESS` emission (`build_ast_graph.py` count-first pass 1 + passes 2–6); wire renderer into operator commands' graph phase | none | pass-1 count-first cost; emission gating; `_run_with_pipeline_progress` ↔ renderer integration | graph progress + CLI (light) | PR-1 | +| PR-3 | Vectors-phase `JCIRAG_PROGRESS` emission (`process_*_file` + approximate pre-walk); wire renderer into vectors phase incl. both `Optimize` call sites; retire `emit_vectors_*` | none | two-layer filtering / approximate denominator; `memo=True` indeterminacy; two optimize call sites; flow-stderr assumption | flow progress (heavy-gated) + divergence | PR-1, PR-2 | +| PR-4 | Un-silence `install`/`update` subprocess calls; wire `--quiet`/`--verbose` through `update`/`install`; docs | none | wizard stdout contract preserved; verbosity wiring; renderer wraps indexing sub-step only | installer + CLI (light) | PR-2, PR-3 | + +Landing order: **Spike (gate) → PR-1 → PR-2 → PR-3 → PR-4**. Do not start PR-1 until +the spike passes; do not start PR-N+1 until PR-N is merged to `master`. + +## Resolved design decisions + +| Topic | Decision | +| --- | --- | +| Renderer library | `rich` (`rich>=13.7,<14`); parent-process only. | +| New module | `java_codebase_rag/progress.py` holds `ProgressEvent`, `parse_progress_line`, `IndexProgressRenderer`, `ProgressRelay`. | +| Protocol line | `JCIRAG_PROGRESS kind= [phase=…] [pass=N/6] [done=N] [total=N] [status=running\|done\|failed] [elapsed_s=…]` | +| Drain integration | `pipeline._popen_capturing_stderr` and `cli_progress.accumulate_and_relay_subprocess_streams` gain an optional `on_progress` callback and route complete lines through a `ProgressRelay` (parse-first; progress lines suppressed from relay). | +| Emission gating | `build_ast_graph.py` emits `JCIRAG_PROGRESS` under its existing `--verbose` path (the parent already passes `--verbose` for default+verbose, only suppresses it for `--quiet`). The flow emits `JCIRAG_PROGRESS` unconditionally to its stderr; parent mode controls handling. No new CLI flags on `build_ast_graph.py`. | +| Single stderr writer | While the `rich` `Live` region is active, `ProgressRelay` prints non-progress lines via `console.print(...)`; raw `buffer.write` relay runs only when there is no `Live` region (`--verbose`). | +| Graph pass-1 total | Count-first: one filtered `os.walk` (no parse) sets the exact total, then the parse loop ticks per file. Single-layer ignore → exact. | +| Vectors full-reprocess total | Approximate pre-walk (matcher includes + layered-ignore); bar clamps to 100% on `status=done`. Authoritative-count-from-flow is the escalation path if the spike's divergence test shows a large gap. | +| Vectors incremental total | `total=None` (indeterminate pulsing bar) + "files touched: N" counter (`memo=True` only calls the function for changed files). | +| `Optimize` phase | Surfaced as its own task; auto-collapses to a vectors sub-state when it completes under ~1 s (Open Q4). Both call sites emit `kind=optimize`. | +| `Spinner` | Retired (only caller is the vectors phase, replaced by the renderer). | +| `install`/`update` wiring | Un-silence the `run_cocoindex_update`/`run_build_ast_graph` calls (drop `quiet=True`, pass progress context); do not wrap the wizards. `update` gets `--quiet`/`--verbose` wired through `_cmd_update`/`run_update`; `install` gets `--verbose` wired. | + +--- + +# Spike — validate CocoIndex flow-function stderr relay (gate, no PR) + +**Branch:** throwaway (e.g. `spike/flow-stderr`), not merged. **Not** a deliverable on +`master`. + +## Objective + +Confirm the load-bearing assumption: a line written to `sys.stderr` from inside +`process_java_file` (`java_index_flow_lancedb.py`) reaches the parent process that +spawned `cocoindex update`. Also size the vectors pre-walk divergence. + +## Steps + +### 1. `java_index_flow_lancedb.py` (throwaway edit, not merged) +- At the top of `process_java_file`, before the ignore check, emit one line: + `print("JCIRAG_PROGRESS kind=vectors phase=java done=0 total=0 status=running", file=sys.stderr, flush=True)`. + +### 2. Drive + capture +- Run `cocoindex update java_index_flow_lancedb.py:JavaCodeIndexLance --full-reprocess` + against `tests/bank-chat-system` with stdout/stderr captured to pipes (mirror + `pipeline._popen_capturing_stderr`'s capture). +- Inspect captured stderr for the `JCIRAG_PROGRESS` line. + +### 3. Pre-walk divergence +- Pre-walk `tests/bank-chat-system` with the matcher includes + `LayeredIgnore` + (`cocoindex_excluded_patterns()` + `is_ignored`) and record the count. +- Record the actual `done` at completion. +- Compute the gap (ignored/empty/undecodable files). + +## Done when + +- **Go/no-go written** (a one-paragraph note on the throwaway branch): either + "stderr relays — proceed with PR-1" with the divergence number, or + "stderr suppressed — halt, re-propose transport". The gate is binary. + +## Result — **GO** (2026-06-14) + +Spike passed. A `print("JCIRAG_PROGRESS …", file=sys.stderr, flush=True)` at the top +of `process_java_file`, run via `cocoindex update … --full-reprocess` on +`tests/bank-chat-system`, produced **130** `JCIRAG_PROGRESS` lines in captured stderr +(cocoindex exit 0) — flow-function stderr reaches the parent unmodified, no +suppression/buffering. Pre-walk divergence: **0** (130 non-ignored `.java` files == +130 processed). PR-1 is unblocked. + +--- + +# PR-1 — `rich` dep + `progress.py` (parser, renderer, non-TTY fallback, relay) + +**Branch:** `feat/index-progress-protocol` off `master` **after the spike passes**. +No command wiring, no flow/builder emission in this PR — pure library + unit tests. + +## File-by-file changes + +### 1. `pyproject.toml` +- Add `rich>=13.7,<14` to `dependencies`. + +### 2. `java_codebase_rag/progress.py` (new) +- `ProgressEvent` dataclass: `kind` (`Literal["vectors","graph","optimize"]`), + `phase: str | None`, `pass_: str | None` (e.g. `"3/6"`), `done: int | None`, + `total: int | None`, `status: Literal["running","done","failed"]`, `elapsed_s: float | None`. +- `parse_progress_line(line: bytes) -> ProgressEvent | None` — returns `None` for any + line not starting with the `JCIRAG_PROGRESS` prefix; parses `key=value` tokens; + tolerates extra spaces; never raises. +- `IndexProgressRenderer`: + - `__init__(self, phases: list[str], *, console: Console | None = None)` — builds a + `rich.progress.Progress` (TTY) or prepares the concise-line fallback (non-TTY); + one task per phase, all `total=None` until a `done`/`total` event arrives; + - `start()` / `stop()` — enter/exit the `Live` region (TTY) or no-op (non-TTY); + - `apply(self, ev: ProgressEvent)` — update the matching task: on `total` set the + denominator; on `done` advance; on `status=done` **clamp completed to total** and + mark the task finished; on `status=failed` mark the task failed (red `✗`); + - a task is only marked visible/`running` after the first event for its `kind` + arrives (so a never-spawned phase stays pending, never `running`); + - the non-TTY fallback prints concise lines on `apply` at most every ~5 s per phase + plus on every terminal (`done`/`failed`) event. +- `ProgressRelay`: + - wraps the existing line-buffering used by `_LineFilter` / `_AsyncLineFilter`; + - `feed(self, chunk: bytes) -> None` — reassembles complete lines; for each, run + `parse_progress_line` first; if it returns an event, call `renderer.apply(ev)` and + **suppress** the line from relay; otherwise hand the line to the configured + sink: `console.print(...)` while the `Live` region is up, or raw + `sys.stderr.buffer.write` when relaying verbatim (`--verbose`, no `Live` region); + non-progress lines still pass through the `_NOISE_CONTAINS` noise matcher before + the sink; + - `flush()` — drain the partial buffer. + +### 3. `tests/test_progress.py` (new) +- Pure unit tests against `progress.py`; no subprocess, no cocoindex, no torch — + fully light. + +## Tests for PR-1 + +1. `test_parse_progress_line_vectors_running` +2. `test_parse_progress_line_graph_pass` +3. `test_parse_progress_line_optimize_running` +4. `test_parse_progress_line_done_with_elapsed` +5. `test_parse_progress_line_non_progress_returns_none` +6. `test_parse_progress_line_malformed_returns_none` +7. `test_progress_relay_parses_split_chunk_once` (one logical line split across two + `feed()` calls → exactly one `apply` call, no relay of the raw line) +8. `test_progress_relay_relays_non_progress_line` (a non-progress line reaches the sink) +9. `test_renderer_task_pending_until_first_event` (no event ⇒ task not `running`) +10. `test_renderer_clamps_completed_to_total_on_done` +11. `test_renderer_indeterminate_total_none` (`total=None` ⇒ pulsing bar, no `%`) +12. `test_renderer_failed_marks_task_red` +13. `test_non_tty_fallback_emits_concise_lines` (renderer constructed with a non-TTY + console ⇒ prints interval lines + a terminal line) + +## Definition of done (PR-1) + +- [ ] `rich>=13.7,<14` in `pyproject.toml`; `.venv/bin/pip install -e .` succeeds. +- [ ] `progress.py` exists with the four symbols above; no production caller yet. +- [ ] All 13 `tests/test_progress.py` tests pass. +- [ ] `.venv/bin/ruff check .` clean; `.venv/bin/python -m pytest tests -v` green + (no new heavy gating introduced — all new tests are light). +- [ ] No `JCIRAG_PROGRESS` emission added to production files yet. + +## Implementation step list + +| # | Step | File(s) | Done when | +| - | - | - | - | +| 1 | Add `rich` dep | `pyproject.toml` | `pip install -e .` pulls rich | +| 2 | `ProgressEvent` + `parse_progress_line` | `progress.py` | parser tests 1–6 pass | +| 3 | `ProgressRelay` (split-chunk, parse-first, single-writer sink) | `progress.py` | relay tests 7–8 pass | +| 4 | `IndexProgressRenderer` (TTY `rich` + non-TTY fallback + clamp) | `progress.py` | renderer tests 9–13 pass | +| 5 | Full suite green | — | `ruff` + `pytest tests -v` pass | + +--- + +# PR-2 — graph-phase progress (operator commands) + +**Branch:** `feat/index-graph-progress` off `master` **after PR-1 merged**. + +## File-by-file changes + +### 1. `build_ast_graph.py` +- Add a `_emit_progress(parts: dict[str,str])` helper that writes one + `JCIRAG_PROGRESS kind=graph …` line to stderr, flushed, gated by the existing + verbose flag (so the parent's `--verbose`-on-non-quiet path emits it; `--quiet` + suppresses the whole subprocess relay anyway). +- `pass1_parse`: add a cheap count-first step — one filtered `os.walk` over + `iter_java_source_files(root, ignore=ignore)` semantics (no parsing) to set the + total, then emit `pass=1 total=N` once and a `done=k` tick per parsed file + (throttled every ~25 files + on pass completion). +- `pass2_edges` … `pass6_match_edges`: emit `pass=N/6 status=running` on entry and + `pass=N/6 status=done elapsed_s=…` on exit. +- Keep the existing `[graph] pass N` heartbeat lines for `--verbose` raw relay; the + new structured lines are additive. + +### 2. `java_codebase_rag/pipeline.py` +- `_popen_capturing_stderr`: accept an optional `on_progress: Callable[[ProgressEvent], None] | None`. + Replace the inline `_LineFilter` drain with a `ProgressRelay` that, per complete + line, parses first; progress events → `on_progress`; non-progress → existing + noise/relay path (raw `buffer.write` here, since this helper is the verbatim-relay + path used in default+verbose; the `console.print` routing is used by the renderer + context in step 4). +- `run_build_ast_graph` / `run_incremental_graph`: thread `on_progress` through from + the caller; when `--verbose` is the mode, do not attach a renderer (raw relay) — + pass `on_progress=None`. + +### 3. `java_codebase_rag/cli_progress.py` +- `accumulate_and_relay_subprocess_streams` (async, used by `server.run_refresh_pipeline`): + same `ProgressRelay` + `on_progress` wiring as the sync helper. + +### 4. `java_codebase_rag/cli.py` +- Introduce a renderer context around `_run_with_pipeline_progress`'s `work()` (TTY + only; `--quiet` skips it, `--verbose` skips it). For this PR it owns the **graph** + task: create the phase list from the command's phase set, mark the graph task + `running` only once `run_build_ast_graph`/`run_incremental_graph` actually spawns, + feed `ProgressRelay` events into the renderer, and route non-progress relay lines + through `console.print` while the `Live` region is up. +- `_cmd_init`, `_cmd_increment`, `_cmd_reprocess`: pass the graph-phase `on_progress` + callback through the pipeline helpers. The vectors task remains pending in this PR + (PR-3 fills it). + +### 5. `tests/test_ast_graph_build.py` +- Add graph-progress tests (run the builder against `tests/bank-chat-system`; assert + on stderr lines, not ANSI). + +### 6. `tests/test_java_codebase_rag_cli.py` +- Add CLI-level assertions that graph-phase progress reaches stderr in default mode + and is absent in `--quiet`, by patching the pipeline helpers to a fixture subprocess + that emits known `JCIRAG_PROGRESS` lines. + +## Tests for PR-2 + +1. `test_build_ast_graph_pass1_emits_per_file_progress` (count-first: a `total=` line + precedes the first `done=`; ticks advance) +2. `test_build_ast_graph_pass1_total_is_exact_filtered_count` (total == count of + non-ignored `.java` files in the fixture) +3. `test_build_ast_graph_passes_2_to_6_emit_step_progress` (six `pass=N/6` lines) +4. `test_build_ast_graph_quiet_emits_no_progress` (`--quiet` ⇒ no `JCIRAG_PROGRESS`) +5. `test_cli_init_default_mode_graph_phase_progress_on_stderr` (patched helper emits + a graph line; assert it is parsed and not raw-relayed in default mode) +6. `test_cli_increment_graph_phase_progress` (symmetric) +7. `test_cli_graph_progress_absent_when_quiet` + +(Tests 5–7 patch the pipeline helpers so they do **not** require cocoindex/torch and +run in the default light suite.) + +## Definition of done (PR-2) + +- [ ] `build_ast_graph.py` emits `kind=graph` progress (count-first pass 1 exact total; + passes 2–6 step) under the non-quiet path. +- [ ] `_popen_capturing_stderr` and the async drain route progress events to a caller + callback and suppress them from raw relay. +- [ ] `init`/`increment`/`reprocess` render the graph task (determinate) in default + TTY mode; `--quiet` shows nothing; `--verbose` raw-relays. +- [ ] All 7 PR-2 tests pass; `ruff` + full `pytest tests -v` green. +- [ ] `install`/`update` unchanged in this PR (still `quiet=True`). + +## Implementation step list + +| # | Step | File(s) | Done when | +| - | - | - | - | +| 1 | `_emit_progress` helper + pass 2–6 step lines | `build_ast_graph.py` | tests 3–4 pass | +| 2 | Count-first pass 1 + per-file ticks | `build_ast_graph.py` | tests 1–2 pass | +| 3 | `on_progress` plumbing in sync + async drains | `pipeline.py`, `cli_progress.py` | relay routes events, suppresses raw | +| 4 | Renderer context for graph task in operator commands | `cli.py` | tests 5–7 pass | +| 5 | Full suite green | — | `ruff` + `pytest tests -v` pass | + +--- + +# PR-3 — vectors-phase progress (operator commands) + retire `emit_vectors_*` + +**Branch:** `feat/index-vectors-progress` off `master` **after PR-2 merged**. + +## File-by-file changes + +### 1. `java_index_flow_lancedb.py` +- Add a small `_emit_vectors_progress(kind, done, total, status, elapsed_s=None)` + helper writing `JCIRAG_PROGRESS kind=vectors …` to stderr (flushed). +- In `app_main`, after mounting, emit a one-shot approximate total: walk the three + matchers (`**/*.java`, `…/migration/*.sql`, `application*.yml`) through + `LayeredIgnore` (`cocoindex_excluded_patterns()` + `is_ignored`) and emit + `kind=vectors total=N status=running` (approximate — ignored/empty/undecodable files + overstate it; the parent clamps on completion). +- In each `process_*_file`, increment a module-level atomic counter and emit a + `done=k` tick every ~25 files and on the final file (`status=done elapsed_s=…`). + On incremental catch-up (`memo=True` cache hit ⇒ function not called) no total is + known — the parent renders indeterminate from the absence of a `total` event. + +### 2. `java_codebase_rag/pipeline.py` +- `_run_cocoindex_update_impl`: the `on_progress` plumbing from PR-2 now also carries + `kind=vectors` events. Drop the `Spinner` and the `emit_vectors_start`/`_finish` + calls; the renderer owns the vectors task instead. A vectors task is marked + `running` only after the `cocoindex` `Popen` succeeds (not on the 127 stub). + +### 3. `server.py` +- `run_refresh_pipeline`: route the async-drain vectors events into the same renderer; + drop `emit_vectors_start`/`emit_vectors_finish` here too. The serialized optimize + block (`server.py:359-372`) emits `kind=optimize status=running` / `status=done`. + +### 4. `java_codebase_rag/lance_optimize.py` +- `optimize_lance_tables`: emit `JCIRAG_PROGRESS kind=optimize status=running` on + entry and `status=done elapsed_s=…` on exit — this is the **second** optimize call + site (`_maybe_run_serialized_optimize` in `pipeline.py:129` calls it), so both + reprocess-default and init/increment paths surface the phase consistently. + +### 5. `java_codebase_rag/cli_progress.py` +- Remove `emit_vectors_start` / `emit_vectors_finish` (no remaining callers after + steps 2–3). Keep `_AsyncLineFilter` logic folded into `ProgressRelay` (PR-2). + +### 6. `java_codebase_rag/cli_format.py` +- Remove the `Spinner` class (only caller was the vectors phase, now retired). + +### 7. `java_codebase_rag/cli.py` +- Extend the renderer context from PR-2 to own the **vectors** and **optimize** tasks + for the operator commands: vectors task `running` after `cocoindex` spawns; + optimize task on the optimize events; phase list order + `Vectors → Optimize → Graph`. + +### 8. `tests/test_java_codebase_rag_cli.py` and a new `tests/test_vectors_progress.py` +- Heavy (cocoindex) flow tests gated behind `JAVA_CODEBASE_RAG_RUN_HEAVY=1`; the + parser/divergence/clamp tests stay light via patched helpers. + +## Tests for PR-3 + +1. `test_flow_emits_vectors_progress_per_file` (**heavy-gated**) — run + `cocoindex update` on the fixture, assert `JCIRAG_PROGRESS kind=vectors` lines + appear in captured stderr (the spike, promoted to a regression test). +2. `test_vectors_progress_clamps_on_completion` (light — feed a synthetic event + stream through the renderer: `total=100`, `done=80`, `status=done` ⇒ completed + clamps to 100). +3. `test_vectors_progress_approximate_total_overstates_then_clamps` (light — feed + `total=100` then `done=95 status=done`; assert clamp, no 95% stall). +4. `test_vectors_incremental_renders_indeterminate` (light — no `total` event ⇒ + pulsing bar / `MofN` shows `?`). +5. `test_pre_walk_total_divergence_bounded` (**heavy-gated**) — on the fixture, + pre-walk total vs. actual `done` differ only by the ignored/empty count. +6. `test_cli_init_vectors_phase_progress_on_stderr` (light — patched cocoindex helper + emits a vectors line; parsed, not raw-relayed). +7. `test_cli_reprocess_optimize_phase_progress` (light — patched optimize emits + `kind=optimize`; phase renders). +8. `test_spinner_removed_and_emit_vectors_helpers_removed` (light — import guard: + `Spinner` and `emit_vectors_start`/`_finish` no longer exist). + +## Definition of done (PR-3) + +- [ ] `process_*_file` emit `kind=vectors` progress; approximate total emitted from + `app_main`; bar clamps to 100% on completion. +- [ ] Both optimize call sites (`run_refresh_pipeline`, `optimize_lance_tables`) emit + `kind=optimize`. +- [ ] `Spinner`, `emit_vectors_start`, `emit_vectors_finish` removed; no remaining + callers/imports. +- [ ] Operator commands render the full `Vectors → Optimize → Graph` list in default + TTY mode. +- [ ] All 8 PR-3 tests pass (heavy-gated ones skip without the env var); `ruff` + + full `pytest tests -v` green. + +## Implementation step list + +| # | Step | File(s) | Done when | +| - | - | - | - | +| 1 | Vectors emission helper + per-file ticks + approximate total | `java_index_flow_lancedb.py` | test 1 (heavy) passes | +| 2 | Optimize emission at both call sites | `server.py`, `lance_optimize.py` | test 7 passes | +| 3 | Retire `Spinner` + `emit_vectors_*`; wire vectors/optimize tasks | `pipeline.py`, `cli_progress.py`, `cli_format.py`, `cli.py`, `server.py` | test 8 passes; no dangling imports | +| 4 | Renderer clamp/indeterminate/divergence coverage | `tests/` | tests 2–6 pass | +| 5 | Full suite green | — | `ruff` + `pytest tests -v` pass | + +--- + +# PR-4 — installer alignment (`install`/`update`) + verbosity wiring + docs + +**Branch:** `feat/index-installer-progress` off `master` **after PR-3 merged**. + +## File-by-file changes + +### 1. `java_codebase_rag/installer.py` +- `run_init_if_needed`: replace the plain `print("Creating index…")` / + `print("Index created successfully.")` chatter around the indexing calls with the + renderer context (vectors + optimize + graph tasks); drop `quiet=` silence so the + subprocess calls engage the renderer; keep the wizard's other prompts/summaries on + stdout unchanged. +- `run_update`: drop `quiet=True` on `run_cocoindex_update` / `run_incremental_graph`; + wrap those calls in the renderer context (not `_run_with_pipeline_progress`); accept + and forward `quiet`/`verbose`. +- Replace the stdout `print("\nUpdating index (Lance + graph)…")` / error prints with + stderr renderer framing where they describe indexing progress. + +### 2. `java_codebase_rag/cli.py` +- `_cmd_update`: forward `quiet=bool(args.quiet)` and `verbose=bool(args.verbose)` to + `run_update` (both are ignored today). +- `_cmd_install`: forward `verbose=bool(args.verbose)` to `run_install` (only `quiet` + is wired today). + +### 3. `docs/JAVA-CODEBASE-RAG-CLI.md` +- Document the unified progress output (header / phase list / footer on stderr), the + determinate-vs-indeterminate behaviour per command, and the + `--quiet` / `--verbose` / non-TTY behaviour. +- Note `install`/`update` now emit indexing progress on stderr (behaviour change) and + that their stdout wizard text is otherwise unchanged. + +### 4. `README.md` +- One-line note in the lifecycle section that indexing shows a progress bar; mention + the `rich` dependency. + +### 5. `tests/test_installer.py` and `tests/test_java_codebase_rag_cli.py` +- Installer/CLI tests asserting progress reaches stderr and stdout is preserved. + +## Tests for PR-4 + +1. `test_install_emits_indexing_progress_on_stderr` (patch the pipeline helpers to + emit known `JCIRAG_PROGRESS` lines; assert they reach stderr; wizard stdout + prompts still present on stdout) +2. `test_update_emits_indexing_progress_on_stderr` (symmetric; asserts `update` is no + longer silent) +3. `test_update_runs_indexing_without_quiet_true` (assert `run_cocoindex_update` / + `run_incremental_graph` are called with `quiet=False` in the default path) +4. `test_cmd_update_forwards_quiet_flag` (`_cmd_update --quiet` ⇒ `run_update(quiet=True)`) +5. `test_cmd_update_forwards_verbose_flag` (`_cmd_update --verbose` ⇒ `run_update(verbose=True)`) +6. `test_cmd_install_forwards_verbose_flag` +7. `test_install_update_stdout_contract_preserved` (the wizard's human-readable stdout + shape is unchanged; no `JCIRAG_PROGRESS` line leaks to stdout) + +## Definition of done (PR-4) + +- [ ] `install`/`update` render the unified phase list on stderr during indexing; the + wizards' stdout text is otherwise unchanged. +- [ ] `update` no longer runs indexing with `quiet=True`. +- [ ] `--quiet`/`--verbose` wired through `_cmd_update`/`run_update`; `--verbose` + through `install`. +- [ ] All 7 PR-4 tests pass; `ruff` + full `pytest tests -v` green. +- [ ] CLI docs + README updated. + +## Implementation step list + +| # | Step | File(s) | Done when | +| - | - | - | - | +| 1 | Un-silence + renderer context in `run_init_if_needed` / `run_update` | `installer.py` | tests 1–3 pass | +| 2 | Forward verbosity flags | `cli.py` | tests 4–6 pass | +| 3 | stdout-contract regression | `tests/` | test 7 passes | +| 4 | Docs | `docs/JAVA-CODEBASE-RAG-CLI.md`, `README.md` | docs match behaviour | +| 5 | Full suite green | — | `ruff` + `pytest tests -v` pass | + +--- + +# Cross-PR risks and mitigations + +| # | Risk | Severity | Mitigation | +| --- | --- | --- | --- | +| 1 | CocoIndex suppresses flow-function stderr → PR-3's vectors ticks never arrive | High | **Spike gate before PR-1.** If it fails, halt and re-propose the transport (file-tail is not a committed fallback). | +| 2 | Two concurrent stderr writers (relay `buffer.write` + `rich` `Live`) corrupt the display | High | `ProgressRelay` routes non-progress lines through `console.print` while `Live` is up; raw relay only in `--verbose`. Test `test_progress_relay_parses_split_chunk_once`. | +| 3 | Vectors pre-walk overstates the total (two-layer ignore + early-returns) | Medium | Bar clamps to 100% on `status=done`; divergence test (`test_pre_walk_total_divergence_bounded`) sizes the gap; authoritative-count-from-flow is the documented escalation. | +| 4 | Graph pass-1 count-first doubles the walk cost | Low | Count step is a metadata-only `os.walk` with ignore filtering (no parse); negligible vs. the parse loop. | +| 5 | Missing `cocoindex`/builder binary (126/127) leaves a task hung at `running` | Medium | Task marked `running` only after `Popen` spawns; stubs emit `status=failed`. Test in PR-1 renderer suite + PR-2/3 wiring. | +| 6 | Retiring `Spinner`/`emit_vectors_*` leaves a dangling import | Medium | PR-3 test `test_spinner_removed_and_emit_vectors_helpers_removed` + full-suite import check. | +| 7 | PR-2/PR-3/PR-4 all touch the operator-command renderer context → merge conflicts | Medium | Strict landing order (PR-2 → PR-3 → PR-4); each PR rebase-merges the previous. | +| 8 | `update` verbosity wiring changes observable behaviour for existing scripts | Low | `--quiet`/`--verbose` already existed on the parser (ignored); wiring them through matches siblings, no new flags. | +| 9 | Heavy (cocoindex/torch) tests added to the default suite slow CI / break the segfault-isolation work | Medium | All cocoindex-flow tests gated behind `JAVA_CODEBASE_RAG_RUN_HEAVY=1`; parser/renderer/CLI tests use patched helpers and stay light. | + +# Out of scope + +- Switching the vectors phase to in-process `cocoindex` `app.update()`/`watch()`. +- A determinate denominator for incremental catch-up (would require diffing the memo store). +- Parallelising the two phases. +- Giving `install`/`update` a machine-readable stdout JSON payload. +- Splitting `init`/`increment` the way `reprocess` is split. +- Drift detection between Lance and LadybugDB stores. +- Any schema/ontology/enrichment change. + +# Whole-plan done definition + +1. All five lifecycle commands render the unified header / phase-list / footer on + stderr during indexing; `init`/`increment`/`reprocess` keep their stdout payload + and `install`/`update` keep their wizard stdout. +2. Both phases render real progress — graph determinate (exact count-first total), + vectors determinate-approximate on full reprocess (clamp on completion) and + indeterminate on catch-up; optimize surfaced (auto-collapsing when sub-second). +3. `JCIRAG_PROGRESS` is the single cross-process protocol; `ProgressRelay` enforces + the parse-first / single-writer invariants; split-chunk and missing-binary cases + are covered. +4. `Spinner` and `emit_vectors_start`/`_finish` are gone; `--quiet`/`--verbose` are + wired through `update`/`install`. +5. `ruff` + full `pytest tests -v` green with no new heavy tests in the default suite; + heavy-gated vectors/divergence tests pass under `JAVA_CODEBASE_RAG_RUN_HEAVY=1`. + +# Tracking + +- Spike: **GO — passed 2026-06-14** (130/130 `JCIRAG_PROGRESS` lines relayed to parent stderr; pre-walk divergence 0 on the fixture) +- `PR-1`: _pending_ +- `PR-2`: _pending_ +- `PR-3`: _pending_ +- `PR-4`: _pending_ diff --git a/propose/active/INDEX-OUTPUT-REWORK-PROPOSE.md b/propose/active/INDEX-OUTPUT-REWORK-PROPOSE.md new file mode 100644 index 0000000..43466af --- /dev/null +++ b/propose/active/INDEX-OUTPUT-REWORK-PROPOSE.md @@ -0,0 +1,453 @@ +# INDEX-OUTPUT-REWORK — Unified progress-bearing index-build output across `init` / `increment` / `install` / `reprocess` / `update` + +## Status +Proposal — not yet implemented. Design aligned in brainstorming session 2026-06-14; +revised after an adversarial self-review; **gating spike PASSED 2026-06-14**. + +The load-bearing assumption — that CocoIndex does not suppress/buffer stderr written +from inside `@coco.fn` flow functions — is **confirmed**. A throwaway spike emitted +one `JCIRAG_PROGRESS` line from `process_java_file` and ran `cocoindex update` on +`tests/bank-chat-system`: 130 lines reached captured stderr (cocoindex exit 0), zero +suppression/buffering. Pre-walk divergence measured **0** on the fixture (130 +non-ignored `.java` files == 130 processed). The plan's PR-1 is unblocked; the +clamp-on-completion safeguard (§ Vectors phase) remains for the general case. + +## Problem Statement + +All five lifecycle commands build the index through the same two subprocesses — +`cocoindex update` (vectors → Lance) and `build_ast_graph.py` (graph → LadybugDB) — +but their **output during that build differs per command**, and **none shows real +progress**. + +Today's behaviour: + +| Command | Header/footer | Phase markers | Progress stream | Spinner / bar | +|---|---|---|---|---| +| `init` | ✓ `_run_with_pipeline_progress` | ✓ `[vectors]` / `[graph]` | **stderr** (stdout = JSON payload) | ✓ braille spinner (TTY only) | +| `increment` | ✓ | ✓ `[vectors]` / `[increment]` | stderr | ✓ spinner (TTY only) | +| `reprocess` | ✓ | mixed per-mode paths | stderr | partial | +| `install` | ✗ none | ✗ none | **stdout** via plain `print()` | ✗ | +| `update` | ✗ none | ✗ none (`run_…(quiet=True)`!) | stdout via `print()` | ✗ (silent) | + +Two concrete problems: + +1. **Inconsistent, sometimes missing output.** `install` and `update` are the + outliers: they bypass the shared `_run_with_pipeline_progress` framing, write + indexing chatter to **stdout** (breaking the "stdout = machine-readable payload, + stderr = human progress" contract the other three honour), and `update` runs the + whole indexing step with `quiet=True` — completely silent. Operators cannot tell + whether `update` is still working or hung. +2. **No progress, only elapsed time.** Every command's "progress" is either a + braille spinner with an elapsed-seconds counter or nothing. There is no + percentage, no items-done / items-total, no ETA. On a large tree the vectors + phase alone can run for minutes with the operator staring at `⠹ cocoindex update · 42s`. + +User asks this proposal addresses: + +- **Align** the index-build output across all five commands and make it beautiful + and user-friendly. +- **Add a progress bar** to the indexing process. + +## Proposed Solution + +One shared rendering path drives the index-build output for **all five commands**. +Both subprocesses emit structured progress lines the parent parses and feeds to a +single `rich`-based renderer. + +### Design principles + +1. **One renderer, one code path.** The operator commands render the indexing step + via `_run_with_pipeline_progress` (`cli.py`); `install`/`update`'s installer + call sites (`installer.run_init_if_needed`, `installer.run_update`) already + invoke the same `run_cocoindex_update` / `run_build_ast_graph` helpers, so they + are **un-silenced** (drop `quiet=True`, pass the progress context) to engage the + same renderer for just the indexing sub-step — not wrapped around the whole + wizard. No per-command output code. +2. **Progress is observable from inside the subprocesses.** The vectors phase emits + ticks from inside `process_java_file` / `process_sql_file` / `process_yaml_file` + (our code, called once per file by CocoIndex); the graph phase emits ticks from + `build_ast_graph.py` (our code, run as a subprocess). We control both sides of + the protocol. +3. **Determinate where the denominator is knowable; indeterminate where it isn't.** + A full reprocess can *approximate* the file count up front (see §Vectors phase — + it is approximate, not exact) → `%` bar that clamps to 100% on completion. + Incremental catch-up only sees changed files (CocoIndex `memo=True` cache) → + indeterminate pulsing bar with a "files touched: N" counter. Honest, never fake. +4. **stderr is for humans; stdout is the payload.** The renderer writes to stderr. + `init`/`increment`/`reprocess` keep their stdout JSON/pprint payload unchanged. + `install`/`update` keep their human-readable wizard stdout; only their indexing + progress moves off stdout onto the stderr renderer. +5. **Three verbosity tiers, preserved.** `--quiet` suppresses the whole progress + stream (payload unchanged); default is the rich display; `--verbose` relays raw + subprocess output verbatim (as today) for debugging. +6. **Subprocess isolation is not touched.** CocoIndex stays a subprocess + (`cocoindex update `). The deliberate isolation exists because native + lance/pyarrow worker threads crash on interpreter shutdown (that is why + `_console_script_main` calls `os._exit`). Switching to the in-process + `app.update()` API — which *does* expose `handle.watch()` progress — would + re-introduce that instability and is explicitly out of scope. + +### The progress protocol (`JCIRAG_PROGRESS`) + +A single, deliberately-prefixed line format so it cannot collide with relayed noise +and so the existing `_LineFilter` noise matcher is unaffected: + +``` +JCIRAG_PROGRESS kind=vectors phase=java done=842 total=1240 status=running +JCIRAG_PROGRESS kind=graph pass=3/6 done=1204 total=1204 status=running +JCIRAG_PROGRESS kind=optimize status=running +JCIRAG_PROGRESS kind=vectors status=done done=1240 total=1240 elapsed_s=42.1 +``` + +Fields: `kind` ∈ {`vectors`,`graph`,`optimize`}; `phase` (java/sql/yaml) optional; +`pass` = `N/6` for graph; `done`/`total` for determinate; `status` ∈ +{`running`,`done`,`failed`}; `elapsed_s` on completion. The parent's subprocess +stderr-drain thread already exists (`pipeline._popen_capturing_stderr`, +`cli_progress.accumulate_and_relay_subprocess_streams`); it gains a parser that, +for each `JCIRAG_PROGRESS` line, calls into the renderer and **does not relay the +raw line** to the terminal (it is consumed, not displayed). + +**Parse/relay ordering invariant.** Per complete line (the existing line-buffering +already reassembles lines split across `read()` chunks): the parser runs *first*; if +the line is `JCIRAG_PROGRESS` it updates the renderer and is suppressed from the +relay; otherwise the existing `_LineFilter` noise/relay path runs unchanged. A +regression test covers a progress line split across two chunks. + +**Single stderr writer.** `rich`'s `Live` region and the relay thread both target +stderr; two concurrent raw writers corrupt the display (`redirect_stderr=False` +only stops `rich` capturing Python's `sys.stderr`; it does not serialize the relay). +So while the `Live` region is active, the relay thread routes every non-progress +line through `rich`'s `console.print(...)` (which reprints the live region cleanly) +instead of writing raw bytes to `sys.stderr.buffer`. Raw `buffer.write` relay runs +only in `--verbose` mode, where there is no `Live` region. + +### Vectors phase + +`process_java_file` / `process_sql_file` / `process_yaml_file` increment a shared +counter per file (thread-safe — whether `coco.mount_each` parallelizes is part of +the gating spike) and print a `JCIRAG_PROGRESS` line every Nth file and on completion. + +- **Denominator (full reprocess — `init`, `reprocess` default, `reprocess --vectors-only`):** + **approximate, not exact.** A pre-walk reproduces the matcher's include globs + (`**/*.java`, `…/migration/*.sql`, `application*.yml`) plus the layered-ignore + logic, but CocoIndex applies *two* filtering layers — the `PatternFilePathMatcher` + excludes at walk time, then `LayeredIgnore.is_ignored()` plus an early-return for + empty / undecodable files *inside* each `process_*_file` + (`java_index_flow_lancedb.py:181,245,289`). Files that early-return never tick, so + a pre-walk overstates the total by the ignored/empty count. The bar therefore + **clamps to 100% on the `status=done` line** rather than asymptoting. The spike + includes a divergence test (pre-walk total vs. actual `done` at completion) to size + the gap; if it is large, the alternative is an authoritative count emitted *from + inside the flow* (Open Q3). +- **Denominator (incremental catch-up — `increment`, `update`, `increment --vectors-only`):** + CocoIndex's `@coco.fn(memo=True)` cache means the per-file function is only called + for changed files, so the total is unknown up front. → `total=None`, rich's + indeterminate pulsing bar, plus a "files touched: N" counter. +- **ETA:** derived by rich from the rate of `advance` calls (`TimeRemainingColumn`). + +### Graph phase + +`build_ast_graph.py` already emits `[graph] pass N` heartbeats in verbose mode; we +add the structured `JCIRAG_PROGRESS kind=graph` line. The graph builder applies +ignore filtering in a **single** layer (`iter_java_source_files(root, ignore=…)`), +so — unlike the vectors phase — its denominator is exact. + +- **Pass 1 (file parse):** today `pass1_parse` walks `iter_java_source_files` as a + generator and only knows the total *after* the walk + (`build_ast_graph.py:865-910`). To get per-file `done/total`, pass 1 gains a cheap + count-first step: one filtered `os.walk` (no parsing) to set the total, then the + existing parse loop ticks per file. Determinate. +- **Passes 2–6:** each advances the bar by 1/6 with a pass label + (`pass 3/6 · calls`). Determinate by construction (six known passes). + +### Renderer (`rich`) + +A `rich.progress.Progress` with one task per phase, writing to a stderr `Console`: + +```python +progress = Progress( + SpinnerColumn(), + TextColumn("[bold]{task.fields[label]}"), + BarColumn(), MofNCompleteColumn(), TaskProgressColumn(), + TextColumn("· {task.fields[detail]}"), + TimeRemainingColumn(), + console=Console(stderr=True), transient=False, +) +with progress: + v = progress.add_task("vectors", total=1240, label="Vectors") + o = progress.add_task("optimize", total=None, label="Optimize") # pending + g = progress.add_task("graph", total=None, label="Graph") # pending + # relay thread parses JCIRAG_PROGRESS → progress.update(v, advance=1) +``` + +`rich` auto-disables to plain text when not a TTY, handles terminal width/resize, +redraw, and interrupt cleanup, and `Progress.update` is thread-safe (RLock) so the +subprocess-drain thread can feed it. `transient=False` so the final state stays +visible. `rich` lives only in the parent CLI process; the heavy native stack +(torch/pyarrow) still loads in the CocoIndex *child* subprocess, so the dep adds no +native crash surface to the parent. + +**Task state must follow the subprocess, not the phase plan.** A task is marked +`running` only once the subprocess actually spawns; the pre-spawn checks (missing +`cocoindex` binary → `returncode=127` stub in `pipeline.py:168-174`; missing builder +→ `126`) emit `status=failed` and never mark the task `running`, so a missing binary +cannot leave a phase hung at `running` with no ticks. + +### Concrete rendering + +TTY (default): + +``` +java-codebase-rag init · source=/repo · index=/repo/.java-codebase-rag + + ◉ Vectors ████████████░░░░ 842/1240 (68%) · ~18s left + ○ Optimize pending + ○ Graph pending + +✓ java-codebase-rag init · finished in 86.4s +``` + +The `◉`/`○` state glyphs come from rich's spinner column (active vs pending task). +On a phase that is indeterminate (incremental catch-up), the bar renders as a +pulsing block and the `MofN` column shows `?`: + +``` + ◉ Vectors ◖◖◖◖ files touched: 37 · 9s +``` + +Non-TTY / CI (rich auto-disabled): the parser emits concise interval-based lines +(default every ~5 s + on completion) so CI logs still show progress: + +``` +java-codebase-rag init · source=/repo · index=/repo/.java-codebase-rag +vectors 842/1240 (68%) +vectors done · 1240 files · 42.1s +optimize done · 3.2s +graph pass 3/6 · calls +graph done · 6/6 · 31.4s +✓ java-codebase-rag init · finished in 86.4s +``` + +### Per-command matrix + +| Command | Phases in the list | Notes | +|---|---|---| +| `init` | `Vectors` → `Optimize` → `Graph` | first-time full build | +| `increment` | `Vectors` → `Optimize` → `Graph` | `Graph` indeterminate on catch-up; `--vectors-only` → `Vectors` → `Optimize` (keeps the LadybugDB-stale warning) | +| `reprocess` (default) | `Vectors` → `Optimize` → `Graph` | the serialized Lance optimize surfaces as its own phase | +| `reprocess --vectors-only` | `Vectors` → `Optimize` | keeps the drift warning | +| `reprocess --graph-only` | `Graph` | keeps the drift warning | +| `install` | `Vectors` → `Optimize` → `Graph` | wizard conversational text stays on stdout; renderer wraps only the indexing sub-step | +| `update` | `Vectors` → `Optimize` → `Graph` | no longer runs `quiet=True`; renderer wraps the indexing sub-step | + +`Optimize` is the serialized Lance compaction that already runs today after every +successful vectors phase (via `_maybe_run_serialized_optimize` / `optimize_lance_tables`). +It exposes no item count, so it is **always indeterminate**; `Vectors`/`Graph` are +determinate on full reprocess and indeterminate on incremental catch-up. + +For `install`/`update`, the renderer engages **only around the indexing subprocess +calls** inside `run_init_if_needed` / `run_update` — the wizard's own prompts and +summaries keep their existing stdout output and are *not* put under the +`_pipeline_header`/`_pipeline_footer` framing (that framing wraps the indexing +sub-step, not the whole wizard). Concretely: drop `quiet=True` and pass the progress +context to the `run_cocoindex_update` / `run_build_ast_graph` calls; do not wrap +`run_install` / `run_update` in `_run_with_pipeline_progress`. + +### Flags, TTY, and failure + +| Mode | Behaviour | +|---|---| +| TTY (default) | rich `Live` region — the multi-line phase display above | +| Non-TTY / CI | rich auto-disabled; concise interval-based stderr lines | +| `--quiet` / `-q` | suppress the entire progress stream; stdout payload unchanged (as today) | +| `--verbose` / `-v` | bypass parsing; relay raw subprocess output verbatim (as today) | +| Phase failure | failing task renders red `✗` + clipped error; footer `✗ … (exit=N)`; stdout payload keeps its current failure shape; rich `Live` torn down cleanly (not transient) so the error stays visible | +| Never spawned (missing `cocoindex` / builder binary → 126/127 stub) | task never marked `running`; renderer emits a `status=failed` line + the existing failure payload; no hung bar | + +`install` wires `--quiet` through today (not `--verbose`); `update` defines both +flags on its parser via `_add_verbosity_flags` but `_cmd_update` / `run_update` +ignore them entirely. This proposal wires `--quiet`/`--verbose` through both. + +## Scope + +### In scope + +- New dependency: `rich` (pure-Python; transitive `pygments`; no native code → no + crash risk). Pin in `pyproject.toml`. +- A new module (working name `java_codebase_rag/progress.py`) holding: the + `JCIRAG_PROGRESS` parser, the `rich` renderer wrapper, the non-TTY line fallback, + and the phase-list builder. +- `JCIRAG_PROGRESS` emission inside `process_java_file` / `process_sql_file` / + `process_yaml_file` (`java_index_flow_lancedb.py`) + an approximate total pre-walk + that reproduces the matcher includes + layered-ignore logic (see §Vectors phase). +- `JCIRAG_PROGRESS` emission inside `build_ast_graph.py`: a count-first step in + pass 1 (filtered walk, no parse) for an exact total, then per-file ticks; passes + 2–6 per-pass. +- Wire the renderer into all five commands' indexing steps; **un-silence** the + subprocess calls in `installer.run_init_if_needed` / `installer.run_update` + (drop `quiet=True`, pass the progress context) — do not wrap the wizards. +- Retire the now-redundant per-command markers (`emit_vectors_start`/`_finish`, the + inline `[graph] done` / `[increment] done` prints) in favour of the renderer. +- Wire `--quiet`/`--verbose` through `_cmd_update` / `run_update` (both flags exist + on `update`'s parser today but are ignored entirely) and `--verbose` through + `install` (which wires only `--quiet` today). + +### Files affected + +| File | Change | +|---|---| +| `pyproject.toml` | add `rich` dependency | +| `java_codebase_rag/progress.py` | **new** — parser + rich renderer + non-TTY fallback + phase-list builder | +| `java_codebase_rag/cli_format.py` | `Spinner` retired (its only caller is `pipeline.py:210`, the vectors phase); ANSI helpers reused by non-TTY fallback | +| `java_codebase_rag/cli_progress.py` | `emit_vectors_start`/`_finish` retired; subprocess-drain threads gain a `JCIRAG_PROGRESS` consumer | +| `java_codebase_rag/pipeline.py` | `_popen_capturing_stderr` + the async drain in `cli_progress` parse `JCIRAG_PROGRESS` and feed the renderer instead of (or alongside) relaying | +| `java_codebase_rag/cli.py` | `_run_with_pipeline_progress` drives the renderer; `_cmd_init`/`_cmd_increment`/`_cmd_reprocess` pass phase lists | +| `java_codebase_rag/installer.py` | `run_init_if_needed`, `run_update`: un-silence the `run_cocoindex_update` / `run_build_ast_graph` calls (drop `quiet=True`, pass progress context); drop plain `print()` indexing chatter | +| `server.py` | `run_refresh_pipeline`'s vectors phase feeds the same renderer (reprocess default path); **two** optimize call sites — `run_refresh_pipeline` (`server.py:359-372`) and `_maybe_run_serialized_optimize` (`pipeline.py:129`) — both emit `JCIRAG_PROGRESS kind=optimize` for consistent phase display | +| `java_index_flow_lancedb.py` | `JCIRAG_PROGRESS` emission in the three `process_*_file` functions + approximate total pre-walk | +| `build_ast_graph.py` | count-first step in pass 1 (exact total) + per-file `JCIRAG_PROGRESS`; passes 2–6 per-pass | +| `docs/JAVA-CODEBASE-RAG-CLI.md` | document the new progress output + non-TTY/`--quiet`/`--verbose` behaviour | +| `README.md` | note `rich` dep + one-line progress mention if the lifecycle section warrants it | + +## Schema / Ontology / Re-index impact + +- **Ontology bump:** not required. No graph/edge semantic change; `ontology_version` + stays at 17. +- **Re-index required:** no. Pure output/UX change; index artefacts and payload + shapes are unchanged. +- **Config/tool surface changes:** none. No new env vars, no new CLI flags. The + `--quiet`/`--verbose` flags already exist on every lifecycle subparser; this wires + the (currently ignored) ones on `update`/`install` through. Flag semantics are + preserved, not redefined. + +## Tests / Validation + +Per `tests/README.md` — assert invariants, not exact formatting; never special-case +the `tests/bank-chat-system/` fixture. + +- **Protocol contract:** new tests assert a `JCIRAG_PROGRESS` line written from + inside a flow function reaches the parent's captured stderr (the gating spike, + promoted to a regression test). +- **install/update now emit progress:** new tests assert `install`/`update` emit + indexing progress on **stderr** during the indexing step (today they emit nothing + on stderr); their stdout machine-readable shape (where present) is unchanged. +- **Operator commands unchanged payload:** `init`/`increment`/`reprocess` keep their + stdout JSON/pprint payload exactly; new tests pin that no `JCIRAG_PROGRESS` line + leaks onto stdout. +- **Regression anchor:** `increment` (no flag, full path) must NOT emit the + LadybugDB-stale warning, while `increment --vectors-only` must STILL emit it + (`cli.py:321-324`); `reprocess --vectors-only` / `--graph-only` drift warnings + still fire. +- **Verbosity tiers:** `--quiet` produces no progress stderr; `--verbose` relays + raw subprocess output (assert a known raw line passes through unfiltered). +- **Failure:** a failing phase renders a non-zero exit + error payload on stdout; + the renderer does not mask it. +- **Never-spawned:** missing `cocoindex`/builder binary (126/127 stub) emits + `status=failed` and leaves no task hung at `running`. +- **Protocol robustness:** a `JCIRAG_PROGRESS` line split across two `read()` chunks + parses once; a non-progress line is relayed/printed unchanged. +- **Vectors divergence (spike):** on the fixture, pre-walk total vs. actual `done` at + completion differ only by the ignored/empty count; the bar clamps to 100%. +- **Validation gate (AGENTS.md):** `.venv/bin/ruff check .` + `.venv/bin/python -m + pytest tests -v` (heavy e2e gated behind `JAVA_CODEBASE_RAG_RUN_HEAVY=1`, as + usual). Manual hello-world: `rm -rf /tmp/check && .venv/bin/python build_ast_graph.py + --source-root tests/bank-chat-system --ladybug-path /tmp/check/code_graph.lbug + --verbose` to confirm graph-phase progress lines render. + +## Open Questions ([TBD]) + +1. Tick cadence for the vectors phase — every file, every N files, or time-based? + — Recommended: **every N files (N≈25) + on completion**, to bound stderr volume + on huge trees without making the bar feel stale. +2. Graph pass-1 determinacy — count-first (filtered walk for an exact total, then + parse) or render indeterminate during the walk? — Recommended: **count-first** + (the graph's single-layer ignore makes the count exact and cheap). +3. Vectors total accuracy — accept the approximate pre-walk + clamp-on-completion, + or emit an authoritative `total` from inside the flow (a preflight running + `is_ignored` + content checks)? — Recommended: **approximate + clamp** for v1; + escalate to the authoritative count only if the spike's divergence test shows a + large gap. +4. Should `Optimize` get its own row, or collapse into the vectors tail + (`Vectors → optimizing…`)? On small repos optimize is sub-second and a third row + is noise. — Recommended: **own row, auto-collapse to a vectors sub-state when the + phase completes under ~1 s**. +5. Non-TTY concise-line cadence — time-based or count-based? — Recommended: + **time-based, every ~5 s + on phase completion** (matches CI log rhythm). +6. Should `install`/`update` gain a stdout JSON payload like the operator commands + for full scriptability parity? — Recommended: **no** for v1. They are interactive + wizards; their human-readable stdout is the point. Revisit if a real automation + ask surfaces. +7. `rich` version pin width? — Recommended: **`rich>=13.7,<14`** (stable, broadly + compatible Progress API). + +## Decisions taken + +1. **Determinate for both phases where the denominator is knowable.** The graph + phase is exactly determinate (count-first pass 1; six known passes). The vectors + phase is *approximately* determinate on full reprocess (pre-walk overstates by + the ignored/empty count; bar clamps to 100%) and indeterminate on catch-up. +2. **Renderer:** `rich` (not hand-rolled). Chosen for polish, thread-safe updates, + auto non-TTY fallback, and ~50–80 lines of glue vs. ~250–350 of fiddly ANSI. +3. **CocoIndex stays a subprocess.** In-process `app.update()`/`watch()` would give + first-class progress but re-introduces the shutdown native-thread crash the + subprocess isolation exists to avoid. Out of scope. +4. **stderr for the renderer; each command keeps its stdout contract.** Alignment is + the indexing UX, not the commands' stdout surface. +5. **`JCIRAG_PROGRESS` structured lines** as the cross-process channel. A + progress-file tail is *not* a committed fallback — if the gating spike shows + CocoIndex suppresses flow-function stderr, this proposal pauses for a transport + re-design rather than ship a racy file-tail with the same denominator problem. +6. **Three verbosity tiers preserved** (`--quiet` / default / `--verbose`). + +## Risks and mitigation + +| Risk | Mitigation | +|---|---| +| CocoIndex suppresses/buffers stderr written from inside `@coco.fn` flow functions → vectors ticks never reach the parent | **Throwaway spike branch first** (no PR): emit one `JCIRAG_PROGRESS` line from `process_java_file` and confirm the parent sees it. If it does not, **halt and re-propose the transport** — a progress-file tail is *not* a committed fallback (it re-introduces the denominator problem plus a writer/tailer race). Nothing else lands until the spike settles. | +| `rich` adds a dependency the repo has avoided | Pure-Python, parent-process only (the heavy native stack still loads in the CocoIndex *child* subprocess, unaffected); pulls only `pygments`. `rich.progress` is stable across 13.x. Acceptable for a CLI dev tool whose explicit goal is beautiful output. | +| `memo=True` makes the incremental denominator unknown → bar looks "stuck" at indeterminate | Intended behaviour, not a bug: indeterminate pulsing bar + "files touched: N" counter is honest. Documented in §Vectors phase. | +| Per-file `%` over-/under-reports because files have very different chunk counts (embedding cost) | The bar reports *files* done, not embedding cost — accurate as a file counter. ETA comes from rich's rolling rate, which absorbs chunk-count variance across many files. Acceptable approximation. | +| Two concurrent stderr writers (relay thread's raw `sys.stderr.buffer.write` + `rich` `Live`) interleave/corrupt the display | While the `Live` region is active, the relay thread routes every non-progress line through `console.print(...)` (rich reprints the live region cleanly) — never raw `buffer.write`. Raw relay runs only in `--verbose`, where there is no `Live` region. Existing `_LineFilter` noise matcher still drops `lance::`/`FutureWarning`/brownfield noise before `console.print`. Regression test: a `JCIRAG_PROGRESS` line split across two `read()` chunks parses once. | +| `Live` region torn down uncleanly on `SIGINT`/`KeyboardInterrupt` | rich handles `KeyboardInterrupt` in its `Live` context manager; the phase that was interrupted renders `✗`. Verify in the spike. | +| Missing `cocoindex`/builder binary → subprocess never spawns → phase hung at `running` with no ticks | Pre-spawn stub (`pipeline.py:168-174`) emits `status=failed`; a task is marked `running` only after the subprocess spawns, so the stub leaves no hung bar. Regression test covers 126/127. | +| Tiny repo: a phase finishes before its first tick → bar flashes pending→done | Harmless; the `status=done` line still renders. Spike covers a 3-file run to confirm no empty-bar flicker. | +| Tests over-assert on ANSI/rich formatting and become brittle | Assert on `JCIRAG_PROGRESS` contract + payload invariants, never on ANSI codes or exact column layout (per `tests/README.md`). | + +## Out of scope + +- Switching the vectors phase to in-process `cocoindex` `app.update()`/`watch()` + (stability risk; see principle 6). +- Splitting `init` or `increment` the way `reprocess` is split (no use case; see + `REPROCESS-SPLIT-PROPOSE` decisions). +- A determinate denominator for incremental catch-up (would require diffing against + CocoIndex's memo store — non-trivial, separate propose). +- Drift detection between Lance and LadybugDB stores (separate propose). +- Parallelising the two phases when both run (separate perf propose). +- Giving `install`/`update` a machine-readable stdout JSON payload (see Open Q4). +- Keeping the hand-rolled `Spinner` for any other caller (there are none — its only + caller is the vectors phase, so it is fully retired here). + +## Sequencing / Follow-ups + +Multi-PR; suggested split (a matching `plans/active/PLAN-INDEX-OUTPUT-REWORK.md` +follows once the propose is approved): + +- **Spike (throwaway branch, no PR):** confirm CocoIndex relays flow-function + stderr; size the pre-walk divergence. Gate. If it fails, halt and re-propose the + transport. +- **PR-1 (protocol + renderer skeleton):** only if the spike passes — land the + `JCIRAG_PROGRESS` parser + `rich` renderer + non-TTY fallback + the + parse/relay/single-writer invariants, with unit tests (split-chunk, + missing-binary). Ship the `rich` dep here. No command wiring yet. +- **PR-2 (graph phase):** count-first pass 1 + per-file `JCIRAG_PROGRESS`, passes + 2–6 per-pass; wire the renderer into the graph phase of all five commands. +- **PR-3 (vectors phase):** `JCIRAG_PROGRESS` from `process_*_file` + approximate + total pre-walk (clamp-on-completion); wire the renderer into the vectors phase + (incl. the serialized `Optimize` phase — both call sites). Retire + `emit_vectors_start`/`_finish`. +- **PR-4 (installer alignment + docs):** un-silence the subprocess calls in + `run_init_if_needed` / `run_update` (drop `quiet=True`, pass progress context); + wire `--quiet`/`--verbose` through `_cmd_update`/`run_update` and `--verbose` + through `install`; update `docs/JAVA-CODEBASE-RAG-CLI.md`. + +Each PR independently green (`ruff` + `pytest tests -v`). From 4f500d8bf020f95e2fa0dcb88707bb7b025a11ec Mon Sep 17 00:00:00 2001 From: Dmitry Teryaev Date: Sun, 14 Jun 2026 23:41:27 +0300 Subject: [PATCH 02/11] add rich + progress protocol/renderer skeleton rich>=14,<15 (cocoindex[lancedb] requires rich>=14). New java_codebase_rag/progress.py: ProgressEvent, parse_progress_line, IndexProgressRenderer (rich Live + non-TTY fallback), ProgressRelay (parse-first, single-writer). 13 light unit tests. No production caller yet; PR-2+ wire it in. Co-Authored-By: Claude --- java_codebase_rag/progress.py | 400 ++++++++++++++++++++++++++++++++++ pyproject.toml | 1 + tests/test_progress.py | 291 +++++++++++++++++++++++++ 3 files changed, 692 insertions(+) create mode 100644 java_codebase_rag/progress.py create mode 100644 tests/test_progress.py diff --git a/java_codebase_rag/progress.py b/java_codebase_rag/progress.py new file mode 100644 index 0000000..8a100cd --- /dev/null +++ b/java_codebase_rag/progress.py @@ -0,0 +1,400 @@ +"""JCIRAG_PROGRESS protocol: parse, render, and relay subprocess progress. + +The five lifecycle commands (``init`` / ``increment`` / ``install`` / +``reprocess`` / ``update``) drive two subprocesses — ``cocoindex update`` +(vectors → Lance) and ``build_ast_graph.py`` (graph → LadybugDB) — whose +stderr output is inconsistent and shows no real progress. This module is the +foundation of a unified progress surface. + +A subprocess prints lines of the form:: + + JCIRAG_PROGRESS kind=vectors phase=embed done=842 total=1240 + JCIRAG_PROGRESS kind=graph phase=build pass=3/6 done=120 total=600 status=running + JCIRAG_PROGRESS kind=optimize status=done elapsed_s=42.1 total=1240 + +to its **stderr**. The parent (``pipeline._LineFilter`` / +``cli_progress._AsyncLineFilter`` drain path) feeds each stderr chunk to a +:class:`ProgressRelay`, which: + +1. buffers chunks and splits on ``\\n`` (mirrors the existing line filters), +2. parses each complete line with :func:`parse_progress_line`, +3. if it is a progress line → forwards a :class:`ProgressEvent` to an + :class:`IndexProgressRenderer` and **suppresses** the raw line, else +4. runs the existing noise matcher (``is_noise_line``) and routes the + surviving line to the active sink (console / raw stderr / drop). + +On a TTY the renderer drives a ``rich.progress.Live`` region; off-TTY it +prints concise throttled lines (one per phase, ~every 5 s, plus a final +terminal line). This module is pure library — no production caller is wired +in yet. +""" +from __future__ import annotations + +import sys +import time +from dataclasses import dataclass +from typing import Literal + +from rich.console import Console +from rich.live import Live +from rich.progress import ( + BarColumn, + MofNCompleteColumn, + Progress, + SpinnerColumn, + TaskProgressColumn, + TextColumn, + TimeRemainingColumn, +) +from rich.text import Text + +from java_codebase_rag.cli_format import is_noise_line + +__all__ = [ + "ProgressEvent", + "parse_progress_line", + "IndexProgressRenderer", + "ProgressRelay", +] + +ProgressKind = Literal["vectors", "graph", "optimize"] +ProgressStatus = Literal["running", "done", "failed"] + +_PREFIX = "JCIRAG_PROGRESS" +# Non-TTY concise-line throttle window, in seconds, per phase. +_FALLBACK_THROTTLE_S = 5.0 + + +# --------------------------------------------------------------------------- +# Protocol +# --------------------------------------------------------------------------- + + +@dataclass +class ProgressEvent: + """A single parsed ``JCIRAG_PROGRESS`` line. + + ``pass_`` carries a multi-pass label like ``"3/6"`` (graph builder passes). + All optional fields are ``None`` when the emitting line omits the token. + """ + + kind: ProgressKind + phase: str | None + pass_: str | None + done: int | None + total: int | None + status: ProgressStatus + elapsed_s: float | None + + +def parse_progress_line(line: bytes) -> ProgressEvent | None: + """Parse one raw stderr line into a :class:`ProgressEvent`. + + Returns ``None`` (never raises) when the line: + + * does not start (after stripping a leading ``\\r``) with the exact + ``JCIRAG_PROGRESS`` prefix, or + * has the prefix but no usable tokens afterwards (malformed). + + The remainder after the prefix is whitespace-separated ``key=value`` + tokens. ``kind`` is required (omitting it → ``None``). ``status`` defaults + to ``"running"``. Double / extra spaces are tolerated. + """ + try: + text = line.decode("utf-8", errors="replace") + except Exception: + return None + # Tolerate a carriage-return rewind before the prefix. + stripped = text.lstrip("\r") + if not stripped.startswith(_PREFIX): + return None + tail = stripped[len(_PREFIX):] + # Tokenize on whitespace; empty/whitespace-only tails are malformed. + tokens = tail.split() + if not tokens: + return None + + fields: dict[str, str] = {} + for tok in tokens: + if "=" not in tok: + # A bare token is not a key=value pair → malformed line. + continue + key, _, value = tok.partition("=") + key = key.strip() + value = value.strip() + if key: + fields[key] = value + + if not fields: + return None + + kind_raw = fields.get("kind") + if kind_raw not in ("vectors", "graph", "optimize"): + # kind is required and must be one of the known kinds. + return None + + def _maybe_int(name: str) -> int | None: + raw = fields.get(name) + if raw is None or raw == "": + return None + try: + return int(raw) + except ValueError: + return None + + def _maybe_float(name: str) -> float | None: + raw = fields.get(name) + if raw is None or raw == "": + return None + try: + return float(raw) + except ValueError: + return None + + status_raw = fields.get("status", "running") + if status_raw not in ("running", "done", "failed"): + status_raw = "running" + + return ProgressEvent( + kind=kind_raw, # type: ignore[arg-type] + phase=fields.get("phase") or None, + pass_=fields.get("pass") or None, + done=_maybe_int("done"), + total=_maybe_int("total"), + status=status_raw, # type: ignore[arg-type] + elapsed_s=_maybe_float("elapsed_s"), + ) + + +# --------------------------------------------------------------------------- +# Renderer +# --------------------------------------------------------------------------- + + +def _build_progress(console: Console) -> Progress: + """Construct the rich.Progress with the canonical column layout. + + The label TextColumn reads ``task.fields["label"]`` so the phase name + shown in the bar is independent of the (mutable) description, which we + repurpose for status suffixes (``✗`` on failure). + """ + return Progress( + SpinnerColumn(), + TextColumn("[bold cyan]{task.fields[label]}"), + BarColumn(), + MofNCompleteColumn(), + TaskProgressColumn(), + TextColumn("{task.description}"), + TimeRemainingColumn(), + console=console, + transient=False, + ) + + +class IndexProgressRenderer: + """Owns the rich Progress region (TTY) or the concise-line fallback (non-TTY). + + Construction registers one task per phase name, each ``total=None`` and + **invisible** (the "never spawned" invariant: no task is ``running`` until + its first :meth:`apply` arrives). The first event for a kind flips that + kind's task visible + started. + + On a non-TTY console (``console.is_terminal is False``) no Live region is + used; instead concise lines are printed via :meth:`_fallback_print`, + throttled to once per ~5 s per phase, plus one terminal line per phase. + """ + + def __init__(self, phases: list[str], *, console: Console | None = None) -> None: + self._console: Console = console if console is not None else Console(stderr=True) + self._phases: list[str] = list(phases) + self._fallback: bool = not self._console.is_terminal + self._progress: Progress = _build_progress(self._console) + # kind -> rich task id. Start every task invisible + not started so the + # "never spawned" invariant holds until the first event arrives. + self._task_ids: dict[str, int] = {} + for phase in self._phases: + tid = self._progress.add_task( + phase, + total=None, + visible=False, + start=False, + label=phase, + ) + self._task_ids[phase] = tid + self._live: Live | None = None + # Non-TTY throttle bookkeeping (monotonic seconds of last concise print). + self._last_print_at: dict[str, float] = {phase: 0.0 for phase in self._phases} + self._started: bool = False + + # -- lifecycle ------------------------------------------------------- + + def start(self) -> None: + """Enter the Live region (TTY) or no-op (non-TTY). Idempotent.""" + if self._started: + return + self._started = True + if not self._fallback: + self._live = Live( + self._progress, + console=self._console, + refresh_per_second=10, + transient=False, + ) + self._live.start() + + def stop(self) -> None: + """Exit the Live region (TTY) or no-op (non-TTY). Safe to call once.""" + if not self._started: + return + self._started = False + if self._live is not None: + self._live.stop() + self._live = None + + # -- routing --------------------------------------------------------- + + def apply(self, ev: ProgressEvent) -> None: + """Route a single :class:`ProgressEvent` to its matching phase task. + + First event for a kind makes that task visible + started. ``total`` and + ``done`` are applied directly when present. ``status == "done"`` clamps + completed to the total (an approximate total can't stall below 100%). + ``status == "failed"`` halts the task and marks the description with a + red ``✗`` (rich renders the spinner stopped). On non-TTY consoles this + delegates to the throttled concise-line printer. + """ + if self._fallback: + self._fallback_apply(ev) + return + tid = self._task_ids.get(ev.kind) + if tid is None: + return + task = self._progress.tasks[tid] + # First event: promote from pending to running/visible. + if not task.started: + self._progress.start_task(tid) + self._progress.update(tid, visible=True) + if ev.total is not None: + self._progress.update(tid, total=ev.total) + if ev.done is not None: + # Set-based (not advance-based) for determinism: each event carries + # the absolute completed count, not a delta. + self._progress.update(tid, completed=ev.done) + if ev.status == "done": + # Clamp completed to total so an approximate total can't stall. + if task.total is not None and task.completed < task.total: + self._progress.update(tid, completed=task.total) + self._progress.update(tid, description=f"{ev.kind} ✓") + self._progress.stop_task(tid) + elif ev.status == "failed": + self._progress.update(tid, description=f"{ev.kind} ✗") + self._progress.stop_task(tid) + + # -- non-TTY fallback ------------------------------------------------ + + def _now(self) -> float: + """Indirection for tests; returns the monotonic clock in seconds.""" + return time.monotonic() + + def _fallback_apply(self, ev: ProgressEvent) -> None: + """Concise-line path for non-TTY consoles.""" + last = self._last_print_at.get(ev.kind, 0.0) + now = self._now() + terminal = ev.status in ("done", "failed") + throttle_ok = (now - last) >= _FALLBACK_THROTTLE_S + if not terminal and not throttle_ok and last != 0.0: + # Suppressed by the throttle window. (``last != 0.0`` lets the very + # first event for a phase print immediately.) + return + self._last_print_at[ev.kind] = now + self._console.print(self._format_concise(ev)) + + def _format_concise(self, ev: ProgressEvent) -> Text: + """Render one concise line for the non-TTY fallback.""" + kind = ev.kind + if ev.status == "done": + total = ev.total if ev.total is not None else "" + elapsed = f"{ev.elapsed_s:.1f}s" if ev.elapsed_s is not None else "" + bits = [kind, "done"] + if total != "": + bits.append(str(total)) + if elapsed: + bits.append(elapsed) + return Text(" · ".join(bits)) + if ev.status == "failed": + return Text(f"{kind} failed") + if ev.done is not None and ev.total is not None and ev.total > 0: + pct = int(round(ev.done * 100.0 / ev.total)) + return Text(f"{kind} {ev.done}/{ev.total} ({pct}%)") + if ev.done is not None: + return Text(f"{kind} {ev.done}") + return Text(kind) + + +# --------------------------------------------------------------------------- +# Relay +# --------------------------------------------------------------------------- + + +class ProgressRelay: + """Single-writer bridge between a subprocess stderr drain and the renderer. + + Mirrors the byte-buffering of ``cli_progress._AsyncLineFilter`` / + ``pipeline._LineFilter``: accumulate chunks, split on ``\\n``, route each + complete line. When ``verbose`` is True (and no renderer is attached) the + relay writes raw bytes to ``sys.stderr.buffer`` (raw mode, no Live region). + """ + + def __init__( + self, + renderer: IndexProgressRenderer | None, + *, + console: Console | None = None, + verbose: bool = False, + ) -> None: + self._renderer = renderer + self._verbose = verbose + self._console: Console | None = console + self._buf = bytearray() + # Live region is only meaningful when a renderer is attached. + self._live_active: bool = renderer is not None + + def feed(self, chunk: bytes) -> None: + """Buffer ``chunk`` and route each complete (``\\n``-terminated) line.""" + self._buf.extend(chunk) + while b"\n" in self._buf: + line, self._buf = self._buf.split(b"\n", 1) + line += b"\n" + self._route_line(line) + + def flush(self) -> None: + """Emit any trailing partial buffer (without a trailing newline).""" + if self._buf: + self._route_line(bytes(self._buf), _is_partial=True) + self._buf.clear() + + def _route_line(self, line: bytes, *, _is_partial: bool = False) -> None: + ev = parse_progress_line(line) + if ev is not None and self._renderer is not None: + # Consumed by the protocol — never echoed to any sink. + self._renderer.apply(ev) + return + # Non-progress line: drop noise, then route to the active sink. + if is_noise_line(line): + return + text = line.decode("utf-8", errors="replace") + if self._renderer is not None and self._live_active: + console = self._console if self._console is not None else self._renderer._console # noqa: SLF001 + # rich.Console over a Live region must suspend/resume to interleave + # a one-off line without corrupting the bar redraw; print() handles + # this correctly when the Live was started on the same console. + console.print(text, end="") + return + if self._verbose and self._renderer is None: + try: + sys.stderr.buffer.write(line) + sys.stderr.buffer.flush() + except Exception: + pass + return + # Neither verbose nor a renderer: drop quietly (quiet mode). diff --git a/pyproject.toml b/pyproject.toml index 4f972d0..b756023 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -33,6 +33,7 @@ dependencies = [ "pydantic>=2.0,<3", "PyYAML>=6.0.3,<7", "questionary>=2.0,<3", + "rich>=14,<15", "sentence-transformers>=5.4.0,<6", "tree-sitter>=0.25.2,<0.26", "tree-sitter-java>=0.23.5,<0.24", diff --git a/tests/test_progress.py b/tests/test_progress.py new file mode 100644 index 0000000..ca8d75d --- /dev/null +++ b/tests/test_progress.py @@ -0,0 +1,291 @@ +"""Unit tests for java_codebase_rag.progress (JCIRAG_PROGRESS protocol). + +All tests are LIGHT: no subprocess, no cocoindex, no torch. They exercise the +parser, the renderer (against a non-TTY Console over io.StringIO), the +non-TTY concise-line fallback, and the relay's byte-buffering line split. +""" +from __future__ import annotations + +import io +from dataclasses import dataclass +from typing import Literal + +from rich.console import Console + +from java_codebase_rag.progress import ( + IndexProgressRenderer, + ProgressEvent, + ProgressRelay, + parse_progress_line, +) + +_PREFIX = "JCIRAG_PROGRESS" + + +# --------------------------------------------------------------------------- +# parse_progress_line +# --------------------------------------------------------------------------- + + +def test_parse_progress_line_vectors_running() -> None: + line = f"{_PREFIX} kind=vectors phase=embed done=842 total=1240\n".encode() + ev = parse_progress_line(line) + assert ev is not None + assert ev.kind == "vectors" + assert ev.phase == "embed" + assert ev.pass_ is None + assert ev.done == 842 + assert ev.total == 1240 + assert ev.status == "running" # default + assert ev.elapsed_s is None + + +def test_parse_progress_line_graph_pass() -> None: + line = f"{_PREFIX} kind=graph phase=build pass=3/6 done=120 total=600\n".encode() + ev = parse_progress_line(line) + assert ev is not None + assert ev.kind == "graph" + assert ev.phase == "build" + assert ev.pass_ == "3/6" + assert ev.done == 120 + assert ev.total == 600 + + +def test_parse_progress_line_optimize_running() -> None: + line = f"{_PREFIX} kind=optimize phase=compact done=3 total=12\n".encode() + ev = parse_progress_line(line) + assert ev is not None + assert ev.kind == "optimize" + assert ev.phase == "compact" + assert ev.done == 3 + assert ev.total == 12 + + +def test_parse_progress_line_done_with_elapsed() -> None: + line = f"{_PREFIX} kind=vectors status=done elapsed_s=42.1 total=1240\n".encode() + ev = parse_progress_line(line) + assert ev is not None + assert ev.status == "done" + assert ev.elapsed_s == 42.1 + assert ev.total == 1240 + + +def test_parse_progress_line_non_progress_returns_none() -> None: + # A cocoindex/lance line that is NOT progress: must return None. + assert parse_progress_line(b"lance:: reading fragment\n") is None + assert parse_progress_line(b"some random stderr noise\n") is None + + +def test_parse_progress_line_malformed_returns_none() -> None: + # Prefix present but nothing usable after it. + assert parse_progress_line(f"{_PREFIX}\n".encode()) is None + assert parse_progress_line(f"{_PREFIX} \n".encode()) is None + # Must never raise, even on garbage. + assert parse_progress_line(b"") is None + assert parse_progress_line(f"{_PREFIX} = = =\n".encode()) is None + + +# --------------------------------------------------------------------------- +# ProgressRelay +# --------------------------------------------------------------------------- + + +@dataclass +class _ApplyRecord: + kind: str + phase: str | None + pass_: str | None + done: int | None + total: int | None + status: Literal["running", "done", "failed"] + elapsed_s: float | None + + +class _StubRenderer: + """Captures apply() calls; never prints. Used to test the relay in isolation.""" + + def __init__(self) -> None: + self.applied: list[ProgressEvent] = [] + + def apply(self, ev: ProgressEvent) -> None: + self.applied.append(ev) + + def start(self) -> None: + pass + + def stop(self) -> None: + pass + + +def test_progress_relay_parses_split_chunk_once() -> None: + """One logical progress line fed as two feed() calls across a chunk boundary + results in exactly ONE renderer.apply() call, and the raw line is NOT printed.""" + stub = _StubRenderer() + buf = io.BytesIO() + relay = ProgressRelay(renderer=stub, console=Console(file=buf, force_terminal=False)) + full = f"{_PREFIX} kind=vectors done=5 total=10\n".encode() + mid = len(full) // 2 + relay.feed(full[:mid]) + relay.feed(full[mid:]) + assert len(stub.applied) == 1 + ev = stub.applied[0] + assert ev.kind == "vectors" + assert ev.done == 5 + assert ev.total == 10 + # The progress line must be consumed/suppressed, not echoed to the console. + assert buf.getvalue() == b"" + + +def test_progress_relay_relays_non_progress_line() -> None: + """A non-progress, non-noise line reaches the output sink (console).""" + stub = _StubRenderer() + buf = io.StringIO() + relay = ProgressRelay( + renderer=stub, console=Console(file=buf, force_terminal=False, force_interactive=False) + ) + relay.feed(b"cocoindex: importing flow\n") + out = buf.getvalue() + assert "cocoindex: importing flow" in out + assert len(stub.applied) == 0 + + +# --------------------------------------------------------------------------- +# IndexProgressRenderer — TTY path (against a forced-terminal Console) +# --------------------------------------------------------------------------- + + +def test_renderer_task_pending_until_first_event() -> None: + buf = io.StringIO() + # Force a terminal so the Live path is taken (not the concise fallback). + console = Console(file=buf, force_terminal=True, width=120, color_system=None) + r = IndexProgressRenderer(phases=["vectors", "graph", "optimize"], console=console) + r.start() + try: + # No event yet: every task must be invisible / not started (pending). + for task in r._progress.tasks: + assert not task.visible + assert not task.started + finally: + r.stop() + + +def test_renderer_clamps_completed_to_total_on_done() -> None: + buf = io.StringIO() + console = Console(file=buf, force_terminal=True, width=120, color_system=None) + r = IndexProgressRenderer(phases=["vectors"], console=console) + r.start() + try: + r.apply( + ProgressEvent( + kind="vectors", phase=None, pass_=None, done=80, total=100, status="running", elapsed_s=None + ) + ) + # While running, completed tracks the event's done exactly. + assert r._progress.tasks[r._task_ids["vectors"]].completed == 80 + # Terminal event with status=done and no new done value: clamp to total. + r.apply( + ProgressEvent( + kind="vectors", phase=None, pass_=None, done=None, total=100, status="done", elapsed_s=1.0 + ) + ) + assert r._progress.tasks[r._task_ids["vectors"]].completed == 100 + assert r._progress.tasks[r._task_ids["vectors"]].finished + finally: + r.stop() + + +def test_renderer_indeterminate_total_none() -> None: + buf = io.StringIO() + console = Console(file=buf, force_terminal=True, width=120, color_system=None) + r = IndexProgressRenderer(phases=["graph"], console=console) + r.start() + try: + # Events with no total → task stays indeterminate (total is None). + r.apply( + ProgressEvent( + kind="graph", phase="build", pass_=None, done=5, total=None, status="running", elapsed_s=None + ) + ) + task = r._progress.tasks[r._task_ids["graph"]] + assert task.total is None + finally: + r.stop() + + +def test_renderer_failed_marks_task_red() -> None: + buf = io.StringIO() + console = Console(file=buf, force_terminal=True, width=120, color_system=None) + r = IndexProgressRenderer(phases=["vectors"], console=console) + r.start() + try: + r.apply( + ProgressEvent( + kind="vectors", phase=None, pass_=None, done=2, total=10, status="running", elapsed_s=None + ) + ) + r.apply( + ProgressEvent( + kind="vectors", phase=None, pass_=None, done=2, total=10, status="failed", elapsed_s=None + ) + ) + task = r._progress.tasks[r._task_ids["vectors"]] + # The task is stopped (spinner halted) — rich records a stop_time — and the + # description carries the red ✗ marker. ``started`` stays True (start is + # irreversible); ``stop_time`` is the authoritative "halted" signal. + assert task.stop_time is not None + assert "✗" in task.description + finally: + r.stop() + + +# --------------------------------------------------------------------------- +# IndexProgressRenderer — non-TTY concise-line fallback +# --------------------------------------------------------------------------- + + +def test_non_tty_fallback_emits_concise_lines() -> None: + """Non-TTY: concise lines appear, throttled to ~5s/phase, plus a terminal line.""" + buf = io.StringIO() + console = Console(file=buf, force_terminal=False) + r = IndexProgressRenderer(phases=["vectors"], console=console) + assert r._fallback is True + r.start() + t0 = r._now() + try: + # First event: a concise progress line appears immediately. + r.apply( + ProgressEvent( + kind="vectors", phase=None, pass_=None, done=842, total=1240, status="running", elapsed_s=None + ) + ) + first = buf.getvalue() + assert "vectors" in first + assert "842" in first and "1240" in first + # A second running event within the throttle window is suppressed. + n_before = len(buf.getvalue()) + r.apply( + ProgressEvent( + kind="vectors", phase=None, pass_=None, done=900, total=1240, status="running", elapsed_s=None + ) + ) + # Throttle: no new line within the window (monotonic clock hasn't advanced). + assert len(buf.getvalue()) == n_before + # Push the throttle window past ~5s: next running event prints again. + r._last_print_at["vectors"] = t0 - 10.0 + r.apply( + ProgressEvent( + kind="vectors", phase=None, pass_=None, done=1000, total=1240, status="running", elapsed_s=None + ) + ) + assert "1000" in buf.getvalue() + # Terminal event: always prints (done line), regardless of throttle. + r.apply( + ProgressEvent( + kind="vectors", phase=None, pass_=None, done=1240, total=1240, status="done", elapsed_s=42.1 + ) + ) + final = buf.getvalue() + assert "done" in final + assert "42.1" in final + finally: + r.stop() From 34af98ed016d0176c3c755c7577af025f659d3b5 Mon Sep 17 00:00:00 2001 From: Dmitry Teryaev Date: Sun, 14 Jun 2026 23:42:36 +0300 Subject: [PATCH 03/11] correct rich version pin to >=14,<15 cocoindex[lancedb]>=1.0.0a43 transitively requires rich>=14, so the originally-suggested rich>=13.7,<14 cap is unsatisfiable (pip install -e . fails). Corrected in propose Open Q5 + risk row, plan Resolved decisions + PR-1 step + DoD, and the PR-1 agent prompt. Verified compatible on rich 14.3.4. Co-Authored-By: Claude --- plans/active/AGENT-PROMPTS-INDEX-OUTPUT-REWORK.md | 2 +- plans/active/PLAN-INDEX-OUTPUT-REWORK.md | 6 +++--- propose/active/INDEX-OUTPUT-REWORK-PROPOSE.md | 8 +++++--- 3 files changed, 9 insertions(+), 7 deletions(-) diff --git a/plans/active/AGENT-PROMPTS-INDEX-OUTPUT-REWORK.md b/plans/active/AGENT-PROMPTS-INDEX-OUTPUT-REWORK.md index 4774d83..343282a 100644 --- a/plans/active/AGENT-PROMPTS-INDEX-OUTPUT-REWORK.md +++ b/plans/active/AGENT-PROMPTS-INDEX-OUTPUT-REWORK.md @@ -93,7 +93,7 @@ You are implementing PR-1 from `plans/active/PLAN-INDEX-OUTPUT-REWORK.md`. Read over this prompt. ## Scope -1. Add `rich>=13.7,<14` to `pyproject.toml` `dependencies`. +1. Add `rich>=14,<15` to `pyproject.toml` `dependencies` (`cocoindex[lancedb]>=1.0.0a43` requires `rich>=14`; a `<14` cap is unsatisfiable). 2. Create `java_codebase_rag/progress.py` with exactly these symbols: - `ProgressEvent` dataclass (`kind`, `phase`, `pass_`, `done`, `total`, `status`, `elapsed_s`). diff --git a/plans/active/PLAN-INDEX-OUTPUT-REWORK.md b/plans/active/PLAN-INDEX-OUTPUT-REWORK.md index 9e3ffd3..d9c87f8 100644 --- a/plans/active/PLAN-INDEX-OUTPUT-REWORK.md +++ b/plans/active/PLAN-INDEX-OUTPUT-REWORK.md @@ -64,7 +64,7 @@ the spike passes; do not start PR-N+1 until PR-N is merged to `master`. | Topic | Decision | | --- | --- | -| Renderer library | `rich` (`rich>=13.7,<14`); parent-process only. | +| Renderer library | `rich` (`rich>=14,<15` — `cocoindex[lancedb]>=1.0.0a43` requires `rich>=14`, so a `<14` cap is unsatisfiable); parent-process only. | | New module | `java_codebase_rag/progress.py` holds `ProgressEvent`, `parse_progress_line`, `IndexProgressRenderer`, `ProgressRelay`. | | Protocol line | `JCIRAG_PROGRESS kind= [phase=…] [pass=N/6] [done=N] [total=N] [status=running\|done\|failed] [elapsed_s=…]` | | Drain integration | `pipeline._popen_capturing_stderr` and `cli_progress.accumulate_and_relay_subprocess_streams` gain an optional `on_progress` callback and route complete lines through a `ProgressRelay` (parse-first; progress lines suppressed from relay). | @@ -133,7 +133,7 @@ No command wiring, no flow/builder emission in this PR — pure library + unit t ## File-by-file changes ### 1. `pyproject.toml` -- Add `rich>=13.7,<14` to `dependencies`. +- Add `rich>=14,<15` to `dependencies` (`cocoindex[lancedb]>=1.0.0a43` requires `rich>=14`; a `<14` cap is unsatisfiable). ### 2. `java_codebase_rag/progress.py` (new) - `ProgressEvent` dataclass: `kind` (`Literal["vectors","graph","optimize"]`), @@ -189,7 +189,7 @@ No command wiring, no flow/builder emission in this PR — pure library + unit t ## Definition of done (PR-1) -- [ ] `rich>=13.7,<14` in `pyproject.toml`; `.venv/bin/pip install -e .` succeeds. +- [ ] `rich>=14,<15` in `pyproject.toml`; `.venv/bin/pip install -e .` succeeds. - [ ] `progress.py` exists with the four symbols above; no production caller yet. - [ ] All 13 `tests/test_progress.py` tests pass. - [ ] `.venv/bin/ruff check .` clean; `.venv/bin/python -m pytest tests -v` green diff --git a/propose/active/INDEX-OUTPUT-REWORK-PROPOSE.md b/propose/active/INDEX-OUTPUT-REWORK-PROPOSE.md index 43466af..2d60ef9 100644 --- a/propose/active/INDEX-OUTPUT-REWORK-PROPOSE.md +++ b/propose/active/INDEX-OUTPUT-REWORK-PROPOSE.md @@ -377,8 +377,10 @@ the `tests/bank-chat-system/` fixture. for full scriptability parity? — Recommended: **no** for v1. They are interactive wizards; their human-readable stdout is the point. Revisit if a real automation ask surfaces. -7. `rich` version pin width? — Recommended: **`rich>=13.7,<14`** (stable, broadly - compatible Progress API). +7. `rich` version pin width? — Recommended: **`rich>=14,<15`** + (`cocoindex[lancedb]>=1.0.0a43` transitively requires `rich>=14`, so the + `>=13.7,<14` cap originally suggested is unsatisfiable; verified compatible with + the renderer's `Progress` API usage on rich 14.3.4). ## Decisions taken @@ -404,7 +406,7 @@ the `tests/bank-chat-system/` fixture. | Risk | Mitigation | |---|---| | CocoIndex suppresses/buffers stderr written from inside `@coco.fn` flow functions → vectors ticks never reach the parent | **Throwaway spike branch first** (no PR): emit one `JCIRAG_PROGRESS` line from `process_java_file` and confirm the parent sees it. If it does not, **halt and re-propose the transport** — a progress-file tail is *not* a committed fallback (it re-introduces the denominator problem plus a writer/tailer race). Nothing else lands until the spike settles. | -| `rich` adds a dependency the repo has avoided | Pure-Python, parent-process only (the heavy native stack still loads in the CocoIndex *child* subprocess, unaffected); pulls only `pygments`. `rich.progress` is stable across 13.x. Acceptable for a CLI dev tool whose explicit goal is beautiful output. | +| `rich` adds a dependency the repo has avoided | Pure-Python, parent-process only (the heavy native stack still loads in the CocoIndex *child* subprocess, unaffected); pulls only `pygments`. `rich.progress` is stable across 14.x. Pinned `>=14,<15` (forced by `cocoindex[lancedb]`'s `rich>=14` requirement). Acceptable for a CLI dev tool whose explicit goal is beautiful output. | | `memo=True` makes the incremental denominator unknown → bar looks "stuck" at indeterminate | Intended behaviour, not a bug: indeterminate pulsing bar + "files touched: N" counter is honest. Documented in §Vectors phase. | | Per-file `%` over-/under-reports because files have very different chunk counts (embedding cost) | The bar reports *files* done, not embedding cost — accurate as a file counter. ETA comes from rich's rolling rate, which absorbs chunk-count variance across many files. Acceptable approximation. | | Two concurrent stderr writers (relay thread's raw `sys.stderr.buffer.write` + `rich` `Live`) interleave/corrupt the display | While the `Live` region is active, the relay thread routes every non-progress line through `console.print(...)` (rich reprints the live region cleanly) — never raw `buffer.write`. Raw relay runs only in `--verbose`, where there is no `Live` region. Existing `_LineFilter` noise matcher still drops `lance::`/`FutureWarning`/brownfield noise before `console.print`. Regression test: a `JCIRAG_PROGRESS` line split across two `read()` chunks parses once. | From b67eb436666e8f7190d3baee8d891ff38278cd80 Mon Sep 17 00:00:00 2001 From: Dmitry Teryaev Date: Mon, 15 Jun 2026 00:14:20 +0300 Subject: [PATCH 04/11] address PR-1 review: port _suppress_next, two-way clamp, monotonic done, carry-forward totals MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - ProgressRelay now mirrors _LineFilter's noise-continuation suppression (_suppress_next) + new test_progress_relay_suppresses_noise_continuation - remove dead _is_partial param from _route_line - clamp completed→total on status=done both directions (over-count safe) - monotonic completed (max with current) so a stray smaller done can't rewind - non-TTY done concise line carries forward last-seen total/elapsed_s Minor #6 (apply/stop atomicity) deferred to PR-2 (drain-thread wiring). Co-Authored-By: Claude --- java_codebase_rag/progress.py | 102 ++++++++++++++++++++++++++++------ tests/test_progress.py | 22 ++++++++ 2 files changed, 106 insertions(+), 18 deletions(-) diff --git a/java_codebase_rag/progress.py b/java_codebase_rag/progress.py index 8a100cd..7527836 100644 --- a/java_codebase_rag/progress.py +++ b/java_codebase_rag/progress.py @@ -224,6 +224,11 @@ def __init__(self, phases: list[str], *, console: Console | None = None) -> None self._live: Live | None = None # Non-TTY throttle bookkeeping (monotonic seconds of last concise print). self._last_print_at: dict[str, float] = {phase: 0.0 for phase in self._phases} + # Non-TTY carry-forward: a minimal ``done`` event may omit total / + # elapsed_s; fall back to the last-seen values so the concise terminal + # line never degrades (e.g. stays ``vectors done · 1240 · 42.1s``). + self._last_total: dict[str, int | None] = {phase: None for phase in self._phases} + self._last_elapsed: dict[str, float | None] = {phase: None for phase in self._phases} self._started: bool = False # -- lifecycle ------------------------------------------------------- @@ -257,8 +262,12 @@ def apply(self, ev: ProgressEvent) -> None: """Route a single :class:`ProgressEvent` to its matching phase task. First event for a kind makes that task visible + started. ``total`` and - ``done`` are applied directly when present. ``status == "done"`` clamps - completed to the total (an approximate total can't stall below 100%). + ``done`` are applied directly when present. ``done`` is clamped to be + non-decreasing (producers should emit monotonically non-decreasing + ``done``; this defensive clamp enforces it so a stray smaller value + can't rewind the bar). ``status == "done"`` unconditionally clamps + completed to the total in both directions (an approximate total can't + stall below 100%, nor can an approximate pre-walk over-count exceed it). ``status == "failed"`` halts the task and marks the description with a red ``✗`` (rich renders the spinner stopped). On non-TTY consoles this delegates to the throttled concise-line printer. @@ -278,11 +287,15 @@ def apply(self, ev: ProgressEvent) -> None: self._progress.update(tid, total=ev.total) if ev.done is not None: # Set-based (not advance-based) for determinism: each event carries - # the absolute completed count, not a delta. - self._progress.update(tid, completed=ev.done) + # the absolute completed count, not a delta. Monotonic clamp: never + # let a smaller done rewind the bar. + new_completed = max(task.completed, ev.done) + self._progress.update(tid, completed=new_completed) if ev.status == "done": - # Clamp completed to total so an approximate total can't stall. - if task.total is not None and task.completed < task.total: + # Two-way clamp: completed must equal total on done. An approximate + # total can under-count (stall below 100%) or the propose's + # approximate pre-walk can over-count; both resolve to == total. + if task.total is not None and task.completed != task.total: self._progress.update(tid, completed=task.total) self._progress.update(tid, description=f"{ev.kind} ✓") self._progress.stop_task(tid) @@ -304,20 +317,51 @@ def _fallback_apply(self, ev: ProgressEvent) -> None: throttle_ok = (now - last) >= _FALLBACK_THROTTLE_S if not terminal and not throttle_ok and last != 0.0: # Suppressed by the throttle window. (``last != 0.0`` lets the very - # first event for a phase print immediately.) + # first event for a phase print immediately.) Still track totals so + # a later terminal line can carry them forward. + if ev.total is not None: + self._last_total[ev.kind] = ev.total + if ev.elapsed_s is not None: + self._last_elapsed[ev.kind] = ev.elapsed_s return + # Carry forward last-seen total / elapsed_s so a minimal ``done`` event + # that omits them still prints a complete terminal line. + carried_total = ev.total + if carried_total is None: + carried_total = self._last_total.get(ev.kind) + carried_elapsed = ev.elapsed_s + if carried_elapsed is None: + carried_elapsed = self._last_elapsed.get(ev.kind) + # Update the carry-forward state with whatever this event carried. + if ev.total is not None: + self._last_total[ev.kind] = ev.total + if ev.elapsed_s is not None: + self._last_elapsed[ev.kind] = ev.elapsed_s self._last_print_at[ev.kind] = now - self._console.print(self._format_concise(ev)) + self._console.print( + self._format_concise(ev, total=carried_total, elapsed_s=carried_elapsed) + ) - def _format_concise(self, ev: ProgressEvent) -> Text: - """Render one concise line for the non-TTY fallback.""" + def _format_concise( + self, + ev: ProgressEvent, + *, + total: int | None = None, + elapsed_s: float | None = None, + ) -> Text: + """Render one concise line for the non-TTY fallback. + + ``total`` / ``elapsed_s`` are the already-carry-forwarded values to show + (the caller resolves event-vs-last-seen before formatting); ``ev``'s own + fields are only used for status / done / phase. + """ kind = ev.kind if ev.status == "done": - total = ev.total if ev.total is not None else "" - elapsed = f"{ev.elapsed_s:.1f}s" if ev.elapsed_s is not None else "" + total_str = str(total) if total is not None else "" + elapsed = f"{elapsed_s:.1f}s" if elapsed_s is not None else "" bits = [kind, "done"] - if total != "": - bits.append(str(total)) + if total_str != "": + bits.append(total_str) if elapsed: bits.append(elapsed) return Text(" · ".join(bits)) @@ -358,6 +402,12 @@ def __init__( self._buf = bytearray() # Live region is only meaningful when a renderer is attached. self._live_active: bool = renderer is not None + # Mirrors ``_LineFilter._suppress_next``: a noise header line (e.g. a + # ``FutureWarning:`` banner) suppresses the NEXT line too, which is its + # indented traceback frame(s). ``line[:1] in (b" ", b"\t")`` is the + # continuation signal. Progress lines reset this (they are consumed by + # the renderer, never noise). + self._suppress_next: bool = False def feed(self, chunk: bytes) -> None: """Buffer ``chunk`` and route each complete (``\\n``-terminated) line.""" @@ -370,18 +420,34 @@ def feed(self, chunk: bytes) -> None: def flush(self) -> None: """Emit any trailing partial buffer (without a trailing newline).""" if self._buf: - self._route_line(bytes(self._buf), _is_partial=True) + # Trailing partial line: continuation suppression does not apply + # (there is no following line). Route it directly. + self._route_line(bytes(self._buf)) self._buf.clear() - def _route_line(self, line: bytes, *, _is_partial: bool = False) -> None: + def _route_line(self, line: bytes) -> None: ev = parse_progress_line(line) if ev is not None and self._renderer is not None: - # Consumed by the protocol — never echoed to any sink. + # Consumed by the protocol — never echoed to any sink. It is not + # noise, so it must not keep the suppression flag armed. + self._suppress_next = False self._renderer.apply(ev) return - # Non-progress line: drop noise, then route to the active sink. + if ev is not None and self._renderer is None: + # Parsed as progress but no renderer attached: still reset the flag + # (a progress line is never noise) and drop quietly. + self._suppress_next = False + return + # Non-progress line: noise path, with continuation suppression. if is_noise_line(line): + self._suppress_next = True + return + if self._suppress_next and line[:1] in (b" ", b"\t"): + # Indented continuation of the preceding noise header (e.g. a + # traceback frame). Drop without disarming: a multi-frame traceback + # has several such lines in a row. return + self._suppress_next = False text = line.decode("utf-8", errors="replace") if self._renderer is not None and self._live_active: console = self._console if self._console is not None else self._renderer._console # noqa: SLF001 diff --git a/tests/test_progress.py b/tests/test_progress.py index ca8d75d..ba25e83 100644 --- a/tests/test_progress.py +++ b/tests/test_progress.py @@ -149,6 +149,28 @@ def test_progress_relay_relays_non_progress_line() -> None: assert len(stub.applied) == 0 +def test_progress_relay_suppresses_noise_continuation() -> None: + """A noise header (``FutureWarning:``) plus its indented traceback frame are + BOTH suppressed; a following normal line is still emitted. Mirrors + ``_LineFilter``/``_AsyncLineFilter``'s ``_suppress_next`` behavior.""" + stub = _StubRenderer() + buf = io.StringIO() + relay = ProgressRelay( + renderer=stub, console=Console(file=buf, force_terminal=False, force_interactive=False) + ) + relay.feed(b"/some/conda/env.py:1: FutureWarning: something deprecated\n") + relay.feed(b" some/frame.py:42: DeprecationWarning\n") + relay.feed(b"cocoindex: indexing batch\n") + out = buf.getvalue() + # The noise header and its indented continuation must NOT reach the sink. + assert "FutureWarning" not in out + assert "some/frame.py:42" not in out + # The normal line that follows must still be emitted. + assert "cocoindex: indexing batch" in out + # No progress events were parsed. + assert len(stub.applied) == 0 + + # --------------------------------------------------------------------------- # IndexProgressRenderer — TTY path (against a forced-terminal Console) # --------------------------------------------------------------------------- From 0ef2bccc595d303a59969c6575119216453bf7be Mon Sep 17 00:00:00 2001 From: Dmitry Teryaev Date: Mon, 15 Jun 2026 00:18:59 +0300 Subject: [PATCH 05/11] note PR-2 follow-up: IndexProgressRenderer apply/stop atomicity PR-1 code-quality review Minor #6: apply() issues several locked rich calls not atomic vs. a concurrent stop() from the main thread. Only matters once PR-2 wires the drain thread. Added as cross-PR risk #10 with the PR-2 mitigation (gate apply on _started, or hold the Live lock across the sequence). Co-Authored-By: Claude --- plans/active/PLAN-INDEX-OUTPUT-REWORK.md | 1 + 1 file changed, 1 insertion(+) diff --git a/plans/active/PLAN-INDEX-OUTPUT-REWORK.md b/plans/active/PLAN-INDEX-OUTPUT-REWORK.md index d9c87f8..7c48b0d 100644 --- a/plans/active/PLAN-INDEX-OUTPUT-REWORK.md +++ b/plans/active/PLAN-INDEX-OUTPUT-REWORK.md @@ -489,6 +489,7 @@ run in the default light suite.) | 7 | PR-2/PR-3/PR-4 all touch the operator-command renderer context → merge conflicts | Medium | Strict landing order (PR-2 → PR-3 → PR-4); each PR rebase-merges the previous. | | 8 | `update` verbosity wiring changes observable behaviour for existing scripts | Low | `--quiet`/`--verbose` already existed on the parser (ignored); wiring them through matches siblings, no new flags. | | 9 | Heavy (cocoindex/torch) tests added to the default suite slow CI / break the segfault-isolation work | Medium | All cocoindex-flow tests gated behind `JAVA_CODEBASE_RAG_RUN_HEAVY=1`; parser/renderer/CLI tests use patched helpers and stay light. | +| 10 | `IndexProgressRenderer.apply()` issues several locked rich calls (start→update→…→stop) that are not atomic vs. a concurrent `stop()` from the main thread — a momentary display flicker on Live teardown once the drain thread feeds it (PR-1 review Minor #6) | Low | Address in PR-2 when the drain thread is wired: gate `apply()` on `self._started` (no-op after `stop()`), or hold the Live lock across the sequence. Stress-test concurrent apply/stop. | # Out of scope From 36802d2a624f9ea4578de2d3657340eb01604e0c Mon Sep 17 00:00:00 2001 From: Dmitry Teryaev Date: Mon, 15 Jun 2026 01:11:53 +0300 Subject: [PATCH 06/11] wire graph-phase index progress (count-first pass1 + pass steps) build_ast_graph emits JCIRAG_PROGRESS kind=graph under --verbose (count-first exact total in pass 1; pass 2-6 step lines). The sync and async subprocess drains route progress events to an on_progress callback via ProgressRelay (parse-first, single-writer). init/increment/ reprocess render a determinate graph-phase bar in default TTY mode (running only after the builder spawns); --quiet silent; --verbose raw. Renderer.apply() is a no-op after stop() (drain-thread safety). Co-Authored-By: Claude --- build_ast_graph.py | 80 +++++++++++- java_codebase_rag/cli.py | 79 ++++++++++-- java_codebase_rag/cli_format.py | 8 ++ java_codebase_rag/cli_progress.py | 22 +++- java_codebase_rag/pipeline.py | 35 +++++- java_codebase_rag/progress.py | 35 ++++++ tests/test_ast_graph_build.py | 100 +++++++++++++++ tests/test_cli_progress_stdout_invariant.py | 4 +- tests/test_java_codebase_rag_cli.py | 131 ++++++++++++++++++++ tests/test_progress.py | 101 +++++++++++++++ 10 files changed, 570 insertions(+), 25 deletions(-) diff --git a/build_ast_graph.py b/build_ast_graph.py index 5fda415..4f10b56 100644 --- a/build_ast_graph.py +++ b/build_ast_graph.py @@ -25,6 +25,7 @@ from __future__ import annotations import argparse +import contextlib import hashlib import json import logging @@ -84,6 +85,53 @@ def _verbose_stderr_line(content: str) -> None: print(content, file=sys.stderr, flush=True) +def _emit_graph_progress(parts: dict[str, object], *, verbose: bool) -> None: + """Emit one ``JCIRAG_PROGRESS kind=graph …`` line to stderr (gated by verbose). + + The parent process (``pipeline.run_build_ast_graph`` / + ``run_incremental_graph``) passes ``--verbose`` in default AND verbose modes + (only suppressed for ``--quiet``), so this structured progress surfaces in + default mode (where the parent renders it) and verbose mode (raw relay). In + ``--quiet`` the builder is never invoked with ``--verbose`` so nothing is + emitted. Field order is fixed so the parser and tests can pin substrings. + """ + if not verbose: + return + fields = ["kind=graph"] + for key in ("pass", "done", "total", "status", "elapsed_s"): + if key in parts: + fields.append(f"{key}={parts[key]}") + line = "JCIRAG_PROGRESS " + " ".join(fields) + _verbose_stderr_line(line) + + +# Pass-1 per-file tick cadence: bound stderr volume on huge trees without making +# the bar feel stale. A final tick on pass completion carries status=done. +_PASS1_TICK_EVERY = 25 + + +@contextlib.contextmanager +def _graph_pass_progress(pass_label: str, *, verbose: bool): + """Emit ``pass=N/6 status=running`` on entry and ``status=done elapsed_s=…`` + on exit for passes 2–6 (each advances the rendered bar by 1/6). + + Usage: ``with _graph_pass_progress("2/6", verbose=verbose): …`` + """ + if not verbose: + yield + return + _emit_graph_progress({"pass": pass_label, "status": "running"}, verbose=verbose) + t0 = time.time() + try: + yield + finally: + elapsed = time.time() - t0 + _emit_graph_progress( + {"pass": pass_label, "status": "done", "elapsed_s": f"{elapsed:.2f}"}, + verbose=verbose, + ) + + class _VerbosePassHeartbeats: """Emit ``[tag] running … Ns elapsed`` every 5s on stderr while in scope (verbose only).""" @@ -852,6 +900,19 @@ def pass1_parse(root: Path, tables: GraphTables, *, verbose: bool, scope_files: n_files = 0 if verbose: _verbose_stderr_line(_PASS1_START) + # Count-first: one filtered walk (no parsing) to set the EXACT total before + # the parse loop ticks. Single-layer ignore → the count is exact, so the + # rendered bar is determinate. For a scoped (incremental) parse the total is + # the scope size; for a full rebuild it is the non-ignored .java count. + if verbose: + if scope_files is not None: + pass1_total = len(scope_files) + else: + pass1_total = sum(1 for _ in iter_java_source_files(root, ignore=ignore)) + _emit_graph_progress( + {"pass": "1/6", "done": 0, "total": pass1_total, "status": "running"}, + verbose=verbose, + ) slow_sec = 0.0 raw_slow = os.environ.get("JAVA_CODEBASE_RAG_TEST_GRAPH_SLOW_SEC", "").strip() if raw_slow: @@ -871,6 +932,11 @@ def pass1_parse(root: Path, tables: GraphTables, *, verbose: bool, scope_files: if scope_files is not None and rel not in scope_files: continue n_files += 1 + if verbose and (n_files % _PASS1_TICK_EVERY == 0): + _emit_graph_progress( + {"pass": "1/6", "done": n_files, "status": "running"}, + verbose=verbose, + ) try: content = p.read_bytes() except OSError: @@ -906,6 +972,10 @@ def pass1_parse(root: Path, tables: GraphTables, *, verbose: bool, scope_files: if verbose: elapsed = time.time() - t0 + _emit_graph_progress( + {"pass": "1/6", "done": n_files, "status": "done", "elapsed_s": f"{elapsed:.2f}"}, + verbose=verbose, + ) _verbose_stderr_line( f"[graph] pass 1 · parsed {n_files} files in {elapsed:.2f}s: " f"{len(tables.types)} types, {len(tables.members)} members, " @@ -1145,7 +1215,7 @@ def pass2_edges(tables: GraphTables, asts: dict[str, JavaFileAst], *, verbose: b seen_inj: set[tuple[str, str, str, str]] = set() if verbose: _verbose_stderr_line(_PASS2_START) - with _VerbosePassHeartbeats("[graph] pass 2", verbose=verbose): + with _graph_pass_progress("2/6", verbose=verbose), _VerbosePassHeartbeats("[graph] pass 2", verbose=verbose): for fqn, entry in tables.types.items(): ast = asts.get(entry.file_path) if ast is None: @@ -1818,7 +1888,7 @@ def pass3_calls(tables: GraphTables, asts: dict[str, JavaFileAst], *, verbose: b _verbose_stderr_line(_PASS3_START) _build_member_indexes(tables) stats = CallResolutionStats() - with _VerbosePassHeartbeats("[graph] pass 3", verbose=verbose): + with _graph_pass_progress("3/6", verbose=verbose), _VerbosePassHeartbeats("[graph] pass 3", verbose=verbose): for rel_path, file_ast in asts.items(): try: _process_file_calls(file_ast, rel_path, tables, stats) @@ -1972,7 +2042,7 @@ def pass4_routes( meta_chain = collect_annotation_meta_chain(prs) if verbose: _verbose_stderr_line(_PASS4_START) - with _VerbosePassHeartbeats("[graph] pass 4", verbose=verbose): + with _graph_pass_progress("4/6", verbose=verbose), _VerbosePassHeartbeats("[graph] pass 4", verbose=verbose): for ast in asts.values(): stats.routes_skipped_unresolved += ast.routes_skipped_unresolved @@ -2149,7 +2219,7 @@ def _phantom_async_route_id(call: OutgoingCallDecl) -> str: if verbose: _verbose_stderr_line(_PASS5_START) - with _VerbosePassHeartbeats("[graph] pass 5", verbose=verbose): + with _graph_pass_progress("5/6", verbose=verbose), _VerbosePassHeartbeats("[graph] pass 5", verbose=verbose): for member in sorted(tables.members, key=lambda x: x.node_id): if member.decl.is_constructor: continue @@ -2551,7 +2621,7 @@ def _micro_factor(member: MemberEntry | None) -> float: if verbose: _verbose_stderr_line(_PASS6_START) - with _VerbosePassHeartbeats("[graph] pass 6", verbose=verbose): + with _graph_pass_progress("6/6", verbose=verbose), _VerbosePassHeartbeats("[graph] pass 6", verbose=verbose): for row in tables.http_call_rows: if row.match != "unresolved": continue diff --git a/java_codebase_rag/cli.py b/java_codebase_rag/cli.py index 5fca79f..02173cf 100644 --- a/java_codebase_rag/cli.py +++ b/java_codebase_rag/cli.py @@ -131,15 +131,41 @@ def _run_with_pipeline_progress( cfg: ResolvedOperatorConfig, *, quiet: bool, - work: Callable[[], int], + verbose: bool = False, + work: Callable[["PipelineProgress | None"], int], ) -> int: - if quiet: - return int(work()) + """Run ``work`` under the unified progress renderer (default TTY mode only). + + ``work`` receives a :class:`PipelineProgress` whose ``on_progress`` callback + should be forwarded to the graph/vectors pipeline helpers so their + ``JCIRAG_PROGRESS`` events feed the renderer. In ``--quiet`` or ``--verbose`` + mode the context is ``None`` (no Live region: quiet is silent, verbose + raw-relays subprocess output). + """ + if quiet or verbose: + return int(work(None)) + from java_codebase_rag.progress import IndexProgressRenderer, ProgressEvent + + # PR-2 owns the graph task only; vectors/optimize stay pending (PR-3). + phases = ["graph"] + renderer = IndexProgressRenderer(phases) + progress = PipelineProgress(renderer=renderer) + + def on_progress(ev: ProgressEvent) -> None: + renderer.apply(ev) + + progress.on_progress = on_progress + progress.console = renderer._console # noqa: SLF001 — shared with the drain for Live-safe routing + _pipeline_header(subcommand, cfg) t0 = time.perf_counter() code = 0 + # start() always flips _started (the non-TTY fallback is a no-op for Live but + # still needs the flag so apply() routes to the concise-line printer). The + # TTY Live region is entered inside start() only when the console is a TTY. + renderer.start() try: - code = int(work()) + code = int(work(progress)) return code except BaseException as exc: # Keep footer aligned with process outcome (main maps unhandled Exception -> exit 2). @@ -155,9 +181,26 @@ def _run_with_pipeline_progress( code = 2 raise finally: + renderer.stop() _pipeline_footer(subcommand, t0, code) +class PipelineProgress: + """Progress context handed to ``work``: the renderer + a ready ``on_progress``. + + ``on_progress``/``console`` are wired by :func:`_run_with_pipeline_progress` + and should be forwarded to the pipeline helpers' ``on_progress`` / + ``on_progress_console`` parameters. ``console`` is the renderer's stderr + ``rich.Console`` so the subprocess drain routes non-progress lines through + ``console.print`` while the Live region is up (single-writer invariant). + """ + + def __init__(self, *, renderer: "object | None") -> None: + self.renderer = renderer + self.on_progress: "Callable | None" = None + self.console: "object | None" = None + + def _jsonable(value: Any) -> Any: if hasattr(value, "model_dump"): return value.model_dump() @@ -266,7 +309,7 @@ def _cmd_init(args: argparse.Namespace) -> int: return 2 cfg.index_dir.mkdir(parents=True, exist_ok=True) - def work() -> int: + def work(progress: "PipelineProgress | None") -> int: env = cfg.subprocess_env() verbose = bool(args.verbose) coco = run_cocoindex_update( @@ -295,6 +338,8 @@ def work() -> int: verbose=verbose, quiet=bool(args.quiet), env=env, + on_progress=progress.on_progress if progress is not None else None, + on_progress_console=progress.console if progress is not None else None, ) if g.returncode != 0: _emit( @@ -310,7 +355,9 @@ def work() -> int: _emit({"success": True, "message": "init completed"}) return 0 - return _run_with_pipeline_progress("init", cfg, quiet=bool(args.quiet), work=work) + return _run_with_pipeline_progress( + "init", cfg, quiet=bool(args.quiet), verbose=bool(args.verbose), work=work + ) def _cmd_increment(args: argparse.Namespace) -> int: @@ -323,7 +370,7 @@ def _cmd_increment(args: argparse.Namespace) -> int: if vectors_only: _emit_increment_ladybug_warning() - def work() -> int: + def work(progress: "PipelineProgress | None") -> int: env = cfg.subprocess_env() coco = run_cocoindex_update( env, @@ -356,6 +403,8 @@ def work() -> int: verbose=bool(args.verbose), quiet=bool(args.quiet), env=env, + on_progress=progress.on_progress if progress is not None else None, + on_progress_console=progress.console if progress is not None else None, ) # Check if incremental fell back to full rebuild @@ -389,7 +438,9 @@ def work() -> int: _emit({"success": True, "message": "increment completed (Lance + graph updated)"}) return 0 - return _run_with_pipeline_progress("increment", cfg, quiet=bool(args.quiet), work=work) + return _run_with_pipeline_progress( + "increment", cfg, quiet=bool(args.quiet), verbose=bool(args.verbose), work=work + ) def _cmd_reprocess(args: argparse.Namespace) -> int: @@ -397,7 +448,7 @@ def _cmd_reprocess(args: argparse.Namespace) -> int: _startup_hints(cfg) cfg.apply_to_os_environ() - def work() -> int: + def work(progress: "PipelineProgress | None") -> int: env = cfg.subprocess_env() verbose = bool(args.verbose) vectors_only = bool(getattr(args, "vectors_only", False)) @@ -443,6 +494,8 @@ def work() -> int: verbose=verbose, quiet=bool(args.quiet), env=env, + on_progress=progress.on_progress if progress is not None else None, + on_progress_console=progress.console if progress is not None else None, ) if _is_graph_preflight_blocker(g): payload = { @@ -482,7 +535,9 @@ def work() -> int: _emit_reprocess_outcome(payload) return _reprocess_exit_code(payload) - return _run_with_pipeline_progress("reprocess", cfg, quiet=bool(args.quiet), work=work) + return _run_with_pipeline_progress( + "reprocess", cfg, quiet=bool(args.quiet), verbose=bool(args.verbose), work=work + ) def _cmd_install(args: argparse.Namespace) -> int: @@ -537,7 +592,7 @@ def _cmd_erase(args: argparse.Namespace) -> int: print("Aborted.", file=sys.stderr) return 2 - def work() -> int: + def work(progress: "PipelineProgress | None") -> int: env = cfg.subprocess_env() drop = run_cocoindex_drop(env, quiet=bool(args.quiet)) if drop.returncode == 127: @@ -570,7 +625,7 @@ def work() -> int: _emit({"success": True, "message": "erase completed"}) return 0 - return _run_with_pipeline_progress("erase", cfg, quiet=bool(args.quiet), work=work) + return _run_with_pipeline_progress("erase", cfg, quiet=bool(args.quiet), verbose=bool(getattr(args, "verbose", False)), work=work) def _cmd_meta(args: argparse.Namespace) -> int: diff --git a/java_codebase_rag/cli_format.py b/java_codebase_rag/cli_format.py index 6b4f001..a8b7e80 100644 --- a/java_codebase_rag/cli_format.py +++ b/java_codebase_rag/cli_format.py @@ -25,6 +25,14 @@ b'"event": "brownfield-', b"unknown producer source strategy", b"unknown client source strategy", + # Builder verbose heartbeats / pass banners: in default mode the renderer's + # bar subsumes these, so they must NOT also appear as raw lines above the + # Live region. --verbose raw-relay bypasses this filter and still shows them. + b"[graph] pass ", + b"[graph] scoped write ", + b"[graph] writing ", + b"[graph] done ", + b"[increment] ", ) diff --git a/java_codebase_rag/cli_progress.py b/java_codebase_rag/cli_progress.py index f025738..9b31863 100644 --- a/java_codebase_rag/cli_progress.py +++ b/java_codebase_rag/cli_progress.py @@ -3,8 +3,10 @@ import asyncio import sys +from typing import Callable from java_codebase_rag.cli_format import bold_cyan, is_noise_line, styled_check, styled_cross +from java_codebase_rag.progress import CallbackRenderer, ProgressEvent, ProgressRelay def emit_vectors_start() -> None: @@ -61,8 +63,15 @@ async def accumulate_and_relay_subprocess_streams( *, relay: bool, verbose: bool = True, + on_progress: Callable[[ProgressEvent], None] | None = None, + on_progress_console: object | None = None, ) -> tuple[bytes, bytes]: - """Read stdout and stderr until EOF; optionally copy non-noise stderr chunks to stderr.""" + """Read stdout and stderr until EOF; optionally copy non-noise stderr chunks to stderr. + + When ``on_progress`` is set, stderr is drained through a :class:`ProgressRelay` + so ``JCIRAG_PROGRESS`` lines are parsed and routed to ``on_progress`` (and + suppressed from the relay), matching the sync ``pipeline._popen_capturing_stderr``. + """ stdout = proc.stdout stderr = proc.stderr if stdout is None or stderr is None: @@ -70,7 +79,16 @@ async def accumulate_and_relay_subprocess_streams( out_buf = bytearray() err_buf = bytearray() - filt = _AsyncLineFilter() if (relay and not verbose) else None + if on_progress is not None: + filt = ProgressRelay( + CallbackRenderer(on_progress, on_progress_console), + console=on_progress_console, + verbose=verbose, + ) + elif relay and not verbose: + filt = _AsyncLineFilter() + else: + filt = None async def drain_stdout(reader: asyncio.StreamReader, target: bytearray) -> None: while True: diff --git a/java_codebase_rag/pipeline.py b/java_codebase_rag/pipeline.py index 1caa7e5..8e668dd 100644 --- a/java_codebase_rag/pipeline.py +++ b/java_codebase_rag/pipeline.py @@ -9,10 +9,12 @@ import threading import time from pathlib import Path +from typing import Callable from java_codebase_rag.cli_format import Spinner, is_noise_line, stderr_is_tty from java_codebase_rag.cli_progress import emit_vectors_finish, emit_vectors_start from java_codebase_rag.config import cocoindex_subprocess_env_defaults +from java_codebase_rag.progress import CallbackRenderer, ProgressEvent, ProgressRelay COCOINDEX_TARGET = "java_index_flow_lancedb.py:JavaCodeIndexLance" @@ -66,11 +68,28 @@ def _popen_capturing_stderr( proc: subprocess.Popen[bytes], *, verbose: bool = True, + on_progress: Callable[[ProgressEvent], None] | None = None, + on_progress_console: object | None = None, ) -> tuple[str, str, int]: - """Capture stdout/stderr; relay stderr through noise filter (or verbatim in verbose mode).""" + """Capture stdout/stderr; relay stderr through noise filter (or verbatim in verbose mode). + + When ``on_progress`` is set, stderr is drained through a :class:`ProgressRelay` + instead of the bare ``_LineFilter``: progress lines are parsed first, routed to + ``on_progress``, and suppressed from the relay; non-progress lines follow the + relay's routing (``console.print`` while a Live region is up via + ``on_progress_console``, raw ``buffer.write`` in verbose mode). + """ out_buf = bytearray() err_buf = bytearray() - filt = _LineFilter() if not verbose else None + if on_progress is not None: + relay = ProgressRelay( + CallbackRenderer(on_progress, on_progress_console), + console=on_progress_console, + verbose=verbose, + ) + filt: _LineFilter | ProgressRelay | None = relay + else: + filt = _LineFilter() if not verbose else None def drain_out() -> None: assert proc.stdout is not None @@ -259,6 +278,8 @@ def run_build_ast_graph( verbose: bool, quiet: bool = False, env: dict[str, str] | None = None, + on_progress: Callable[[ProgressEvent], None] | None = None, + on_progress_console: object | None = None, ) -> subprocess.CompletedProcess[str]: builder = bundle_dir() / "build_ast_graph.py" if not builder.is_file(): @@ -297,7 +318,9 @@ def run_build_ast_graph( stderr=subprocess.PIPE, bufsize=0, ) - out_s, err_s, code = _popen_capturing_stderr(proc, verbose=verbose) + out_s, err_s, code = _popen_capturing_stderr( + proc, verbose=verbose, on_progress=on_progress, on_progress_console=on_progress_console + ) if not verbose: from java_codebase_rag.cli_format import bold_cyan, styled_check, styled_cross marker = styled_check() if code == 0 else styled_cross() @@ -312,6 +335,8 @@ def run_incremental_graph( verbose: bool, quiet: bool = False, env: dict[str, str] | None = None, + on_progress: Callable[[ProgressEvent], None] | None = None, + on_progress_console: object | None = None, ) -> subprocess.CompletedProcess[str]: """Run incremental graph rebuild by passing --incremental flag to build_ast_graph.py.""" builder = bundle_dir() / "build_ast_graph.py" @@ -352,7 +377,9 @@ def run_incremental_graph( stderr=subprocess.PIPE, bufsize=0, ) - out_s, err_s, code = _popen_capturing_stderr(proc, verbose=verbose) + out_s, err_s, code = _popen_capturing_stderr( + proc, verbose=verbose, on_progress=on_progress, on_progress_console=on_progress_console + ) if not verbose: from java_codebase_rag.cli_format import bold_cyan, styled_check, styled_cross marker = styled_check() if code == 0 else styled_cross() diff --git a/java_codebase_rag/progress.py b/java_codebase_rag/progress.py index 7527836..baa9725 100644 --- a/java_codebase_rag/progress.py +++ b/java_codebase_rag/progress.py @@ -55,6 +55,7 @@ "parse_progress_line", "IndexProgressRenderer", "ProgressRelay", + "CallbackRenderer", ] ProgressKind = Literal["vectors", "graph", "optimize"] @@ -271,7 +272,16 @@ def apply(self, ev: ProgressEvent) -> None: ``status == "failed"`` halts the task and marks the description with a red ``✗`` (rich renders the spinner stopped). On non-TTY consoles this delegates to the throttled concise-line printer. + + Safe to call after :meth:`stop`: once the Live region is torn down a + drain thread may still feed a trailing event; this is a no-op then so + the apply/stop pair is atomic from the caller's view (PR-1 review + Minor #6 / cross-PR risk #10). """ + if not self._started: + # After stop() the Live region is gone and rich tasks are torn down; + # a trailing event from the drain thread must not mutate state. + return if self._fallback: self._fallback_apply(ev) return @@ -375,6 +385,31 @@ def _format_concise( return Text(kind) +class CallbackRenderer: + """Adapter exposing a ``renderer.apply(ev)`` surface to :class:`ProgressRelay`. + + ``ProgressRelay`` calls ``renderer.apply(ev)`` for each parsed progress line. + This adapter forwards to a caller-supplied ``on_progress`` callback (used by + the sync/async subprocess drains in ``pipeline`` / ``cli_progress`` to route + progress events up to the command-level renderer without owning a Live region + themselves). It carries a ``_console`` attribute so the relay can route + non-progress lines through ``console.print`` while a Live region is up. + """ + + def __init__(self, on_progress, console=None) -> None: # type: ignore[no-untyped-def] + self._on_progress = on_progress + self._console = console + + def apply(self, ev: ProgressEvent) -> None: + self._on_progress(ev) + + def start(self) -> None: + pass + + def stop(self) -> None: + pass + + # --------------------------------------------------------------------------- # Relay # --------------------------------------------------------------------------- diff --git a/tests/test_ast_graph_build.py b/tests/test_ast_graph_build.py index b78a1ac..57e06ea 100644 --- a/tests/test_ast_graph_build.py +++ b/tests/test_ast_graph_build.py @@ -404,3 +404,103 @@ def test_pass3_known_external_calls_preserved(ladybug_db_path: Path) -> None: found = [str(r[0]) for r in rows] assert found, "bank fixture should have known-external CALLS rows" assert all(s not in ("phantom", "chained_receiver") for s in found), found + + +# --------------------------------------------------------------------------- +# Graph-phase JCIRAG_PROGRESS emission (PR-2) +# --------------------------------------------------------------------------- + + +def _run_builder_verbose(corpus_root: Path, target_db: Path, *, extra_args: list[str] | None = None) -> subprocess.CompletedProcess: + """Run build_ast_graph.py --verbose and return the CompletedProcess.""" + script = Path(__file__).resolve().parent.parent / "build_ast_graph.py" + cmd = [ + sys.executable, + str(script), + "--source-root", str(corpus_root), + "--ladybug-path", str(target_db), + "--verbose", + *(extra_args or []), + ] + return subprocess.run(cmd, capture_output=True, text=True, timeout=300) + + +def _progress_lines(stderr: str) -> list[str]: + return [ln for ln in stderr.splitlines() if "JCIRAG_PROGRESS kind=graph" in ln] + + +def _count_filtered_java_files(corpus_root: Path) -> int: + from path_filtering import LayeredIgnore, iter_java_source_files + + return sum(1 for _ in iter_java_source_files(corpus_root.resolve(), ignore=LayeredIgnore(corpus_root.resolve()))) + + +def test_build_ast_graph_pass1_emits_per_file_progress(corpus_root: Path, tmp_path: Path) -> None: + """Pass 1 is count-first: a `total=` line precedes the first `done=` tick; ticks advance.""" + target = tmp_path / "p1.lbug" + proc = _run_builder_verbose(corpus_root, target) + assert proc.returncode == 0, f"stderr:\n{proc.stderr}" + lines = _progress_lines(proc.stderr) + # Pass-1 lines carry pass=1/6 with a total and per-file done ticks. + pass1 = [ln for ln in lines if "pass=1/6" in ln] + assert pass1, f"expected pass 1 progress lines, got: {lines!r}" + # The first pass-1 line with a total must precede the first line with a done tick. + totals = [ln for ln in pass1 if "total=" in ln] + dones = [ln for ln in pass1 if "done=" in ln] + assert totals, f"pass 1 must emit a count-first total; lines: {pass1!r}" + assert dones, f"pass 1 must emit per-file done ticks; lines: {pass1!r}" + first_total_idx = lines.index(totals[0]) + first_done_idx = lines.index(dones[0]) + assert first_total_idx <= first_done_idx, "total must precede the first done tick" + # Done ticks advance (monotonic non-decreasing values). + done_vals = [int(re.search(r"done=(\d+)", ln).group(1)) for ln in dones if re.search(r"done=(\d+)", ln)] + assert done_vals == sorted(done_vals), f"done ticks must be monotonic: {done_vals}" + + +def test_build_ast_graph_pass1_total_is_exact_filtered_count(corpus_root: Path, tmp_path: Path) -> None: + """The count-first pass-1 total equals the exact non-ignored .java file count.""" + target = tmp_path / "p1total.lbug" + proc = _run_builder_verbose(corpus_root, target) + assert proc.returncode == 0, f"stderr:\n{proc.stderr}" + lines = _progress_lines(proc.stderr) + pass1_total_lines = [ + ln for ln in lines if "pass=1/6" in ln and "total=" in ln and "done=" in ln + ] + # Fall back to any pass-1 line carrying a total. + if not pass1_total_lines: + pass1_total_lines = [ln for ln in lines if "pass=1/6" in ln and "total=" in ln] + assert pass1_total_lines, f"no pass-1 total line found; lines: {lines!r}" + totals = [int(re.search(r"total=(\d+)", ln).group(1)) for ln in pass1_total_lines if re.search(r"total=(\d+)", ln)] + assert totals, f"could not parse total from: {pass1_total_lines!r}" + expected = _count_filtered_java_files(corpus_root) + assert totals[0] == expected, f"pass-1 total {totals[0]} != filtered count {expected}" + + +def test_build_ast_graph_passes_2_to_6_emit_step_progress(corpus_root: Path, tmp_path: Path) -> None: + """Each of passes 2–6 emits a `pass=N/6` step line on entry/exit.""" + target = tmp_path / "p2to6.lbug" + proc = _run_builder_verbose(corpus_root, target) + assert proc.returncode == 0, f"stderr:\n{proc.stderr}" + lines = _progress_lines(proc.stderr) + for n in range(2, 7): + step_lines = [ln for ln in lines if f"pass={n}/6" in ln] + assert step_lines, f"pass {n}/6 emitted no progress lines; full: {lines!r}" + + +def test_build_ast_graph_quiet_emits_no_progress(corpus_root: Path, tmp_path: Path) -> None: + """Without --verbose the builder emits no JCIRAG_PROGRESS lines.""" + script = Path(__file__).resolve().parent.parent / "build_ast_graph.py" + target = tmp_path / "quiet.lbug" + proc = subprocess.run( + [ + sys.executable, + str(script), + "--source-root", str(corpus_root), + "--ladybug-path", str(target), + ], + capture_output=True, + text=True, + timeout=300, + ) + assert proc.returncode == 0, f"stderr:\n{proc.stderr}" + assert _progress_lines(proc.stderr) == [], "quiet build must not emit JCIRAG_PROGRESS" diff --git a/tests/test_cli_progress_stdout_invariant.py b/tests/test_cli_progress_stdout_invariant.py index 444f826..739f879 100644 --- a/tests/test_cli_progress_stdout_invariant.py +++ b/tests/test_cli_progress_stdout_invariant.py @@ -272,7 +272,7 @@ def capture_footer(_sub: str, _t0: float, code: int) -> None: monkeypatch.setattr(cli_mod, "_pipeline_footer", capture_footer) - def boom() -> int: + def boom(_progress) -> int: raise RuntimeError("simulated handler failure") with pytest.raises(RuntimeError, match="simulated handler failure"): @@ -281,7 +281,7 @@ def boom() -> int: codes.clear() - def exit5() -> int: + def exit5(_progress) -> int: raise SystemExit(5) with pytest.raises(SystemExit) as excinfo: diff --git a/tests/test_java_codebase_rag_cli.py b/tests/test_java_codebase_rag_cli.py index f2b2c29..3b35324 100644 --- a/tests/test_java_codebase_rag_cli.py +++ b/tests/test_java_codebase_rag_cli.py @@ -1230,3 +1230,134 @@ def test_console_script_entry_point_routes_through_wrapper() -> None: assert 'java-codebase-rag = "java_codebase_rag.cli:_console_script_main"' in pyproject assert 'java-codebase-rag = "java-codebase-rag:main"' not in pyproject assert 'java-codebase-rag = "java_codebase_rag.cli:main"' not in pyproject + + +# --------------------------------------------------------------------------- +# PR-2: graph-phase progress wiring (default vs --quiet) +# --------------------------------------------------------------------------- + + +def _make_stub_completed(*, returncode: int = 0, stderr: str = "") -> "subprocess.CompletedProcess[str]": + import subprocess + + return subprocess.CompletedProcess(args=["stub"], returncode=returncode, stdout="", stderr=stderr) + + +def _patch_pipeline_for_graph_progress(monkeypatch: pytest.MonkeyPatch, *, emit_graph: bool) -> None: + """Patch the cocoindex + graph pipeline helpers used by init/increment. + + When ``emit_graph`` is True the patched graph helper invokes the caller's + ``on_progress`` callback with a synthetic ``kind=graph`` event — simulating + what the real subprocess drain would feed the renderer in default mode. + """ + from java_codebase_rag import cli as _cli + from java_codebase_rag import pipeline as _pipeline + + def _fake_cocoindex_update(env, *, full_reprocess, quiet, verbose=True, lance_project_root=None): + return _make_stub_completed(returncode=0) + + def _fake_run_build_ast_graph(*, source_root, ladybug_path, verbose, quiet=False, env=None, on_progress=None, on_progress_console=None): + if emit_graph and on_progress is not None: + from java_codebase_rag.progress import ProgressEvent + + on_progress( + ProgressEvent( + kind="graph", phase=None, pass_="1/6", done=10, total=130, + status="running", elapsed_s=None, + ) + ) + return _make_stub_completed(returncode=0) + + def _fake_run_incremental_graph(*, source_root, ladybug_path, verbose, quiet=False, env=None, on_progress=None, on_progress_console=None): + if emit_graph and on_progress is not None: + from java_codebase_rag.progress import ProgressEvent + + on_progress( + ProgressEvent( + kind="graph", phase=None, pass_="1/6", done=3, total=130, + status="running", elapsed_s=None, + ) + ) + return _make_stub_completed(returncode=0) + + # Patch where cli.py imported them (module-level names in cli). + monkeypatch.setattr(_cli, "run_cocoindex_update", _fake_cocoindex_update) + monkeypatch.setattr(_cli, "run_build_ast_graph", _fake_run_build_ast_graph) + monkeypatch.setattr(_cli, "run_incremental_graph", _fake_run_incremental_graph) + # Also patch the pipeline module attributes in case anything imports there. + monkeypatch.setattr(_pipeline, "run_cocoindex_update", _fake_cocoindex_update) + monkeypatch.setattr(_pipeline, "run_build_ast_graph", _fake_run_build_ast_graph) + monkeypatch.setattr(_pipeline, "run_incremental_graph", _fake_run_incremental_graph) + + +def test_cli_init_default_mode_graph_phase_progress_on_stderr( + corpus_root: Path, tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """In default mode a graph-phase progress event is parsed and rendered to + stderr; the raw ``JCIRAG_PROGRESS`` line is NOT echoed verbatim.""" + idx = tmp_path / "idx_init_prog" + idx.mkdir() + monkeypatch.setenv("JAVA_CODEBASE_RAG_INDEX_DIR", str(idx)) + monkeypatch.setenv("JAVA_CODEBASE_RAG_SOURCE_ROOT", str(corpus_root)) + _patch_pipeline_for_graph_progress(monkeypatch, emit_graph=True) + buf = io.StringIO() + with contextlib.redirect_stderr(buf): + rc = cli_mod.main( + ["init", "--source-root", str(corpus_root), "--index-dir", str(idx)] + ) + assert rc == 0 + err = buf.getvalue() + # The raw structured line is consumed by the parser, never raw-relayed. + assert "JCIRAG_PROGRESS kind=graph" not in err + # But graph-phase progress IS rendered (non-TTY concise fallback prints a + # "graph ..." line). The synthetic event had total=130, done=10. + assert "graph" in err.lower() + + +def test_cli_increment_graph_phase_progress( + corpus_root: Path, tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """Symmetric: increment default mode parses and renders graph progress.""" + idx = tmp_path / "idx_inc_prog" + idx.mkdir() + monkeypatch.setenv("JAVA_CODEBASE_RAG_INDEX_DIR", str(idx)) + monkeypatch.setenv("JAVA_CODEBASE_RAG_SOURCE_ROOT", str(corpus_root)) + # init first (quiet) to populate the index dir so increment has state. + _patch_pipeline_for_graph_progress(monkeypatch, emit_graph=False) + init_rc = cli_mod.main( + ["init", "--source-root", str(corpus_root), "--index-dir", str(idx), "--quiet"] + ) + assert init_rc == 0 + # Now increment in default mode with graph progress emitted. + _patch_pipeline_for_graph_progress(monkeypatch, emit_graph=True) + buf = io.StringIO() + with contextlib.redirect_stderr(buf): + rc = cli_mod.main( + ["increment", "--source-root", str(corpus_root), "--index-dir", str(idx)] + ) + assert rc == 0 + err = buf.getvalue() + assert "JCIRAG_PROGRESS kind=graph" not in err + assert "graph" in err.lower() + + +def test_cli_graph_progress_absent_when_quiet( + corpus_root: Path, tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """--quiet suppresses all progress stderr; no graph rendering occurs.""" + idx = tmp_path / "idx_quiet_prog" + idx.mkdir() + monkeypatch.setenv("JAVA_CODEBASE_RAG_INDEX_DIR", str(idx)) + monkeypatch.setenv("JAVA_CODEBASE_RAG_SOURCE_ROOT", str(corpus_root)) + _patch_pipeline_for_graph_progress(monkeypatch, emit_graph=True) + buf = io.StringIO() + with contextlib.redirect_stderr(buf): + rc = cli_mod.main( + ["init", "--source-root", str(corpus_root), "--index-dir", str(idx), "--quiet"] + ) + assert rc == 0 + err = buf.getvalue() + assert "JCIRAG_PROGRESS kind=graph" not in err + # In quiet mode there is no header/footer framing either. + assert "java-codebase-rag init" not in err + diff --git a/tests/test_progress.py b/tests/test_progress.py index ba25e83..172a4ee 100644 --- a/tests/test_progress.py +++ b/tests/test_progress.py @@ -311,3 +311,104 @@ def test_non_tty_fallback_emits_concise_lines() -> None: assert "42.1" in final finally: r.stop() + + +# --------------------------------------------------------------------------- +# PR-2: drain-thread safety (apply/stop atomicity) + heartbeat suppression +# --------------------------------------------------------------------------- + + +def test_renderer_apply_is_noop_after_stop() -> None: + """apply() after stop() must not raise and must not mutate task state. + + Once the drain thread is wired (PR-2), a progress event can arrive just as + the main thread calls stop(); apply() must be a safe no-op in that window. + """ + console = Console(file=io.StringIO(), force_terminal=False, width=80) + r = IndexProgressRenderer(["graph"], console=console) + assert r._fallback is True # non-TTY path + r.start() + # A normal event before stop sets state. + r.apply( + ProgressEvent( + kind="graph", phase=None, pass_="1/6", done=10, total=100, status="running", elapsed_s=None + ) + ) + r.stop() + # Feeding events after stop must not raise and must not change state. + for _ in range(5): + r.apply( + ProgressEvent( + kind="graph", phase=None, pass_="1/6", done=50, total=100, status="running", elapsed_s=None + ) + ) + # No exception → pass. The carry-forward total stays at the last-seen value. + assert r._last_total["graph"] == 100 + + +def test_renderer_apply_stop_stress_does_not_crash() -> None: + """Concurrent apply()/stop() from two threads must not raise (drain-thread model).""" + import threading + + console = Console(file=io.StringIO(), force_terminal=False, width=80) + r = IndexProgressRenderer(["graph"], console=console) + stop_flag = threading.Event() + errors: list[BaseException] = [] + + def feeder() -> None: + i = 0 + while not stop_flag.is_set(): + try: + r.apply( + ProgressEvent( + kind="graph", + phase=None, + pass_="1/6", + done=i, + total=1000, + status="running", + elapsed_s=None, + ) + ) + except BaseException as exc: # noqa: BLE001 + errors.append(exc) + i += 1 + + t = threading.Thread(target=feeder, daemon=True) + t.start() + try: + for _ in range(20): + r.start() + stop_flag.clear() + # let the feeder run briefly + for _ in range(100): + pass + r.stop() + finally: + stop_flag.set() + t.join(timeout=2.0) + assert not errors, f"concurrent apply/stop raised: {errors!r}" + + +def test_progress_relay_suppresses_graph_heartbeat_in_live_mode() -> None: + """In default (Live) mode the builder's `[graph] pass N` heartbeat must NOT + leak as a raw line above the Live region — the renderer's bar subsumes it. + The heartbeat only reappears in --verbose raw-relay mode (no Live region). + """ + # Live region active (renderer attached) → heartbeat is noise-suppressed. + stub = _StubRenderer() + buf = io.StringIO() + relay = ProgressRelay( + renderer=stub, console=Console(file=buf, force_terminal=False, force_interactive=False) + ) + relay.feed(b"[graph] pass 1 \xc2\xb7 5s elapsed\n") + relay.feed(b"JCIRAG_PROGRESS kind=graph pass=1/6 done=10 total=100\n") + relay.flush() + # The structured progress line was routed to the renderer (1 apply). + assert len(stub.applied) == 1 + assert stub.applied[0].kind == "graph" + # The heartbeat was not echoed as a raw line above the Live region. The + # renderer's bar subsumes it; assert the heartbeat content is entirely + # absent (not just the ``[graph]`` markup tag, which rich would strip). + assert "5s elapsed" not in buf.getvalue() + From 418413ff47fe3623be408ed7cc3f47afb8115768 Mon Sep 17 00:00:00 2001 From: Dmitry Teryaev Date: Mon, 15 Jun 2026 02:01:14 +0300 Subject: [PATCH 07/11] harden PR-2: survive renderer exceptions, exact incremental total, relay factory - ProgressRelay guards renderer.apply() so a render-chain exception can't silently kill the drain thread (try/except + stderr note) - incremental pass-1 total excludes removed files (done no longer undercounts then clamps) - make_relay() factory centralizes the sync+async drain wiring - drop unreachable console fallback in ProgressRelay._route_line erase unused-renderer (review Minor #3) and failing-builder tests (Minor #6) deferred. Co-Authored-By: Claude --- build_ast_graph.py | 26 +++++++++++++--- java_codebase_rag/cli_progress.py | 8 ++--- java_codebase_rag/pipeline.py | 8 ++--- java_codebase_rag/progress.py | 44 ++++++++++++++++++++++++--- tests/test_ast_graph_build.py | 36 +++++++++++++++++++++++ tests/test_progress.py | 49 +++++++++++++++++++++++++++++++ 6 files changed, 152 insertions(+), 19 deletions(-) diff --git a/build_ast_graph.py b/build_ast_graph.py index 4f10b56..6b2c377 100644 --- a/build_ast_graph.py +++ b/build_ast_graph.py @@ -885,7 +885,14 @@ def _register_type( return entry -def pass1_parse(root: Path, tables: GraphTables, *, verbose: bool, scope_files: set[str] | None = None) -> dict[str, JavaFileAst]: +def pass1_parse( + root: Path, + tables: GraphTables, + *, + verbose: bool, + scope_files: set[str] | None = None, + removed_files: set[str] | None = None, +) -> dict[str, JavaFileAst]: """Walk files, parse them, populate node indexes. Returns path -> AST. Args: @@ -893,6 +900,11 @@ def pass1_parse(root: Path, tables: GraphTables, *, verbose: bool, scope_files: tables: GraphTables to populate. verbose: Whether to emit progress output. scope_files: Optional set of relative POSIX paths to parse. If None, parse all files. + removed_files: Optional set of relative POSIX paths that no longer exist + on disk (incremental deletions). These are members of ``scope_files`` + (they were deleted, so they participate in scoped deletion) but are + never visited by the parse walk, so they must be excluded from the + pass-1 total to keep ``done`` from undercounting then two-way-clamping. """ asts: dict[str, JavaFileAst] = {} ignore = LayeredIgnore(root) @@ -903,10 +915,14 @@ def pass1_parse(root: Path, tables: GraphTables, *, verbose: bool, scope_files: # Count-first: one filtered walk (no parsing) to set the EXACT total before # the parse loop ticks. Single-layer ignore → the count is exact, so the # rendered bar is determinate. For a scoped (incremental) parse the total is - # the scope size; for a full rebuild it is the non-ignored .java count. + # the number of files that will actually be visited: scope minus any removed + # files (which are members of scope for deletion but gone from disk, so the + # parse walk never ticks them); for a full rebuild it is the non-ignored + # .java count. if verbose: if scope_files is not None: - pass1_total = len(scope_files) + removed = removed_files if removed_files is not None else set() + pass1_total = len(scope_files - removed) else: pass1_total = sum(1 for _ in iter_java_source_files(root, ignore=ignore)) _emit_graph_progress( @@ -3656,7 +3672,9 @@ def incremental_rebuild( _verbose_stderr_line("[increment] rebuilding scoped files (passes 1-4)") tables = GraphTables() - asts = pass1_parse(source_root, tables, verbose=verbose, scope_files=scope_files) + asts = pass1_parse( + source_root, tables, verbose=verbose, scope_files=scope_files, removed_files=removed + ) # Load existing types and members for cross-file resolution (only from unchanged files) _load_existing_types(conn, tables, exclude_files=scope_files) diff --git a/java_codebase_rag/cli_progress.py b/java_codebase_rag/cli_progress.py index 9b31863..0419252 100644 --- a/java_codebase_rag/cli_progress.py +++ b/java_codebase_rag/cli_progress.py @@ -6,7 +6,7 @@ from typing import Callable from java_codebase_rag.cli_format import bold_cyan, is_noise_line, styled_check, styled_cross -from java_codebase_rag.progress import CallbackRenderer, ProgressEvent, ProgressRelay +from java_codebase_rag.progress import ProgressEvent, make_relay def emit_vectors_start() -> None: @@ -80,11 +80,7 @@ async def accumulate_and_relay_subprocess_streams( out_buf = bytearray() err_buf = bytearray() if on_progress is not None: - filt = ProgressRelay( - CallbackRenderer(on_progress, on_progress_console), - console=on_progress_console, - verbose=verbose, - ) + filt = make_relay(on_progress, console=on_progress_console, verbose=verbose) elif relay and not verbose: filt = _AsyncLineFilter() else: diff --git a/java_codebase_rag/pipeline.py b/java_codebase_rag/pipeline.py index 8e668dd..d5eb7a0 100644 --- a/java_codebase_rag/pipeline.py +++ b/java_codebase_rag/pipeline.py @@ -14,7 +14,7 @@ from java_codebase_rag.cli_format import Spinner, is_noise_line, stderr_is_tty from java_codebase_rag.cli_progress import emit_vectors_finish, emit_vectors_start from java_codebase_rag.config import cocoindex_subprocess_env_defaults -from java_codebase_rag.progress import CallbackRenderer, ProgressEvent, ProgressRelay +from java_codebase_rag.progress import ProgressEvent, ProgressRelay, make_relay COCOINDEX_TARGET = "java_index_flow_lancedb.py:JavaCodeIndexLance" @@ -82,10 +82,8 @@ def _popen_capturing_stderr( out_buf = bytearray() err_buf = bytearray() if on_progress is not None: - relay = ProgressRelay( - CallbackRenderer(on_progress, on_progress_console), - console=on_progress_console, - verbose=verbose, + relay = make_relay( + on_progress, console=on_progress_console, verbose=verbose ) filt: _LineFilter | ProgressRelay | None = relay else: diff --git a/java_codebase_rag/progress.py b/java_codebase_rag/progress.py index baa9725..62c8a45 100644 --- a/java_codebase_rag/progress.py +++ b/java_codebase_rag/progress.py @@ -33,7 +33,7 @@ import sys import time from dataclasses import dataclass -from typing import Literal +from typing import Callable, Literal from rich.console import Console from rich.live import Live @@ -56,6 +56,7 @@ "IndexProgressRenderer", "ProgressRelay", "CallbackRenderer", + "make_relay", ] ProgressKind = Literal["vectors", "graph", "optimize"] @@ -410,6 +411,28 @@ def stop(self) -> None: pass +def make_relay( + on_progress: Callable[[ProgressEvent], None], + *, + console: object | None, + verbose: bool, +) -> ProgressRelay: + """Build the standard drain-side :class:`ProgressRelay`. + + Both subprocess drains (``pipeline._popen_capturing_stderr`` sync path and + ``cli_progress.accumulate_and_relay_subprocess_streams`` async path) wire a + ``CallbackRenderer`` that forwards parsed events to the command-level + ``on_progress`` callback, and route non-progress lines through the same + caller-supplied ``console``. Centralizing the construction here keeps the + sync and async wiring identical (PR-3 forks the async path further). + """ + return ProgressRelay( + CallbackRenderer(on_progress, console), + console=console, + verbose=verbose, + ) + + # --------------------------------------------------------------------------- # Relay # --------------------------------------------------------------------------- @@ -466,7 +489,18 @@ def _route_line(self, line: bytes) -> None: # Consumed by the protocol — never echoed to any sink. It is not # noise, so it must not keep the suppression flag armed. self._suppress_next = False - self._renderer.apply(ev) + # Guard the render chain: a drain thread is a daemon, so an + # unhandled exception here dies silently and truncates the captured + # stderr, masking real bugs. Mirror the defensive ``except Exception`` + # around the raw ``buffer.write`` path below: log a one-line note to + # real stderr and keep draining. Never re-raise. + try: + self._renderer.apply(ev) + except Exception as exc: + sys.stderr.write( + f"java-codebase-rag: progress renderer error: {exc}\n" + ) + sys.stderr.flush() return if ev is not None and self._renderer is None: # Parsed as progress but no renderer attached: still reset the flag @@ -485,11 +519,13 @@ def _route_line(self, line: bytes) -> None: self._suppress_next = False text = line.decode("utf-8", errors="replace") if self._renderer is not None and self._live_active: - console = self._console if self._console is not None else self._renderer._console # noqa: SLF001 + # The drains always construct the relay with the caller's console + # (see make_relay), so ``self._console`` is authoritative here. + assert self._console is not None # invariant: drains always supply it # rich.Console over a Live region must suspend/resume to interleave # a one-off line without corrupting the bar redraw; print() handles # this correctly when the Live was started on the same console. - console.print(text, end="") + self._console.print(text, end="") return if self._verbose and self._renderer is None: try: diff --git a/tests/test_ast_graph_build.py b/tests/test_ast_graph_build.py index 57e06ea..3751fad 100644 --- a/tests/test_ast_graph_build.py +++ b/tests/test_ast_graph_build.py @@ -504,3 +504,39 @@ def test_build_ast_graph_quiet_emits_no_progress(corpus_root: Path, tmp_path: Pa ) assert proc.returncode == 0, f"stderr:\n{proc.stderr}" assert _progress_lines(proc.stderr) == [], "quiet build must not emit JCIRAG_PROGRESS" + + +def test_pass1_parse_incremental_total_excludes_removed_files(tmp_path: Path, capsys: pytest.CaptureFixture[str]) -> None: + """Incremental pass-1 total must count only files that will actually be visited. + + On an incremental run, ``scope_files`` includes removed files (they were + deleted, so they participate in scoped deletion), but they no longer exist + on disk and are therefore never visited by the parse walk. Counting them in + ``pass1_total`` makes ``done`` undercount then two-way-clamp on completion. + The fix: the total is ``len(scope_files - removed_files)``. + """ + import build_ast_graph + + root = tmp_path / "proj" + java_dir = root / "src/main/java/smoke" + java_dir.mkdir(parents=True) + (java_dir / "Real.java").write_text( + "package smoke;\nclass Real { void go() { } }\n", encoding="utf-8" + ) + tables = build_ast_graph.GraphTables() + # scope includes the real file plus a removed (gone-from-disk) file. + scope_files = {"src/main/java/smoke/Real.java", "src/main/java/smoke/Gone.java"} + removed_files = {"src/main/java/smoke/Gone.java"} + build_ast_graph.pass1_parse( + root, tables, verbose=True, scope_files=scope_files, removed_files=removed_files + ) + captured = capsys.readouterr() + pass1_totals = [ + int(m.group(1)) + for m in re.finditer(r"pass=1/6 done=0 total=(\d+)", captured.err) + ] + assert pass1_totals, f"expected a count-first pass-1 total line; stderr:\n{captured.err}" + # The removed file must NOT be counted: total is 1 (only Real.java), not 2. + assert pass1_totals[0] == 1, ( + f"incremental pass-1 total must exclude removed files; got {pass1_totals[0]}" + ) diff --git a/tests/test_progress.py b/tests/test_progress.py index 172a4ee..872504a 100644 --- a/tests/test_progress.py +++ b/tests/test_progress.py @@ -412,3 +412,52 @@ def test_progress_relay_suppresses_graph_heartbeat_in_live_mode() -> None: # absent (not just the ``[graph]`` markup tag, which rich would strip). assert "5s elapsed" not in buf.getvalue() + +# --------------------------------------------------------------------------- +# PR-2 review hardening: survive a renderer exception in the drain thread +# --------------------------------------------------------------------------- + + +class _ExplodingRenderer: + """A renderer whose apply() raises on every call.""" + + def __init__(self) -> None: + self.applied: list[ProgressEvent] = [] + + def apply(self, ev: ProgressEvent) -> None: + raise RuntimeError("boom") + + def start(self) -> None: + pass + + def stop(self) -> None: + pass + + +def test_progress_relay_survives_renderer_apply_exception(capsys) -> None: + """A renderer exception must NOT propagate out of feed() and must not kill + the drain thread (which would silently truncate the captured stderr). + + Feed a JCIRAG_PROGRESS line (triggers the raise) followed by a normal + non-noise line; assert feed() returns cleanly and the normal line still + routes to the sink (console). The exception is noted to stderr instead. + """ + relay = ProgressRelay( + renderer=_ExplodingRenderer(), + console=Console(file=io.StringIO(), force_terminal=False, force_interactive=False), + ) + buf = io.StringIO() + # Swap in a console backed by a buffer we can inspect for the normal line. + relay._console = Console(file=buf, force_terminal=False, force_interactive=False) # noqa: SLF001 + # The progress line triggers renderer.apply() → RuntimeError("boom"). + relay.feed(b"JCIRAG_PROGRESS kind=graph pass=1/6 done=10 total=100\n") + # A normal non-noise line after the exception must still route to the sink. + relay.feed(b"cocoindex: indexing batch\n") + relay.flush() + out = buf.getvalue() + assert "cocoindex: indexing batch" in out + # The exception was noted to real stderr (capsys captures sys.stderr). + captured = capsys.readouterr() + assert "progress renderer error" in captured.err + assert "boom" in captured.err + From 327ff91965c76f304adf158a9d23d630579b9004 Mon Sep 17 00:00:00 2001 From: Dmitry Teryaev Date: Mon, 15 Jun 2026 02:48:21 +0300 Subject: [PATCH 08/11] wire vectors-phase index progress; retire Spinner/emit_vectors; optimize phase process_*_file emit JCIRAG_PROGRESS kind=vectors (approximate total from app_main pre-walk + throttled per-file ticks); the parent emits the terminal vectors event on cocoindex exit (drives clamp-on-completion). optimize_lance_tables emits kind=optimize in-process (covers both call sites). server.run_refresh_pipeline wires the async drain + renderer. Retire Spinner + emit_vectors_start/_finish. Operator commands render Vectors -> Optimize -> Graph. Co-Authored-By: Claude --- java_codebase_rag/cli.py | 28 +- java_codebase_rag/cli_format.py | 35 --- java_codebase_rag/cli_progress.py | 20 +- java_codebase_rag/lance_optimize.py | 171 +++++++--- java_codebase_rag/pipeline.py | 61 ++-- java_index_flow_lancedb.py | 145 +++++++++ server.py | 45 ++- tests/test_cli_progress_stdout_invariant.py | 2 +- tests/test_java_codebase_rag_cli.py | 6 +- tests/test_vectors_progress.py | 330 ++++++++++++++++++++ 10 files changed, 703 insertions(+), 140 deletions(-) create mode 100644 tests/test_vectors_progress.py diff --git a/java_codebase_rag/cli.py b/java_codebase_rag/cli.py index 02173cf..7981c84 100644 --- a/java_codebase_rag/cli.py +++ b/java_codebase_rag/cli.py @@ -146,8 +146,13 @@ def _run_with_pipeline_progress( return int(work(None)) from java_codebase_rag.progress import IndexProgressRenderer, ProgressEvent - # PR-2 owns the graph task only; vectors/optimize stay pending (PR-3). - phases = ["graph"] + # PR-3 owns all three tasks in order: Vectors → Optimize → Graph. The vectors + # task is fed by the cocoindex child's per-file ticks + approximate total + # (subprocess transport, parsed by ProgressRelay); the optimize task is fed + # in-process by lance_optimize; the graph task is fed by the build_ast_graph + # child (subprocess transport). A task only becomes visible/running once its + # first event arrives. + phases = ["vectors", "optimize", "graph"] renderer = IndexProgressRenderer(phases) progress = PipelineProgress(renderer=renderer) @@ -318,6 +323,8 @@ def work(progress: "PipelineProgress | None") -> int: quiet=bool(args.quiet), verbose=verbose, lance_project_root=None if args.quiet else cfg.source_root, + on_progress=progress.on_progress if progress is not None else None, + on_progress_console=progress.console if progress is not None else None, ) if coco.returncode != 0: _emit( @@ -378,6 +385,8 @@ def work(progress: "PipelineProgress | None") -> int: quiet=bool(args.quiet), verbose=bool(args.verbose), lance_project_root=None if args.quiet else cfg.source_root, + on_progress=progress.on_progress if progress is not None else None, + on_progress_console=progress.console if progress is not None else None, ) if coco.returncode != 0: _emit( @@ -455,7 +464,11 @@ def work(progress: "PipelineProgress | None") -> int: graph_only = bool(getattr(args, "graph_only", False)) if vectors_only: - coco = run_cocoindex_update(env, full_reprocess=True, quiet=bool(args.quiet), verbose=verbose) + coco = run_cocoindex_update( + env, full_reprocess=True, quiet=bool(args.quiet), verbose=verbose, + on_progress=progress.on_progress if progress is not None else None, + on_progress_console=progress.console if progress is not None else None, + ) if _is_cocoindex_preflight_blocker(coco): payload: dict[str, Any] = { "success": False, @@ -530,7 +543,14 @@ def work(progress: "PipelineProgress | None") -> int: import server # lazy: pulls sentence_transformers/torch/lancedb/kuzu - result = asyncio.run(server.run_refresh_pipeline(quiet=bool(args.quiet), verbose=verbose)) + result = asyncio.run( + server.run_refresh_pipeline( + quiet=bool(args.quiet), + verbose=verbose, + on_progress=progress.on_progress if progress is not None else None, + on_progress_console=progress.console if progress is not None else None, + ) + ) payload = result.model_dump() _emit_reprocess_outcome(payload) return _reprocess_exit_code(payload) diff --git a/java_codebase_rag/cli_format.py b/java_codebase_rag/cli_format.py index a8b7e80..73c5dee 100644 --- a/java_codebase_rag/cli_format.py +++ b/java_codebase_rag/cli_format.py @@ -1,10 +1,7 @@ """TTY-aware ANSI formatting for CLI stderr progress.""" from __future__ import annotations -import itertools import sys -import threading -import time _RESET = "\033[0m" _BOLD = "\033[1m" @@ -16,8 +13,6 @@ CHECK = "✓" CROSS = "✗" -_SPINNER_FRAMES = "⠋⠙⠹⠸⠼⠴⠦⠧⠇⠏" - _NOISE_CONTAINS: tuple[bytes, ...] = ( b"lance::", b"FutureWarning", @@ -88,33 +83,3 @@ def styled_check() -> str: def styled_cross() -> str: return red(CROSS) if stderr_is_tty() else CROSS - - -class Spinner: - """Braille spinner that overwrites the current stderr line until stopped.""" - - def __init__(self, label: str) -> None: - self._label = label - self._stop = threading.Event() - self._thread: threading.Thread | None = None - - def start(self) -> None: - self._thread = threading.Thread(target=self._run, name="spinner", daemon=True) - self._thread.start() - - def stop(self) -> None: - self._stop.set() - if self._thread is not None: - self._thread.join(timeout=2.0) - sys.stderr.buffer.write(b"\r\x1b[2K") - sys.stderr.buffer.flush() - - def _run(self) -> None: - frames = itertools.cycle(_SPINNER_FRAMES) - t0 = time.monotonic() - while not self._stop.wait(0.3): - elapsed = time.monotonic() - t0 - frame = next(frames) - line = f"\r{frame} {self._label} · {elapsed:.0f}s" - sys.stderr.buffer.write(line.encode()) - sys.stderr.buffer.flush() diff --git a/java_codebase_rag/cli_progress.py b/java_codebase_rag/cli_progress.py index 0419252..8d4e518 100644 --- a/java_codebase_rag/cli_progress.py +++ b/java_codebase_rag/cli_progress.py @@ -5,28 +5,10 @@ import sys from typing import Callable -from java_codebase_rag.cli_format import bold_cyan, is_noise_line, styled_check, styled_cross +from java_codebase_rag.cli_format import is_noise_line from java_codebase_rag.progress import ProgressEvent, make_relay -def emit_vectors_start() -> None: - print( - bold_cyan("[vectors]") + " running · cocoindex update", - file=sys.stderr, - flush=True, - ) - - -def emit_vectors_finish(*, elapsed_s: float, exit_code: int) -> None: - marker = styled_check() if exit_code == 0 else styled_cross() - print( - f"{marker} {bold_cyan('[vectors]')} finished · {elapsed_s:.2f}s" - + (f" (exit={exit_code})" if exit_code != 0 else ""), - file=sys.stderr, - flush=True, - ) - - class _AsyncLineFilter: """Buffers byte chunks and relays only non-noise lines to stderr (async drain path).""" diff --git a/java_codebase_rag/lance_optimize.py b/java_codebase_rag/lance_optimize.py index cb435dd..2914956 100644 --- a/java_codebase_rag/lance_optimize.py +++ b/java_codebase_rag/lance_optimize.py @@ -21,7 +21,9 @@ import asyncio import sys +import time from pathlib import Path +from typing import Callable # Single source of truth for the three Lance table names created by the flow. # Keep in sync with ``search_lancedb.TABLES`` (the values there mirror these). @@ -31,6 +33,33 @@ "yamlconfigindex_yaml_config", ) + +def _make_optimize_event( + *, + status: str, + elapsed_s: float | None = None, +): # type: ignore[no-untyped-def] + """Build a ``ProgressEvent(kind="optimize", …)`` lazily (progress is parent-side). + + ``lance_optimize`` runs in-process in the parent (called by + ``pipeline._maybe_run_serialized_optimize`` and + ``server.run_refresh_pipeline``); it routes progress to the renderer via the + in-process ``on_progress`` callback — NOT via stderr (which would corrupt + the Live region). The import is local so the flow (which imports + ``LANCE_TABLE_NAMES`` at definition time) never pays the ``rich`` cost. + """ + from java_codebase_rag.progress import ProgressEvent + + return ProgressEvent( + kind="optimize", + phase=None, + pass_=None, + done=None, + total=None, + status=status, # type: ignore[arg-type] + elapsed_s=elapsed_s, + ) + # Commit conflicts are transient; a handful of exponential-backoff retries is # enough because, post-flow, there are no concurrent writers — only successive # optimize/compaction passes within this single serialized call can still @@ -60,7 +89,12 @@ async def _list_table_names(db: object) -> set[str]: return set(await db.table_names()) -async def optimize_lance_tables(index_dir: Path, *, quiet: bool = False) -> dict[str, str]: +async def optimize_lance_tables( + index_dir: Path, + *, + quiet: bool = False, + on_progress: Callable | None = None, +) -> dict[str, str]: """Optimize all known Lance tables under *index_dir*, serially, with retry. Runs ``table.optimize()`` for each name in :data:`LANCE_TABLE_NAMES` that @@ -73,6 +107,13 @@ async def optimize_lance_tables(index_dir: Path, *, quiet: bool = False) -> dict index_dir: directory holding the Lance tables (the flow's LanceDB URI). quiet: when True, suppress the per-table success/skip info lines on stderr (errors are always logged). + on_progress: optional in-process progress callback (the parent's + renderer ``on_progress``). When given, emits + ``ProgressEvent(kind="optimize", status="running")`` on entry and a + terminal ``status="done"``/``"failed"`` event on exit (covers BOTH + call sites: ``pipeline._maybe_run_serialized_optimize`` and + ``server.run_refresh_pipeline``). In-process only — NEVER prints to + stderr (that would corrupt the Live region). Returns: Mapping of table name → status. Values are ``"ok"``, ``"skipped"`` @@ -82,67 +123,95 @@ async def optimize_lance_tables(index_dir: Path, *, quiet: bool = False) -> dict # not pay the lancedb import cost at flow-definition time. import lancedb + if on_progress is not None: + on_progress(_make_optimize_event(status="running")) + t0 = time.perf_counter() results: dict[str, str] = {} - db = await lancedb.connect_async(str(index_dir)) + failed = False try: + db = await lancedb.connect_async(str(index_dir)) try: - existing = await _list_table_names(db) - except Exception as exc: - print( - f"java-codebase-rag: optimize: failed to list tables in " - f"{index_dir}: {exc}", - file=sys.stderr, - ) - return {name: f"error: list failed: {exc}" for name in LANCE_TABLE_NAMES} - - for name in LANCE_TABLE_NAMES: - if name not in existing: - results[name] = "skipped" - if not quiet: - print( - f"java-codebase-rag: optimize: {name} absent, skipped", - file=sys.stderr, - ) - continue try: - table = await db.open_table(name) + existing = await _list_table_names(db) except Exception as exc: - results[name] = f"error: open failed: {exc}" print( - f"java-codebase-rag: optimize: {name} open failed: {exc}", + f"java-codebase-rag: optimize: failed to list tables in " + f"{index_dir}: {exc}", file=sys.stderr, ) - continue - - last_exc: BaseException | None = None - for attempt in range(_MAX_ATTEMPTS): + failed = True + return {name: f"error: list failed: {exc}" for name in LANCE_TABLE_NAMES} + + for name in LANCE_TABLE_NAMES: + if name not in existing: + results[name] = "skipped" + if not quiet: + print( + f"java-codebase-rag: optimize: {name} absent, skipped", + file=sys.stderr, + ) + continue try: - await table.optimize() - last_exc = None - break + table = await db.open_table(name) except Exception as exc: - last_exc = exc - if _is_retryable(exc) and attempt < _MAX_ATTEMPTS - 1: - await asyncio.sleep(_BASE_BACKOFF_S * (2**attempt)) - continue - # Non-retryable, or retries exhausted: stop the loop and - # surface below — do not swallow silently. - break - - if last_exc is None: - results[name] = "ok" - if not quiet: + results[name] = f"error: open failed: {exc}" + failed = True print( - f"java-codebase-rag: optimize: {name} ok", + f"java-codebase-rag: optimize: {name} open failed: {exc}", file=sys.stderr, ) - else: - results[name] = f"error: {last_exc}" - print( - f"java-codebase-rag: optimize: {name} failed: {last_exc}", - file=sys.stderr, - ) + continue + + last_exc: BaseException | None = None + for attempt in range(_MAX_ATTEMPTS): + try: + await table.optimize() + last_exc = None + break + except Exception as exc: + last_exc = exc + if _is_retryable(exc) and attempt < _MAX_ATTEMPTS - 1: + await asyncio.sleep(_BASE_BACKOFF_S * (2**attempt)) + continue + # Non-retryable, or retries exhausted: stop the loop and + # surface below — do not swallow silently. + break + + if last_exc is None: + results[name] = "ok" + if not quiet: + print( + f"java-codebase-rag: optimize: {name} ok", + file=sys.stderr, + ) + else: + results[name] = f"error: {last_exc}" + failed = True + print( + f"java-codebase-rag: optimize: {name} failed: {last_exc}", + file=sys.stderr, + ) + finally: + # ``AsyncConnection.close`` is a *sync* method in lancedb 0.30.x. + db.close() + return results + except Exception: + # An unexpected exception (e.g. ``connect_async`` raised, or a table- + # independent failure) must still flip the terminal event to failed so + # the renderer's task doesn't render a green check on a crash. Re-raise + # after marking — the caller (``_maybe_run_serialized_optimize`` / + # ``run_refresh_pipeline``) treats optimize failure as non-fatal and + # logs it, but the renderer must reflect the truth. + failed = True + raise finally: - # ``AsyncConnection.close`` is a *sync* method in lancedb 0.30.x. - db.close() - return results + # Always emit a terminal optimize event so the renderer's task never + # hangs at "running" — even on exception (the parent treats a failed + # optimize as non-fatal: the index is still searchable un-compacted). + if on_progress is not None: + on_progress( + _make_optimize_event( + status="failed" if failed else "done", + elapsed_s=time.perf_counter() - t0, + ) + ) diff --git a/java_codebase_rag/pipeline.py b/java_codebase_rag/pipeline.py index d5eb7a0..fe6376a 100644 --- a/java_codebase_rag/pipeline.py +++ b/java_codebase_rag/pipeline.py @@ -11,8 +11,7 @@ from pathlib import Path from typing import Callable -from java_codebase_rag.cli_format import Spinner, is_noise_line, stderr_is_tty -from java_codebase_rag.cli_progress import emit_vectors_finish, emit_vectors_start +from java_codebase_rag.cli_format import is_noise_line from java_codebase_rag.config import cocoindex_subprocess_env_defaults from java_codebase_rag.progress import ProgressEvent, ProgressRelay, make_relay @@ -129,6 +128,8 @@ def run_cocoindex_update( quiet: bool, verbose: bool = True, lance_project_root: Path | None = None, + on_progress: Callable[[ProgressEvent], None] | None = None, + on_progress_console: object | None = None, ) -> subprocess.CompletedProcess[str]: result = _run_cocoindex_update_impl( env, @@ -136,19 +137,25 @@ def run_cocoindex_update( quiet=quiet, verbose=verbose, lance_project_root=lance_project_root, + on_progress=on_progress, + on_progress_console=on_progress_console, ) # After cocoindex returns exit 0 there are no concurrent writers, so this # is the safe window to compact the Lance tables. The flow disabled its # in-flight background optimize (see java_index_flow_lancedb.py), making # this serialized pass the sole optimizer. Optimize failure does not flip # the cocoindex CompletedProcess (a successful index is still usable, just - # not compacted); the outcome is logged to stderr only. + # not compacted); the outcome is logged to stderr only. Thread the + # in-process on_progress so the optimize phase renders via the same + # renderer (the flow cannot emit it — it runs in the child). if result.returncode == 0: - _maybe_run_serialized_optimize(env, quiet=quiet) + _maybe_run_serialized_optimize(env, quiet=quiet, on_progress=on_progress) return result -def _maybe_run_serialized_optimize(env: dict[str, str], *, quiet: bool) -> None: +def _maybe_run_serialized_optimize( + env: dict[str, str], *, quiet: bool, on_progress: Callable | None = None +) -> None: """Resolve the index dir from *env* and run the serialized Lance optimize. The flow's lifespan reads ``JAVA_CODEBASE_RAG_INDEX_DIR`` (set by the CLI / @@ -167,7 +174,7 @@ def _maybe_run_serialized_optimize(env: dict[str, str], *, quiet: bool) -> None: try: from java_codebase_rag.lance_optimize import optimize_lance_tables - asyncio.run(optimize_lance_tables(Path(idx_raw), quiet=quiet)) + asyncio.run(optimize_lance_tables(Path(idx_raw), quiet=quiet, on_progress=on_progress)) except Exception as exc: # Never crash the CLI on an optimize failure — surface on stderr only. print(f"java-codebase-rag: optimize failed: {exc}", file=sys.stderr) @@ -180,9 +187,15 @@ def _run_cocoindex_update_impl( quiet: bool, verbose: bool = True, lance_project_root: Path | None = None, + on_progress: Callable[[ProgressEvent], None] | None = None, + on_progress_console: object | None = None, ) -> subprocess.CompletedProcess[str]: exe = cocoindex_bin() if not exe.is_file(): + # 127 pre-spawn stub: never mark the vectors task running — emit a + # terminal failed event so the renderer doesn't leave it hung. + if on_progress is not None: + on_progress(ProgressEvent(kind="vectors", phase=None, pass_=None, done=None, total=None, status="failed", elapsed_s=None)) return subprocess.CompletedProcess( args=[str(exe)], returncode=127, @@ -192,6 +205,8 @@ def _run_cocoindex_update_impl( bd = bundle_dir() flow = bd / "java_index_flow_lancedb.py" if not flow.is_file(): + if on_progress is not None: + on_progress(ProgressEvent(kind="vectors", phase=None, pass_=None, done=None, total=None, status="failed", elapsed_s=None)) return subprocess.CompletedProcess( args=[], returncode=126, @@ -218,17 +233,10 @@ def _run_cocoindex_update_impl( text=True, ) - emit_progress = lance_project_root is not None - use_spinner = emit_progress and stderr_is_tty() - if emit_progress and not use_spinner: - emit_vectors_start() - spinner: Spinner | None = None - if use_spinner: - spinner = Spinner("[vectors] running · cocoindex update") - spinner.start() t0 = time.perf_counter() code = -1 out_s, err_s = "", "" + proc: subprocess.Popen[bytes] | None = None try: proc = subprocess.Popen( cmd, @@ -238,12 +246,27 @@ def _run_cocoindex_update_impl( stderr=subprocess.PIPE, bufsize=0, ) - out_s, err_s, code = _popen_capturing_stderr(proc, verbose=verbose) + # Vectors task is marked running only AFTER Popen succeeds — the flow's + # per-file ticks + approximate total stream in from the child via the + # relay (parsed by ProgressRelay, routed to on_progress). + out_s, err_s, code = _popen_capturing_stderr( + proc, verbose=verbose, on_progress=on_progress, on_progress_console=on_progress_console + ) finally: - if spinner is not None: - spinner.stop() - if emit_progress: - emit_vectors_finish(elapsed_s=time.perf_counter() - t0, exit_code=code) + # The flow cannot emit the terminal vectors event (no "all files done" + # hook in cocoindex flows), so the PARENT emits it here based on the + # cocoindex exit code. This drives clamp-on-completion + the phase + # transition to Optimize. Emitted even on a spawn failure (code stays + # -1 → failed) so the renderer's task never hangs at running. + if on_progress is not None: + elapsed = time.perf_counter() - t0 + status = "done" if code == 0 else "failed" + on_progress( + ProgressEvent( + kind="vectors", phase=None, pass_=None, done=None, total=None, + status=status, elapsed_s=elapsed, + ) + ) return subprocess.CompletedProcess(args=cmd, returncode=code, stdout=out_s, stderr=err_s) diff --git a/java_index_flow_lancedb.py b/java_index_flow_lancedb.py index 6213616..db70c41 100644 --- a/java_index_flow_lancedb.py +++ b/java_index_flow_lancedb.py @@ -18,10 +18,13 @@ import inspect import os +import sys +import threading import uuid from collections.abc import AsyncIterator from contextlib import asynccontextmanager from dataclasses import dataclass +from fnmatch import fnmatch from pathlib import Path from typing import Annotated, Any @@ -84,6 +87,128 @@ _NUM_TXN_BEFORE_OPTIMIZE = 10**12 +# --- Vectors-phase progress emission (JCIRAG_PROGRESS kind=vectors) ----------- +# +# The flow runs in a CHILD cocoindex process; it prints structured progress to +# its stderr and the parent (pipeline._popen_capturing_stderr / +# cli_progress.accumulate_and_relay_subprocess_streams) parses it via +# ProgressRelay and feeds the renderer. The flow CANNOT know when all files are +# done (cocoindex offers no "all files done" hook in the flow), so it emits: +# - ONE ``total=N status=running`` line from ``app_main`` (approximate +# pre-walk: matcher includes + LayeredIgnore), and +# - per-file ``done=k status=running`` ticks (throttled every ~25 files) from +# ``process_*_file`` (shared atomic counter). +# The PARENT emits the terminal ``status=done``/``failed`` vectors event on +# cocoindex exit (drives clamp-on-completion + phase transition to Optimize). + +# Per-file tick cadence: bound stderr volume on huge trees without making the +# bar feel stale. Every 25th file (and the modulo boundary is enough — the +# parent clamps to total on the terminal event anyway). +_VECTORS_TICK_EVERY = 25 + +# Thread-safe counter: cocoindex may call process_*_file concurrently +# (mount_each parallelism is implementation-defined). A module-level lock guards +# both the counter and the emission so two threads never interleave a tick. +_vectors_done_lock = threading.Lock() +_vectors_done_count = 0 + + +def _emit_vectors_progress( + *, + done: int | None = None, + total: int | None = None, + status: str = "running", + elapsed_s: float | None = None, +) -> None: + """Emit one ``JCIRAG_PROGRESS kind=vectors …`` line to stderr (flushed). + + Field order is fixed (kind, done, total, status, elapsed_s) so the parser + and tests can pin substrings. Omitted fields are simply absent. + """ + fields = ["kind=vectors"] + if done is not None: + fields.append(f"done={done}") + if total is not None: + fields.append(f"total={total}") + fields.append(f"status={status}") + if elapsed_s is not None: + fields.append(f"elapsed_s={elapsed_s:.2f}") + print("JCIRAG_PROGRESS " + " ".join(fields), file=sys.stderr, flush=True) + + +def _tick_vectors_done() -> None: + """Increment the shared per-file counter and emit a throttled ``done=k`` tick. + + Called once per successfully-processed file (after the ignore / empty + early-returns). The tick is emitted every ``_VECTORS_TICK_EVERY`` files so + stderr volume stays bounded on huge trees; the parent clamps to total on + the terminal event, so the exact tick cadence is not load-bearing. + """ + global _vectors_done_count + with _vectors_done_lock: + _vectors_done_count += 1 + n = _vectors_done_count + if n % _VECTORS_TICK_EVERY != 0: + return + _emit_vectors_progress(done=n, status="running") + + +def _approximate_vectors_total(project_root: Path) -> int: + """Reproduce the matchers' include globs + LayeredIgnore for an approximate total. + + The flow applies two filtering layers: (1) ``PatternFilePathMatcher`` + excludes at walk time via ``LayeredIgnore.cocoindex_excluded_patterns()``, + then (2) ``LayeredIgnore.is_ignored()`` plus an early-return for empty / + undecodable files inside each ``process_*_file``. Files that early-return + never tick, so this pre-walk OVERSTATES the total by the ignored / empty + count. The parent clamps the bar to 100% on the terminal ``status=done`` + event, so the over-count cannot stall the bar. + + Mirrors the three ``localfs.walk_dir`` matchers in ``app_main``: + - ``**/*.java`` + - ``**/src/main/resources/db/migration/*.sql`` + - ``**/src/main/resources/application*.yml`` and ``.yaml`` + """ + ignore = LayeredIgnore(project_root) + excluded = ignore.cocoindex_excluded_patterns() + + def _excluded(rel_posix: str) -> bool: + return any(fnmatch(rel_posix, pat) for pat in excluded) + + total = 0 + for dirpath, dirnames, filenames in os.walk(project_root): + # Prune the same universal nuisance dirs as iter_java_source_files / + # cocoindex walk. (build-output pruning is matcher-dependent in the + # real walk; for an APPROXIMATE total this cheap prune is sufficient + # — the clamp absorbs any residual divergence.) + dirnames[:] = [ + d for d in dirnames if d not in (".git", ".hg", ".svn", "node_modules", ".venv", "venv") + ] + for fn in filenames: + full = Path(dirpath) / fn + try: + rel = full.resolve().relative_to(project_root).as_posix() + except ValueError: + continue + if _excluded(rel): + continue + # Java: **/*.java + if fn.endswith(".java"): + if not ignore.is_ignored(full)[0]: + total += 1 + continue + # SQL: **/src/main/resources/db/migration/*.sql + if fn.endswith(".sql") and "/db/migration/" in rel: + if not ignore.is_ignored(full)[0]: + total += 1 + continue + # YAML: **/src/main/resources/application*.yml / .yaml + if fn.endswith((".yml", ".yaml")) and "/application" in fn and "/src/main/resources/" in rel: + if not ignore.is_ignored(full)[0]: + total += 1 + return total + + @dataclass class JavaLanceChunk: id: str @@ -187,6 +312,8 @@ async def process_java_file( if not content.strip(): return + _tick_vectors_done() + language = detect_code_language(filename=file.file_path.path.name) or "text" cs, mn, ov = JAVA_CHUNK chunks = splitter.split( @@ -251,6 +378,8 @@ async def process_sql_file( if not content.strip(): return + _tick_vectors_done() + language = "sql" cs, mn, ov = SQL_CHUNK chunks = splitter.split( @@ -295,6 +424,8 @@ async def process_yaml_file( if not content.strip(): return + _tick_vectors_done() + ext = file.file_path.path.suffix.lower() language = "yaml" if ext in (".yml", ".yaml") else "text" cs, mn, ov = YAML_CHUNK @@ -362,6 +493,20 @@ async def app_main() -> None: project_root = coco.use_context(PROJECT_ROOT) _ignore = LayeredIgnore(project_root) _walk_excludes = _ignore.cocoindex_excluded_patterns() + # Emit ONE approximate total so the parent's renderer can show a determinate + # bar (clamps to 100% on the terminal vectors event the parent emits on + # cocoindex exit). Approximate — ignored / empty files over-state it; see + # ``_approximate_vectors_total``. ``--full-reprocess`` only: on incremental + # catch-up the @coco.fn(memo=True) cache skips unchanged files, so no total + # is knowable up front → the parent renders indeterminate from the absence. + try: + total = _approximate_vectors_total(project_root) + if total > 0: + _emit_vectors_progress(total=total, status="running") + except Exception: + # The pre-walk must never break indexing — a failure here just means + # the parent falls back to indeterminate. Swallow and continue. + pass java_files = localfs.walk_dir( PROJECT_ROOT, recursive=True, diff --git a/server.py b/server.py index c151f94..526c4aa 100644 --- a/server.py +++ b/server.py @@ -13,9 +13,8 @@ from index_common import SBERT_MODEL from java_codebase_rag.cli_progress import ( accumulate_and_relay_subprocess_streams, - emit_vectors_finish, - emit_vectors_start, ) +from java_codebase_rag.progress import ProgressEvent from java_codebase_rag._fdlimit import raise_fd_limit from java_codebase_rag.config import ( cocoindex_subprocess_env_defaults, @@ -270,10 +269,20 @@ def list_code_index_tables_payload() -> IndexInfoOutput: ) -async def run_refresh_pipeline(*, quiet: bool = False, verbose: bool = True) -> RefreshIndexOutput: +async def run_refresh_pipeline( + *, + quiet: bool = False, + verbose: bool = True, + on_progress=None, + on_progress_console: object | None = None, +) -> RefreshIndexOutput: root = _project_root() cocoindex_bin = Path(sys.executable).parent / "cocoindex" if not cocoindex_bin.is_file(): + # 127 pre-spawn: emit a terminal failed vectors event so the renderer's + # task doesn't hang at running (matches the sync pipeline path). + if on_progress is not None: + on_progress(ProgressEvent(kind="vectors", phase=None, pass_=None, done=None, total=None, status="failed", elapsed_s=None)) return RefreshIndexOutput( success=False, message=f"cocoindex not found next to Python: {cocoindex_bin}", @@ -286,6 +295,8 @@ async def run_refresh_pipeline(*, quiet: bool = False, verbose: bool = True) -> if fallback.is_file(): flow_path = fallback else: + if on_progress is not None: + on_progress(ProgressEvent(kind="vectors", phase=None, pass_=None, done=None, total=None, status="failed", elapsed_s=None)) return RefreshIndexOutput( success=False, message=f"java_index_flow_lancedb.py not found under {root} nor {bundle_dir}", @@ -308,13 +319,14 @@ async def run_refresh_pipeline(*, quiet: bool = False, verbose: bool = True) -> ) out_b, err_b = await proc.communicate() except Exception as exc: + if on_progress is not None: + on_progress(ProgressEvent(kind="vectors", phase=None, pass_=None, done=None, total=None, status="failed", elapsed_s=None)) return RefreshIndexOutput( success=False, message=f"spawn failed: {exc!s}", phases_run=[], ) else: - emit_vectors_start() t0 = time.perf_counter() code_c = -1 try: @@ -329,16 +341,30 @@ async def run_refresh_pipeline(*, quiet: bool = False, verbose: bool = True) -> stdout=asyncio.subprocess.PIPE, stderr=asyncio.subprocess.PIPE, ) - out_b, err_b = await accumulate_and_relay_subprocess_streams(proc, relay=True, verbose=verbose) + # The vectors task is fed by the child's per-file ticks + the + # approximate total line, parsed by the ProgressRelay inside the + # async drain and routed to on_progress. + out_b, err_b = await accumulate_and_relay_subprocess_streams( + proc, relay=True, verbose=verbose, + on_progress=on_progress, on_progress_console=on_progress_console, + ) code_c = proc.returncode if proc.returncode is not None else -1 except Exception as exc: + if on_progress is not None: + on_progress(ProgressEvent(kind="vectors", phase=None, pass_=None, done=None, total=None, status="failed", elapsed_s=None)) return RefreshIndexOutput( success=False, message=f"spawn failed: {exc!s}", phases_run=[], ) finally: - emit_vectors_finish(elapsed_s=time.perf_counter() - t0, exit_code=code_c) + # The parent emits the terminal vectors event (the flow can't — no + # "all files done" hook). Drives clamp-on-completion + phase + # transition to Optimize. + if on_progress is not None: + elapsed = time.perf_counter() - t0 + status = "done" if code_c == 0 else "failed" + on_progress(ProgressEvent(kind="vectors", phase=None, pass_=None, done=None, total=None, status=status, elapsed_s=elapsed)) assert proc is not None out = out_b.decode(errors="replace") err = err_b.decode(errors="replace") @@ -366,7 +392,7 @@ async def run_refresh_pipeline(*, quiet: bool = False, verbose: bool = True) -> idx_dir = Path(idx_raw) else: idx_dir = (root / ".java-codebase-rag").resolve() - await optimize_lance_tables(idx_dir, quiet=quiet) + await optimize_lance_tables(idx_dir, quiet=quiet, on_progress=on_progress) except Exception as exc: optimize_error = f"lance optimize failed: {exc}" print(f"java-codebase-rag: {optimize_error}", file=sys.stderr) @@ -394,7 +420,10 @@ async def run_refresh_pipeline(*, quiet: bool = False, verbose: bool = True) -> if quiet: gout_b, gerr_b = await gproc.communicate() else: - gout_b, gerr_b = await accumulate_and_relay_subprocess_streams(gproc, relay=True, verbose=verbose) + gout_b, gerr_b = await accumulate_and_relay_subprocess_streams( + gproc, relay=True, verbose=verbose, + on_progress=on_progress, on_progress_console=on_progress_console, + ) graph_code = gproc.returncode graph_out = gout_b.decode(errors="replace") graph_err = gerr_b.decode(errors="replace") diff --git a/tests/test_cli_progress_stdout_invariant.py b/tests/test_cli_progress_stdout_invariant.py index 739f879..81fd670 100644 --- a/tests/test_cli_progress_stdout_invariant.py +++ b/tests/test_cli_progress_stdout_invariant.py @@ -160,7 +160,7 @@ def test_cli_lifecycle_stdout_invariant_reprocess( baseline = (_FIXTURE_DIR / "reprocess_quiet_success.stdout.txt").read_text(encoding="utf-8") - async def fake_refresh(*, quiet: bool = False, verbose: bool = True) -> RefreshIndexOutput: + async def fake_refresh(*, quiet: bool = False, verbose: bool = True, on_progress=None, on_progress_console=None) -> RefreshIndexOutput: _ = quiet _ = verbose return RefreshIndexOutput( diff --git a/tests/test_java_codebase_rag_cli.py b/tests/test_java_codebase_rag_cli.py index 3b35324..aa8f3b9 100644 --- a/tests/test_java_codebase_rag_cli.py +++ b/tests/test_java_codebase_rag_cli.py @@ -579,7 +579,7 @@ def test_reprocess_graph_only_then_increment_graph_is_noop( hash_file.write_text(json.dumps(data), encoding="utf-8") # Stub cocoindex so increment exercises ONLY its graph stage. - def _noop_coco(env, *, full_reprocess, quiet, verbose=True, lance_project_root=None): + def _noop_coco(env, *, full_reprocess, quiet, verbose=True, lance_project_root=None, on_progress=None, on_progress_console=None): return subprocess.CompletedProcess(args=[], returncode=0, stdout="", stderr="") monkeypatch.setattr(cli_mod, "run_cocoindex_update", _noop_coco) @@ -1017,7 +1017,7 @@ def test_reprocess_no_flag_cocoindex_failure_records_vectors_only( monkeypatch.setenv("JAVA_CODEBASE_RAG_INDEX_DIR", str(idx)) monkeypatch.setenv("JAVA_CODEBASE_RAG_SOURCE_ROOT", str(tmp_path)) - async def fake_refresh(*, quiet: bool = False, verbose: bool = True) -> server_mod.RefreshIndexOutput: + async def fake_refresh(*, quiet: bool = False, verbose: bool = True, on_progress=None, on_progress_console=None) -> server_mod.RefreshIndexOutput: return server_mod.RefreshIndexOutput( success=False, exit_code=1, @@ -1253,7 +1253,7 @@ def _patch_pipeline_for_graph_progress(monkeypatch: pytest.MonkeyPatch, *, emit_ from java_codebase_rag import cli as _cli from java_codebase_rag import pipeline as _pipeline - def _fake_cocoindex_update(env, *, full_reprocess, quiet, verbose=True, lance_project_root=None): + def _fake_cocoindex_update(env, *, full_reprocess, quiet, verbose=True, lance_project_root=None, on_progress=None, on_progress_console=None): return _make_stub_completed(returncode=0) def _fake_run_build_ast_graph(*, source_root, ladybug_path, verbose, quiet=False, env=None, on_progress=None, on_progress_console=None): diff --git a/tests/test_vectors_progress.py b/tests/test_vectors_progress.py new file mode 100644 index 0000000..c091c7f --- /dev/null +++ b/tests/test_vectors_progress.py @@ -0,0 +1,330 @@ +"""PR-3: vectors-phase + optimize-phase index progress. + +Two tests are HEAVY-gated (``JAVA_CODEBASE_RAG_RUN_HEAVY=1``) because they run +a real ``cocoindex update`` against the bank-chat-system corpus (embedding model +cached). The remaining tests are LIGHT: they exercise the renderer against a +synthetic event stream or patch the pipeline helpers so no cocoindex/torch loads. +""" +from __future__ import annotations + +import io +import os +import re +import subprocess +import sys +from pathlib import Path + +import pytest +from rich.console import Console + +from java_codebase_rag.progress import IndexProgressRenderer, ProgressEvent + +_PREFIX = "JCIRAG_PROGRESS" + +HEAVY = os.environ.get("JAVA_CODEBASE_RAG_RUN_HEAVY", "").strip().lower() in ("1", "true", "yes") + + +# --------------------------------------------------------------------------- +# Light: renderer clamp-on-completion + indeterminate (tests 2, 3, 4) +# --------------------------------------------------------------------------- + + +def _renderer(*, terminal: bool = True) -> IndexProgressRenderer: + """Build a renderer over a buffer; force_terminal=True exercises the Live path.""" + buf = io.StringIO() + console = Console(file=buf, force_terminal=terminal, width=120, color_system=None) + return IndexProgressRenderer(["vectors", "optimize", "graph"], console=console) + + +def test_vectors_progress_clamps_on_completion() -> None: + """total=100, done=80, then parent status=done -> completed clamps to 100.""" + r = _renderer() + r.start() + try: + r.apply(ProgressEvent(kind="vectors", phase=None, pass_=None, done=80, total=100, status="running", elapsed_s=None)) + assert r._progress.tasks[r._task_ids["vectors"]].completed == 80 + # Parent emits the terminal event (the flow cannot — no "all files done" + # hook). done=None here; the clamp must still snap completed to total. + r.apply(ProgressEvent(kind="vectors", phase=None, pass_=None, done=None, total=100, status="done", elapsed_s=1.0)) + task = r._progress.tasks[r._task_ids["vectors"]] + assert task.completed == 100 + assert task.finished + finally: + r.stop() + + +def test_vectors_progress_approximate_total_overstates_then_clamps() -> None: + """Approximate pre-walk overstates total (100) but done only reaches 95; the + parent's status=done still clamps to 100 (no 95% stall).""" + r = _renderer() + r.start() + try: + r.apply(ProgressEvent(kind="vectors", phase=None, pass_=None, done=None, total=100, status="running", elapsed_s=None)) + r.apply(ProgressEvent(kind="vectors", phase=None, pass_=None, done=95, total=100, status="running", elapsed_s=None)) + assert r._progress.tasks[r._task_ids["vectors"]].completed == 95 + r.apply(ProgressEvent(kind="vectors", phase=None, pass_=None, done=95, total=100, status="done", elapsed_s=42.1)) + assert r._progress.tasks[r._task_ids["vectors"]].completed == 100 + assert r._progress.tasks[r._task_ids["vectors"]].finished + finally: + r.stop() + + +def test_vectors_incremental_renders_indeterminate() -> None: + """No total event (incremental catch-up) -> task stays indeterminate (total None).""" + r = _renderer() + r.start() + try: + # Only a done tick, no total — mirrors incremental catch-up where the + # memo cache skips unchanged files and no total is knowable up front. + r.apply(ProgressEvent(kind="vectors", phase=None, pass_=None, done=3, total=None, status="running", elapsed_s=None)) + task = r._progress.tasks[r._task_ids["vectors"]] + assert task.total is None + finally: + r.stop() + + +# --------------------------------------------------------------------------- +# Light: CLI-level vectors/optimize progress (tests 6, 7) +# --------------------------------------------------------------------------- + + +def _make_stub_completed(*, returncode: int = 0, stderr: str = "") -> "subprocess.CompletedProcess[str]": + return subprocess.CompletedProcess(args=["stub"], returncode=returncode, stdout="", stderr=stderr) + + +def _patch_pipeline_for_vectors_progress(monkeypatch: pytest.MonkeyPatch, *, emit_vectors: bool) -> None: + """Patch cocoindex + graph helpers so init/increment run without heavy deps. + + When ``emit_vectors`` is True the patched cocoindex helper invokes the + caller's ``on_progress`` with a synthetic ``kind=vectors`` event — simulating + what the real subprocess drain would feed the renderer in default mode. + """ + from java_codebase_rag import cli as _cli + from java_codebase_rag import pipeline as _pipeline + + def _fake_cocoindex_update(env, *, full_reprocess, quiet, verbose=True, lance_project_root=None, on_progress=None, on_progress_console=None): + if emit_vectors and on_progress is not None: + on_progress( + ProgressEvent(kind="vectors", phase=None, pass_=None, done=10, total=130, status="running", elapsed_s=None) + ) + return _make_stub_completed(returncode=0) + + def _fake_run_build_ast_graph(*, source_root, ladybug_path, verbose, quiet=False, env=None, on_progress=None, on_progress_console=None): + return _make_stub_completed(returncode=0) + + def _fake_run_incremental_graph(*, source_root, ladybug_path, verbose, quiet=False, env=None, on_progress=None, on_progress_console=None): + return _make_stub_completed(returncode=0) + + monkeypatch.setattr(_cli, "run_cocoindex_update", _fake_cocoindex_update) + monkeypatch.setattr(_cli, "run_build_ast_graph", _fake_run_build_ast_graph) + monkeypatch.setattr(_cli, "run_incremental_graph", _fake_run_incremental_graph) + monkeypatch.setattr(_pipeline, "run_cocoindex_update", _fake_cocoindex_update) + monkeypatch.setattr(_pipeline, "run_build_ast_graph", _fake_run_build_ast_graph) + monkeypatch.setattr(_pipeline, "run_incremental_graph", _fake_run_incremental_graph) + + +def test_cli_init_vectors_phase_progress_on_stderr( + corpus_root: Path, tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """In default mode a vectors-phase progress event is parsed and rendered to + stderr; the raw ``JCIRAG_PROGRESS`` line is NOT echoed verbatim.""" + from java_codebase_rag import cli as cli_mod + + idx = tmp_path / "idx_vectors_prog" + idx.mkdir() + monkeypatch.setenv("JAVA_CODEBASE_RAG_INDEX_DIR", str(idx)) + monkeypatch.setenv("JAVA_CODEBASE_RAG_SOURCE_ROOT", str(corpus_root)) + _patch_pipeline_for_vectors_progress(monkeypatch, emit_vectors=True) + buf = io.StringIO() + import contextlib + + with contextlib.redirect_stderr(buf): + rc = cli_mod.main( + ["init", "--source-root", str(corpus_root), "--index-dir", str(idx)] + ) + assert rc == 0 + err = buf.getvalue() + # The raw structured line is consumed by the parser, never raw-relayed. + assert "JCIRAG_PROGRESS kind=vectors" not in err + # But vectors-phase progress IS rendered (non-TTY concise fallback prints a + # "vectors ..." line). The synthetic event had done=10, total=130. + assert "vectors" in err.lower() + + +def test_cli_reprocess_optimize_phase_progress( + corpus_root: Path, tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """The optimize phase renders when optimize_lance_tables emits kind=optimize. + + Patches server.run_refresh_pipeline to drive the renderer's on_progress with + a synthetic optimize event (mirrors the in-process emission). Asserts the + phase renders and the raw line is not echoed.""" + import contextlib + + from java_codebase_rag import cli as cli_mod + import server + + idx = tmp_path / "idx_optimize_prog" + idx.mkdir() + monkeypatch.setenv("JAVA_CODEBASE_RAG_INDEX_DIR", str(idx)) + monkeypatch.setenv("JAVA_CODEBASE_RAG_SOURCE_ROOT", str(corpus_root)) + + async def _fake_refresh(*, quiet=False, verbose=True, on_progress=None, on_progress_console=None): + # Emit a synthetic optimize event (mirrors lance_optimize's in-process + # emission) so the renderer's optimize task is exercised. + if on_progress is not None: + on_progress(ProgressEvent(kind="optimize", phase=None, pass_=None, done=None, total=None, status="running", elapsed_s=None)) + on_progress(ProgressEvent(kind="optimize", phase=None, pass_=None, done=None, total=None, status="done", elapsed_s=1.2)) + from server import RefreshIndexOutput + + return RefreshIndexOutput(success=True, message=None, phases_run=["vectors", "graph"]) + + monkeypatch.setattr(server, "run_refresh_pipeline", _fake_refresh) + + buf = io.StringIO() + with contextlib.redirect_stderr(buf): + rc = cli_mod.main( + ["reprocess", "--source-root", str(corpus_root), "--index-dir", str(idx)] + ) + assert rc == 0 + err = buf.getvalue() + # The raw structured line is consumed by the parser, never raw-relayed. + assert "JCIRAG_PROGRESS kind=optimize" not in err + # The optimize phase rendered (non-TTY concise fallback prints an + # "optimize done ..." line for the terminal event). + assert "optimize" in err.lower() + + +# --------------------------------------------------------------------------- +# Light: retirement guard (test 8) +# --------------------------------------------------------------------------- + + +def test_spinner_removed_and_emit_vectors_helpers_removed() -> None: + """Spinner and emit_vectors_start/_finish are no longer importable; no + remaining references anywhere in the production tree.""" + from java_codebase_rag import cli_format, cli_progress + + # The retired symbols must not be importable. + assert not hasattr(cli_format, "Spinner"), "Spinner should have been removed from cli_format" + assert not hasattr(cli_progress, "emit_vectors_start"), "emit_vectors_start should have been removed" + assert not hasattr(cli_progress, "emit_vectors_finish"), "emit_vectors_finish should have been removed" + # And no remaining references anywhere in the production tree. + repo_root = Path(__file__).resolve().parent.parent + offenders: list[str] = [] + for py in (repo_root / "java_codebase_rag").rglob("*.py"): + text = py.read_text(encoding="utf-8") + # Word-boundary match for the retired Spinner class (not rich's SpinnerColumn). + if re.search(r"\bSpinner\b", text): + offenders.append(str(py)) + server_py = repo_root / "server.py" + if server_py.is_file(): + text = server_py.read_text(encoding="utf-8") + if re.search(r"\bSpinner\b", text) or "emit_vectors_start" in text or "emit_vectors_finish" in text: + offenders.append(str(server_py)) + assert not offenders, f"retired symbols still referenced in: {offenders}" + + +# --------------------------------------------------------------------------- +# Heavy: real cocoindex flow emission + pre-walk divergence (tests 1, 5) +# --------------------------------------------------------------------------- + + +def _cocoindex_bin() -> Path: + return Path(sys.executable).parent / "cocoindex" + + +def _require_cocoindex_runtime_deps() -> None: + try: + import tree_sitter_java # noqa: F401 + except ImportError as exc: + pytest.skip(f"heavy e2e needs project deps: {exc}") + + +pytestmark_heavy = pytest.mark.skipif( + not HEAVY, + reason="set JAVA_CODEBASE_RAG_RUN_HEAVY=1 to run the cocoindex vectors-flow test", +) + + +def _run_cocoindex_update(corpus_root: Path, index_dir: Path) -> subprocess.CompletedProcess: + """Run a real ``cocoindex update --full-reprocess`` and return the result.""" + _require_cocoindex_runtime_deps() + cocoindex_bin = _cocoindex_bin() + if not cocoindex_bin.is_file(): + pytest.skip(f"cocoindex not installed in venv: {cocoindex_bin}") + bundle_dir = Path(__file__).resolve().parent.parent + flow = (bundle_dir / "java_index_flow_lancedb.py").resolve() + start = Path(corpus_root).resolve() + relp = os.path.relpath(str(flow), start=str(start)) + relp = Path(relp).as_posix() + app_spec = f"{relp}:JavaCodeIndexLance" + env = { + **os.environ, + "JAVA_CODEBASE_RAG_INDEX_DIR": str(index_dir.resolve()), + "JAVA_CODEBASE_RAG_SOURCE_ROOT": str(Path(corpus_root).resolve()), + } + return subprocess.run( + [str(cocoindex_bin), "update", app_spec, "--full-reprocess", "-f"], + cwd=str(corpus_root), + env=env, + capture_output=True, + text=True, + timeout=900, + ) + + +@pytestmark_heavy +def test_flow_emits_vectors_progress_per_file(corpus_root: Path, tmp_path: Path) -> None: + """A real ``cocoindex update`` emits ``JCIRAG_PROGRESS kind=vectors`` lines + in captured stderr: the one-shot approximate ``total=`` line from app_main + plus per-file ``done=`` ticks (the gating spike, promoted to a regression test).""" + index_dir = tmp_path / ".java-codebase-rag" + index_dir.mkdir(parents=True) + proc = _run_cocoindex_update(corpus_root, index_dir) + assert proc.returncode == 0, f"cocoindex failed: stdout={proc.stdout}\nstderr={proc.stderr}" + lines = [ln for ln in proc.stderr.splitlines() if "JCIRAG_PROGRESS kind=vectors" in ln] + assert lines, f"expected vectors progress lines, got stderr:\n{proc.stderr}" + # The one-shot approximate total line from app_main. + totals = [ln for ln in lines if "total=" in ln and "done=" not in ln] + assert totals, f"expected a one-shot total line; lines: {lines!r}" + # Per-file done ticks (throttled every ~25 files). + ticks = [ln for ln in lines if "done=" in ln] + assert ticks, f"expected per-file done ticks; lines: {lines!r}" + # Done ticks are monotonic non-decreasing. + done_vals = [int(m.group(1)) for ln in ticks if (m := re.search(r"done=(\d+)", ln))] + assert done_vals, f"could not parse done values from: {ticks!r}" + assert done_vals == sorted(done_vals), f"done ticks must be monotonic: {done_vals}" + + +@pytestmark_heavy +def test_pre_walk_total_divergence_bounded(corpus_root: Path, tmp_path: Path) -> None: + """On the fixture, the approximate pre-walk total vs. the actual done count + differ only by the ignored / empty / undecodable count (the spike measured 0). + + Asserts the gap is small/bounded — the parent clamps to total on completion, + so a bounded over-count is the documented, accepted behaviour.""" + index_dir = tmp_path / ".java-codebase-rag" + index_dir.mkdir(parents=True) + proc = _run_cocoindex_update(corpus_root, index_dir) + assert proc.returncode == 0, f"cocoindex failed: stdout={proc.stdout}\nstderr={proc.stderr}" + lines = [ln for ln in proc.stderr.splitlines() if "JCIRAG_PROGRESS kind=vectors" in ln] + # The one-shot approximate total. + total_lines = [ln for ln in lines if "total=" in ln and "done=" not in ln] + assert total_lines, f"expected a total line; lines: {lines!r}" + total_match = re.search(r"total=(\d+)", total_lines[0]) + assert total_match, f"could not parse total from: {total_lines[0]!r}" + pre_walk_total = int(total_match.group(1)) + # The actual done count is the highest done value emitted (ticks are + # monotonic non-decreasing; the last tick is the final count). + ticks = [ln for ln in lines if "done=" in ln] + done_vals = [int(m.group(1)) for ln in ticks if (m := re.search(r"done=(\d+)", ln))] + actual_done = max(done_vals) if done_vals else 0 + # The pre-walk over-states by the ignored / empty / undecodable count. The + # spike measured 0 on this fixture; the bar clamps regardless. Assert the + # gap is bounded (>= 0 and not absurdly large). + gap = pre_walk_total - actual_done + assert gap >= 0, f"pre-walk total {pre_walk_total} < actual done {actual_done} (under-count)" + # Bounded: the over-count is at most the pre-walk total itself (paranoid + # upper bound — in practice it is the ignored/empty count, near 0). + assert gap <= pre_walk_total, f"gap {gap} exceeds total {pre_walk_total}" From 6f23da4bd492728608b61a5f10b3b1506c3aa00a Mon Sep 17 00:00:00 2001 From: Dmitry Teryaev Date: Mon, 15 Jun 2026 03:41:47 +0300 Subject: [PATCH 09/11] fix PR-3: count application YAML in pre-walk; tighten divergence test _approximate_vectors_total's YAML predicate ("/application" in fn) was always False for a bare filename, so the pre-walk under-counted application YAML and the divergence went negative (gap=-4). Use fn.startswith("application"). Move the vectors tick emission under the lock to match its comment. Drop a dead type-ignore. Tighten the divergence test to assert gap==0 on the fixture. Co-Authored-By: Claude --- java_codebase_rag/lance_optimize.py | 13 ++++--- java_index_flow_lancedb.py | 14 ++++++-- tests/test_vectors_progress.py | 53 ++++++++++++++++++++--------- 3 files changed, 57 insertions(+), 23 deletions(-) diff --git a/java_codebase_rag/lance_optimize.py b/java_codebase_rag/lance_optimize.py index 2914956..b16bebd 100644 --- a/java_codebase_rag/lance_optimize.py +++ b/java_codebase_rag/lance_optimize.py @@ -23,7 +23,12 @@ import sys import time from pathlib import Path -from typing import Callable +from typing import Callable, Literal + +# Mirrors ``ProgressStatus`` in ``progress.py``; kept local (rather than imported) +# so this module never pays the ``rich`` cost at import time — see +# ``_make_optimize_event``. +_OptimizeStatus = Literal["running", "done", "failed"] # Single source of truth for the three Lance table names created by the flow. # Keep in sync with ``search_lancedb.TABLES`` (the values there mirror these). @@ -36,9 +41,9 @@ def _make_optimize_event( *, - status: str, + status: _OptimizeStatus, elapsed_s: float | None = None, -): # type: ignore[no-untyped-def] +): """Build a ``ProgressEvent(kind="optimize", …)`` lazily (progress is parent-side). ``lance_optimize`` runs in-process in the parent (called by @@ -56,7 +61,7 @@ def _make_optimize_event( pass_=None, done=None, total=None, - status=status, # type: ignore[arg-type] + status=status, elapsed_s=elapsed_s, ) diff --git a/java_index_flow_lancedb.py b/java_index_flow_lancedb.py index db70c41..10425a2 100644 --- a/java_index_flow_lancedb.py +++ b/java_index_flow_lancedb.py @@ -150,7 +150,11 @@ def _tick_vectors_done() -> None: n = _vectors_done_count if n % _VECTORS_TICK_EVERY != 0: return - _emit_vectors_progress(done=n, status="running") + # Emit under the lock: the docstring above promises the lock guards both + # the counter AND the emission, so two concurrent ticks can't emit their + # ``done=N`` lines out of order. Contention is negligible (fires every + # ~25 files). + _emit_vectors_progress(done=n, status="running") def _approximate_vectors_total(project_root: Path) -> int: @@ -203,7 +207,13 @@ def _excluded(rel_posix: str) -> bool: total += 1 continue # YAML: **/src/main/resources/application*.yml / .yaml - if fn.endswith((".yml", ".yaml")) and "/application" in fn and "/src/main/resources/" in rel: + # NOTE: ``fn`` is the bare filename (e.g. ``application-cloud.yml``), so + # the prefix predicate must be ``fn.startswith("application")`` — + # ``"/application" in fn`` was always False (no leading slash in a bare + # name) and under-counted every application YAML, driving the pre-walk + # total below the actual done count. The ``rel``-based + # ``"/src/main/resources/"`` gate stays (full path component). + if fn.endswith((".yml", ".yaml")) and fn.startswith("application") and "/src/main/resources/" in rel: if not ignore.is_ignored(full)[0]: total += 1 return total diff --git a/tests/test_vectors_progress.py b/tests/test_vectors_progress.py index c091c7f..8c9a754 100644 --- a/tests/test_vectors_progress.py +++ b/tests/test_vectors_progress.py @@ -299,11 +299,15 @@ def test_flow_emits_vectors_progress_per_file(corpus_root: Path, tmp_path: Path) @pytestmark_heavy def test_pre_walk_total_divergence_bounded(corpus_root: Path, tmp_path: Path) -> None: - """On the fixture, the approximate pre-walk total vs. the actual done count - differ only by the ignored / empty / undecodable count (the spike measured 0). - - Asserts the gap is small/bounded — the parent clamps to total on completion, - so a bounded over-count is the documented, accepted behaviour.""" + """On the fixture, the approximate pre-walk total exactly equals the number of + files actually processed (the spike measured gap == 0). + + The TRUE processed count is read from the LanceDB tables (distinct + ``filename`` values across the three tables) rather than the throttled + ``done=k`` tick stream: ticks fire only every 25th file, so ``max(done)`` + lands on a multiple of 25 and would mask the real divergence. The accepted + over-count on larger trees is the ignored / empty / undecodable file count + (the renderer clamps to total on completion regardless).""" index_dir = tmp_path / ".java-codebase-rag" index_dir.mkdir(parents=True) proc = _run_cocoindex_update(corpus_root, index_dir) @@ -315,16 +319,31 @@ def test_pre_walk_total_divergence_bounded(corpus_root: Path, tmp_path: Path) -> total_match = re.search(r"total=(\d+)", total_lines[0]) assert total_match, f"could not parse total from: {total_lines[0]!r}" pre_walk_total = int(total_match.group(1)) - # The actual done count is the highest done value emitted (ticks are - # monotonic non-decreasing; the last tick is the final count). - ticks = [ln for ln in lines if "done=" in ln] - done_vals = [int(m.group(1)) for ln in ticks if (m := re.search(r"done=(\d+)", ln))] - actual_done = max(done_vals) if done_vals else 0 - # The pre-walk over-states by the ignored / empty / undecodable count. The - # spike measured 0 on this fixture; the bar clamps regardless. Assert the - # gap is bounded (>= 0 and not absurdly large). + # The tick stream is throttled (every 25th file), so it cannot yield the + # true processed count. Read the ground truth from the LanceDB tables: + # distinct filename values across the three tables the flow populated. + import lancedb + + db = lancedb.connect(str(index_dir.resolve())) + actual_done = 0 + for tname in ("javacodeindex_java_code", "sqlschemaindex_sql_schema", "yamlconfigindex_yaml_config"): + try: + tbl = db.open_table(tname) + except Exception as exc: # pragma: no cover - table missing only on a broken flow + raise AssertionError(f"Lance table {tname} missing after flow: {exc}") from exc + rows = tbl.search().select(["filename"]).limit(1_000_000).to_list() + actual_done += len({r["filename"] for r in rows if r.get("filename") is not None}) + # On this fixture the pre-walk matches exactly (gap == 0): all counted + # files are non-empty / decodable / not-ignored, and the YAML predicate + # was fixed to include application*.yml. The accepted over-count on larger + # trees is the ignored / empty file count (the renderer clamps regardless). gap = pre_walk_total - actual_done - assert gap >= 0, f"pre-walk total {pre_walk_total} < actual done {actual_done} (under-count)" - # Bounded: the over-count is at most the pre-walk total itself (paranoid - # upper bound — in practice it is the ignored/empty count, near 0). - assert gap <= pre_walk_total, f"gap {gap} exceeds total {pre_walk_total}" + assert gap >= 0, ( + f"pre-walk total {pre_walk_total} < actual done {actual_done} (under-count: " + f"a matcher predicate is dropping files the flow still processes)" + ) + assert gap == 0, ( + f"pre-walk over-count on fixture: pre_walk_total={pre_walk_total} " + f"actual_done={actual_done} gap={gap} (expected 0; the over-count is the " + f"ignored/empty/undecodable file count — the renderer clamps regardless)" + ) From d2d70619fdb0861fcf9a2633b3fa497eaf1c93f6 Mon Sep 17 00:00:00 2001 From: Dmitry Teryaev Date: Mon, 15 Jun 2026 04:42:21 +0300 Subject: [PATCH 10/11] align install/update index progress; wire --quiet/--verbose install/update render the unified Vectors->Optimize->Graph progress on stderr during their indexing sub-step (no longer stdout chatter / silent quiet=True); wizard conversational stdout is preserved. _cmd_update forwards --quiet/--verbose (both ignored today); _cmd_install forwards --verbose. CLI docs + README updated. Co-Authored-By: Claude --- README.md | 2 + docs/JAVA-CODEBASE-RAG-CLI.md | 42 +++++- java_codebase_rag/cli.py | 14 +- java_codebase_rag/installer.py | 210 ++++++++++++++++++++------ java_codebase_rag/progress.py | 33 +++++ tests/test_installer.py | 221 +++++++++++++++++++++++++++- tests/test_java_codebase_rag_cli.py | 93 ++++++++++++ 7 files changed, 559 insertions(+), 56 deletions(-) diff --git a/README.md b/README.md index 3707696..1f2b68a 100644 --- a/README.md +++ b/README.md @@ -65,6 +65,8 @@ java-codebase-rag install --non-interactive --agent claude-code After `pip install --upgrade java-codebase-rag`, run `java-codebase-rag update` to refresh shipped artifacts and catch up the index (Lance + graph). +All indexing lifecycle commands (`init`, `increment`, `reprocess`, `install`, `update`) show a unified `Vectors → Optimize → Graph` progress bar on stderr during the index build (powered by `rich`); pass `--quiet` to suppress it. + ### Manual registration If you prefer manual configuration, see [`docs/JAVA-CODEBASE-RAG-CLI.md`](./docs/JAVA-CODEBASE-RAG-CLI.md) for the full CLI reference. diff --git a/docs/JAVA-CODEBASE-RAG-CLI.md b/docs/JAVA-CODEBASE-RAG-CLI.md index c761adc..06a6dd3 100644 --- a/docs/JAVA-CODEBASE-RAG-CLI.md +++ b/docs/JAVA-CODEBASE-RAG-CLI.md @@ -43,6 +43,8 @@ java-codebase-rag install --scope user - `--agent {claude-code,qwen-code,gigacode}` — Agent host to configure (can be passed multiple times). - `--scope {project,user}` — Installation scope (default: `project`). Project scope writes to `./` in the project repo; user scope writes to `~/./` (globally available). - `--model MODEL` — Embedding model path or `auto` (default: `auto`, downloads `sentence-transformers/all-MiniLM-L6-v2` on first run). +- `--quiet` / `-q` — Suppress the indexing progress stream on stderr (wizard prompts unchanged). +- `--verbose` / `-v` — Raw-relay subprocess output during the indexing sub-step (no progress bar). **Exit codes:** - `0` — Success (all stages completed). @@ -55,7 +57,7 @@ java-codebase-rag install --scope user 3. Agent host selection — Claude Code, Qwen Code, GigaCode (multi-select). 4. Install scope — project or user. 5. MCP entrypoint resolution + artifact deployment — config, skill, agent files. -6. Index + finish — YAML generation, `.gitignore` update, `init`. +6. Index + finish — YAML generation, `.gitignore` update, `init`. Stage 6's indexing sub-step renders the unified `Vectors → Optimize → Graph` progress on **stderr** (see [Indexing progress](#indexing-progress-stderr)); the wizard's conversational stdout is unchanged. **Re-running `install`:** If `.java-codebase-rag.yml` exists, the installer shows current values and offers "Update" (pre-filled) or "Start fresh". Existing MCP entries are updated in-place (merged, not duplicated). Skill/agent files trigger overwrite confirmation. @@ -78,13 +80,14 @@ java-codebase-rag update --force **Flags:** - `--force` — Overwrite all artifacts even if content matches. - `--dry-run` — Print changes without writing files. +- `--quiet` / `-q` — Suppress the indexing progress stream on stderr (wizard stdout unchanged). +- `--verbose` / `-v` — Raw-relay subprocess output during the indexing sub-step (no progress bar). **Behavior:** - Detects previously configured agent hosts (scans both project-level and user-level config files). - Refreshes skill and agent files (versioned assets from the package). - Updates MCP entrypoint path if `java-codebase-rag-mcp` has moved. -- Runs an incremental index update (Lance + graph) if an index exists — same as `java-codebase-rag increment`. -- Skips MCP config if the entry already exists and is correct. +- Runs an incremental index update (Lance + graph) if an index exists — same as `java-codebase-rag increment`. The indexing sub-step renders the unified `Vectors → Optimize → Graph` progress on **stderr** (see [Indexing progress](#indexing-progress-stderr)); it no longer runs silently. **Exit codes:** - `0` — Success. @@ -95,7 +98,7 @@ java-codebase-rag update --force - **TTY:** human-readable `pprint` of the payload on stdout (except **successful selective `reprocess`** with `--vectors-only` / `--graph-only`, which prints `Rebuilt:` / `Skipped:` lines instead of dumping the full dict). - **Piped / non-TTY:** **single JSON object** per invocation on stdout (no trailing noise). Use this in scripts and CI. -- **Lifecycle stderr:** `init`, `increment`, `reprocess`, and `erase` stream subprocess progress (and relayed child stdout) to **stderr**; pass **`--quiet`** to suppress that stream. **stdout** stays the JSON/pprint payload only. +- **Lifecycle stderr:** `init`, `increment`, `reprocess`, `install`, `update`, and `erase` stream subprocess progress (and relayed child stdout) to **stderr**; pass **`--quiet`** to suppress that stream. **stdout** stays the JSON/pprint payload (`init`/`increment`/`reprocess`) or the wizard conversational text (`install`/`update`) only. Example: @@ -103,6 +106,37 @@ Example: java-codebase-rag meta --source-root /path/to/java/repo --index-dir /path/to/.java-codebase-rag | jq .ontology_version ``` +### Indexing progress (stderr) + +All five lifecycle commands that build the index (`init`, `increment`, `reprocess`, `install`, `update`) render the **same unified progress** on **stderr** during indexing: a header line, a three-phase list `Vectors → Optimize → Graph`, and a footer line. The phase list is the single source of truth for "what's happening right now": + +- **Vectors** — the `cocoindex update` Lance catch-up / full reprocess. +- **Optimize** — the serialized Lance table compaction that runs after a successful vectors phase. +- **Graph** — the `build_ast_graph.py` Kuzu/LadybugDB build (full or incremental). + +**Determinate vs indeterminate per command:** + +| Phase | Determinate? | +| ----- | ------------ | +| `Vectors` (full `init` / `reprocess`) | Approximately determinate — a pre-walk estimates the file count; the bar **clamps to 100% on completion** (the pre-walk overstates by ignored/empty files). | +| `Vectors` (incremental `increment` / `update`) | Indeterminate — CocoIndex's `memo=True` cache only calls the per-file function for changed files, so no denominator is known up front. A pulsing bar plus a "files touched: N" counter. | +| `Optimize` | Always indeterminate (no item count exposed by Lance compaction). | +| `Graph` (full `init` / `reprocess`) | Determinate — pass 1 does a count-first filtered walk for an exact total; passes 2–6 are six known steps. | +| `Graph` (incremental `increment` / `update`) | Determinate when it runs; falls back to a full rebuild on schema change. | + +**Flags, TTY, and failure:** + +| Mode | Behaviour | +| ---- | --------- | +| TTY (default) | `rich` `Live` region — the multi-line phase display (spinner + bar + `%` + ETA). | +| Non-TTY / CI | `rich` auto-disables; concise throttled stderr lines (~every 5 s per phase + a terminal line) so CI logs still show progress. | +| `--quiet` / `-q` | Suppresses the entire progress stream (no header, phases, or footer). The stdout payload is unchanged. | +| `--verbose` / `-v` | Bypasses parsing; relays raw subprocess output verbatim (Lance warnings, brownfield events, the raw `JCIRAG_PROGRESS` protocol lines). No `Live` region. | +| Phase failure | The failing phase renders a red `✗`; the footer carries `(exit=N)`. The `rich` `Live` region is torn down cleanly so the error stays visible. | +| Missing `cocoindex` / builder binary | The pre-spawn stub emits a `status=failed` line; no phase is left hung at `running`. | + +> **Behaviour change (this release).** `install` and `update` now emit their indexing progress on **stderr** (previously `install` printed indexing chatter to stdout, and `update` ran the whole indexing step with `quiet=True` — completely silent). The wizard conversational stdout for both commands is otherwise unchanged. `update`'s previously-ignored `--quiet` / `--verbose` flags, and `install`'s previously-ignored `--verbose` flag, are now wired through (`install` already honored `--quiet`). + ## Environment variables (summary) | Variable | Role | diff --git a/java_codebase_rag/cli.py b/java_codebase_rag/cli.py index 7981c84..58a44b2 100644 --- a/java_codebase_rag/cli.py +++ b/java_codebase_rag/cli.py @@ -144,7 +144,7 @@ def _run_with_pipeline_progress( """ if quiet or verbose: return int(work(None)) - from java_codebase_rag.progress import IndexProgressRenderer, ProgressEvent + from java_codebase_rag.progress import build_index_progress_context # PR-3 owns all three tasks in order: Vectors → Optimize → Graph. The vectors # task is fed by the cocoindex child's per-file ticks + approximate total @@ -152,15 +152,10 @@ def _run_with_pipeline_progress( # in-process by lance_optimize; the graph task is fed by the build_ast_graph # child (subprocess transport). A task only becomes visible/running once its # first event arrives. - phases = ["vectors", "optimize", "graph"] - renderer = IndexProgressRenderer(phases) + renderer, on_progress, console = build_index_progress_context() progress = PipelineProgress(renderer=renderer) - - def on_progress(ev: ProgressEvent) -> None: - renderer.apply(ev) - progress.on_progress = on_progress - progress.console = renderer._console # noqa: SLF001 — shared with the drain for Live-safe routing + progress.console = console _pipeline_header(subcommand, cfg) t0 = time.perf_counter() @@ -570,6 +565,7 @@ def _cmd_install(args: argparse.Namespace) -> int: model=args.model, source_root=None, # None means cwd; installer confirms interactively quiet=bool(args.quiet), + verbose=bool(args.verbose), ) @@ -579,6 +575,8 @@ def _cmd_update(args: argparse.Namespace) -> int: return run_update( force=bool(args.force), dry_run=bool(args.dry_run), + quiet=bool(args.quiet), + verbose=bool(args.verbose), ) diff --git a/java_codebase_rag/installer.py b/java_codebase_rag/installer.py index 0dfcdfd..1f0ef82 100644 --- a/java_codebase_rag/installer.py +++ b/java_codebase_rag/installer.py @@ -14,6 +14,7 @@ import shutil import sys import tempfile +import time from dataclasses import dataclass from pathlib import Path from typing import Literal, NamedTuple @@ -801,6 +802,38 @@ def update_gitignore(cwd: Path) -> None: gitignore_path.write_text("\n".join(lines), encoding="utf-8") +def _index_progress_header(subcommand: str, source_root: Path, index_dir: Path) -> None: + """Print the stderr header framing the indexing sub-step (install/update). + + Mirrors the operator commands' ``_pipeline_header`` but lives in the + installer because the wizard's stdout framing differs. This brackets ONLY + the indexing sub-step — the wizard's prompts stay outside it on stdout. + """ + from java_codebase_rag.cli_format import bold + + print( + bold( + f"java-codebase-rag {subcommand} · source={source_root.resolve()} " + f"· index={index_dir.resolve()}" + ), + file=sys.stderr, + flush=True, + ) + + +def _index_progress_footer(subcommand: str, started: float, *, ok: bool) -> None: + """Print the stderr footer closing the indexing sub-step framing.""" + from java_codebase_rag.cli_format import bold, styled_check, styled_cross + + elapsed = time.time() - started + marker = styled_check() if ok else styled_cross() + print( + f"{marker} {bold(f'java-codebase-rag {subcommand} · finished in {elapsed:.2f}s')}", + file=sys.stderr, + flush=True, + ) + + def run_init_if_needed( source_root: Path, index_dir: Path, @@ -808,15 +841,25 @@ def run_init_if_needed( *, non_interactive: bool, quiet: bool, + verbose: bool = False, ) -> bool: """Run init if index directory has no artifacts. Return True if init was run. + The indexing sub-step (CocoIndex update + AST graph build) renders the + unified ``Vectors → Optimize → Graph`` progress on **stderr** in default + mode (same renderer the operator commands use); the wizard's conversational + stdout is untouched by this function. ``--quiet`` is silent; ``--verbose`` + raw-relays subprocess output. The indexing chatter that used to print to + stdout (``Creating index…`` / ``Index created successfully.``) now lives + on stderr framing so stdout stays the wizard payload. + Args: source_root: Source root directory index_dir: Index directory path model: Embedding model path or "auto" non_interactive: If True, suppress prompts - quiet: If True, suppress output + quiet: If True, suppress progress output + verbose: If True, raw-relay subprocess output (no Live region) Returns: True if init was run, False if skipped @@ -832,36 +875,65 @@ def run_init_if_needed( print("Index already exists. Run `java-codebase-rag reprocess` to rebuild.") return False - print("Creating index...") cfg = resolve_operator_config( source_root=source_root, cli_index_dir=None, # use default (/.java-codebase-rag) cli_embedding_model=model if model != "auto" else None, ) cfg.apply_to_os_environ() - env = cfg.subprocess_env() - # Run CocoIndex update - coco = run_cocoindex_update(env, full_reprocess=False, quiet=quiet) - if coco.returncode != 0: - print(f"Error: CocoIndex update failed with code {coco.returncode}") - return False - - # Run AST graph build - g = run_build_ast_graph( - source_root=cfg.source_root, - ladybug_path=cfg.ladybug_path, - verbose=not quiet, - quiet=quiet, - env=env, - ) - if g.returncode != 0: - print(f"Error: AST graph build failed with code {g.returncode}") - return False - - print("Index created successfully.") - return True + # Indexing sub-step: render unified progress on stderr in default mode only + # (quiet = silent; verbose = raw relay, no Live region). The renderer wraps + # just this sub-step, not the surrounding wizard. + on_progress, on_progress_console = None, None + renderer = None + if not quiet and not verbose: + from java_codebase_rag.progress import build_index_progress_context + + renderer, on_progress, on_progress_console = build_index_progress_context() + + started = time.time() + if renderer is not None: + _index_progress_header("install", cfg.source_root, cfg.index_dir) + renderer.start() + index_ok = True + try: + coco = run_cocoindex_update( + env, + full_reprocess=False, + quiet=quiet, + verbose=verbose, + on_progress=on_progress, + on_progress_console=on_progress_console, + ) + if coco.returncode != 0: + print( + f"Error: CocoIndex update failed with code {coco.returncode}", + file=sys.stderr, + ) + index_ok = False + else: + g = run_build_ast_graph( + source_root=cfg.source_root, + ladybug_path=cfg.ladybug_path, + verbose=verbose, + quiet=quiet, + env=env, + on_progress=on_progress, + on_progress_console=on_progress_console, + ) + if g.returncode != 0: + print( + f"Error: AST graph build failed with code {g.returncode}", + file=sys.stderr, + ) + index_ok = False + finally: + if renderer is not None: + renderer.stop() + _index_progress_footer("install", started, ok=index_ok) + return index_ok def handle_rerun(cwd: Path, *, non_interactive: bool) -> dict | None: @@ -1196,13 +1268,25 @@ def run_update( force: bool, dry_run: bool, cwd: Path | None = None, + quiet: bool = False, + verbose: bool = False, ) -> int: """Run the update pipeline. Returns exit code. + The indexing sub-step (Lance catch-up + incremental graph) renders the + unified ``Vectors → Optimize → Graph`` progress on **stderr** in default + mode and no longer runs with ``quiet=True`` (the reason ``update`` was + silent). ``--quiet`` is silent; ``--verbose`` raw-relays subprocess output. + The wizard's host-detection / refresh / summary stdout is preserved; only + the indexing chatter that used to print to stdout moves onto the stderr + renderer framing. + Args: force: If True, overwrite all artifacts even if matching dry_run: If True, print changes without writing cwd: Current working directory (defaults to Path.cwd()) + quiet: If True, suppress progress output + verbose: If True, raw-relay subprocess output (no Live region) Returns: Exit code (0=success, 1=partial, 2=fatal) @@ -1277,30 +1361,67 @@ def run_update( # The "graph not implemented" warning belongs only on the vectors-only path # (increment --vectors-only), where the graph step is deliberately skipped. if not dry_run: - print("\nUpdating index (Lance + graph)...") cfg.apply_to_os_environ() env = cfg.subprocess_env() - coco = run_cocoindex_update(env, full_reprocess=False, quiet=True) - if coco.returncode != 0: - print(f"Error: Lance index update failed with code {coco.returncode}") - return 1 - - g = run_incremental_graph( - source_root=cfg.source_root, - ladybug_path=cfg.ladybug_path, - verbose=False, - quiet=True, - env=env, - ) - if g.returncode != 0: - # Artifacts above already refreshed; the graph catch-up is best-effort - # here. Surface a truthful, actionable message instead of leaving the - # graph silently stale or claiming the feature is unimplemented. - print( - f"\nWarning: incremental graph update failed (exit {g.returncode}). " - "Run `java-codebase-rag reprocess` for a full rebuild." + # Indexing sub-step: render unified progress on stderr in default mode + # only (quiet = silent; verbose = raw relay). No longer runs quiet=True + # — that was why `update` was silent. The renderer wraps just this + # sub-step; the wizard's summary stdout below is outside it. + on_progress, on_progress_console = None, None + renderer = None + if not quiet and not verbose: + from java_codebase_rag.progress import build_index_progress_context + + renderer, on_progress, on_progress_console = build_index_progress_context() + + started = time.time() + if renderer is not None: + _index_progress_header("update", cfg.source_root, cfg.index_dir) + renderer.start() + index_ok = True + try: + coco = run_cocoindex_update( + env, + full_reprocess=False, + quiet=quiet, + verbose=verbose, + on_progress=on_progress, + on_progress_console=on_progress_console, ) + if coco.returncode != 0: + print( + f"Error: Lance index update failed with code {coco.returncode}", + file=sys.stderr, + ) + index_ok = False + else: + g = run_incremental_graph( + source_root=cfg.source_root, + ladybug_path=cfg.ladybug_path, + verbose=verbose, + quiet=quiet, + env=env, + on_progress=on_progress, + on_progress_console=on_progress_console, + ) + if g.returncode != 0: + # Artifacts above already refreshed; the graph catch-up is + # best-effort here. Surface a truthful, actionable message + # instead of leaving the graph silently stale or claiming + # the feature is unimplemented. Goes to stderr (indexing + # progress framing), not the wizard's stdout summary. + print( + f"\nWarning: incremental graph update failed (exit {g.returncode}). " + "Run `java-codebase-rag reprocess` for a full rebuild.", + file=sys.stderr, + ) + finally: + if renderer is not None: + renderer.stop() + _index_progress_footer("update", started, ok=index_ok) + if not index_ok: + return 1 else: print("\nWould run incremental index update (Lance + graph).") @@ -1320,6 +1441,7 @@ def run_install( model: str | None, source_root: Path | None = None, quiet: bool = False, + verbose: bool = False, ) -> int: """Run the install pipeline. Returns exit code. @@ -1330,6 +1452,7 @@ def run_install( model: Model from CLI flag source_root: Source root path (defaults to cwd if None) quiet: If True, suppress output + verbose: If True, raw-relay subprocess indexing output (no Live region) Returns: Exit code (0=success, 1=partial, 2=fatal) @@ -1428,6 +1551,7 @@ def run_install( resolved_model, non_interactive=non_interactive, quiet=quiet, + verbose=verbose, ) return 0 diff --git a/java_codebase_rag/progress.py b/java_codebase_rag/progress.py index 62c8a45..c07df5f 100644 --- a/java_codebase_rag/progress.py +++ b/java_codebase_rag/progress.py @@ -57,6 +57,7 @@ "ProgressRelay", "CallbackRenderer", "make_relay", + "build_index_progress_context", ] ProgressKind = Literal["vectors", "graph", "optimize"] @@ -433,6 +434,38 @@ def make_relay( ) +# The canonical phase order shared by every lifecycle command that renders +# progress. The operator commands (init/increment/reprocess) and the installer +# sub-steps (install/update indexing) all render this same list so the +# Vectors → Optimize → Graph shape is uniform across the CLI. +_INDEX_PHASES = ["vectors", "optimize", "graph"] + + +def build_index_progress_context( + phases: list[str] | None = None, +) -> tuple["IndexProgressRenderer", Callable[[ProgressEvent], None], "Console"]: + """Construct the shared ``(renderer, on_progress, console)`` triple. + + Both ``cli._run_with_pipeline_progress`` (operator commands, default TTY + mode) and the installer's indexing sub-step (``installer.run_init_if_needed`` + / ``run_update``) use this so the phase list, the callback wiring, and the + single-writer console are defined in exactly one place. The returned + ``on_progress`` forwards each event to ``renderer.apply``; ``console`` is + the renderer's stderr ``rich.Console`` so the subprocess drain routes + non-progress lines through ``console.print`` while a Live region is up. + + The caller owns ``renderer.start()``/``stop()`` lifecycle. In ``--quiet`` + or ``--verbose`` mode the caller simply does not call this helper (quiet + is silent; verbose raw-relays). + """ + renderer = IndexProgressRenderer(phases if phases is not None else _INDEX_PHASES) + + def on_progress(ev: ProgressEvent) -> None: + renderer.apply(ev) + + return renderer, on_progress, renderer._console # noqa: SLF001 — shared console for the drain + + # --------------------------------------------------------------------------- # Relay # --------------------------------------------------------------------------- diff --git a/tests/test_installer.py b/tests/test_installer.py index 4d45e43..716c45a 100644 --- a/tests/test_installer.py +++ b/tests/test_installer.py @@ -1232,7 +1232,8 @@ def test_update_honors_yaml_source_root_for_nested_config_dir( # resolved JAVA_CODEBASE_RAG_SOURCE_ROOT / _INDEX_DIR. captured: dict = {} - def capture_coco(env, *, full_reprocess, quiet, verbose=True, lance_project_root=None): + def capture_coco(env, *, full_reprocess, quiet, verbose=True, lance_project_root=None, + on_progress=None, on_progress_console=None): captured["env"] = env return CompletedProcess(["cocoindex"], 0) @@ -1362,3 +1363,221 @@ def test_update_missing_mcp_binary_returns_partial_failure(self, tmp_path, monke result = run_update(force=False, dry_run=False, cwd=tmp_path) # Should return partial failure (1) because artifact refresh failed assert result == 1 + + +# --------------------------------------------------------------------------- +# PR-4 — install/update unified index progress (stderr renderer) +# --------------------------------------------------------------------------- + + +def _patch_pipeline_for_progress(monkeypatch, *, emit: bool = True) -> dict: + """Patch the three pipeline helpers the installer uses to emit progress. + + Records the ``quiet``/``verbose`` kwargs each was called with so tests can + assert the installer no longer forces ``quiet=True``. Returns the call log. + """ + import subprocess + from java_codebase_rag import pipeline as _pipeline + + calls: dict = {"coco": [], "graph": [], "incremental": []} + + def _coco(env, *, full_reprocess, quiet, verbose=True, lance_project_root=None, + on_progress=None, on_progress_console=None): + calls["coco"].append({"quiet": quiet, "verbose": verbose}) + if emit and on_progress is not None: + from java_codebase_rag.progress import ProgressEvent + on_progress(ProgressEvent( + kind="vectors", phase=None, pass_=None, done=1, total=10, + status="running", elapsed_s=None)) + return subprocess.CompletedProcess(args=["stub"], returncode=0, stdout="", stderr="") + + def _graph(*, source_root, ladybug_path, verbose, quiet=False, env=None, + on_progress=None, on_progress_console=None): + calls["graph"].append({"quiet": quiet, "verbose": verbose}) + if emit and on_progress is not None: + from java_codebase_rag.progress import ProgressEvent + on_progress(ProgressEvent( + kind="graph", phase=None, pass_="1/6", done=1, total=10, + status="running", elapsed_s=None)) + return subprocess.CompletedProcess(args=["stub"], returncode=0, stdout="", stderr="") + + def _incremental(*, source_root, ladybug_path, verbose, quiet=False, env=None, + on_progress=None, on_progress_console=None): + calls["incremental"].append({"quiet": quiet, "verbose": verbose}) + if emit and on_progress is not None: + from java_codebase_rag.progress import ProgressEvent + on_progress(ProgressEvent( + kind="graph", phase=None, pass_="1/6", done=1, total=10, + status="running", elapsed_s=None)) + return subprocess.CompletedProcess(args=["stub"], returncode=0, stdout="", stderr="") + + monkeypatch.setattr(_pipeline, "run_cocoindex_update", _coco) + monkeypatch.setattr(_pipeline, "run_build_ast_graph", _graph) + monkeypatch.setattr(_pipeline, "run_incremental_graph", _incremental) + return calls + + +class TestPR4IndexProgress: + """PR-4: install/update emit unified index progress on stderr.""" + + def _setup_repo(self, tmp_path, monkeypatch): + """Copy the bank-chat fixture and stub MCP discovery for install/update. + + Also writes a configured ``.mcp.json`` so ``update`` (which requires a + prior ``install`` per its docstring) detects a configured host and + reaches its indexing sub-step. + """ + import shutil + bank_chat = Path("tests/bank-chat-system") + if not bank_chat.is_dir(): + pytest.skip("bank-chat-system fixture not found") + shutil.copytree(bank_chat, tmp_path / "bank-chat") + cwd = tmp_path / "bank-chat" + (cwd / ".git").mkdir() + # A configured host entry — the state `update` expects post-install. + (cwd / ".mcp.json").write_text( + json.dumps( + { + "mcpServers": { + "java-codebase-rag": { + "command": "/fake/bin/java-codebase-rag-mcp", + "type": "stdio", + } + } + } + ), + encoding="utf-8", + ) + monkeypatch.setattr(shutil, "which", lambda x: "/fake/bin/java-codebase-rag-mcp") + monkeypatch.setattr( + "java_codebase_rag.installer._read_package_artifact", + lambda path: "PACKAGE CONTENT", + ) + monkeypatch.chdir(cwd) + return cwd + + def test_install_emits_indexing_progress_on_stderr(self, tmp_path, monkeypatch): + """install drives the renderer from the patched pipeline helpers; the + JCIRAG_PROGRESS event is consumed by the parser and surfaces as a + rendered progress line on stderr. Wizard stdout prompts remain on + stdout.""" + import io + import contextlib + from java_codebase_rag.installer import run_install + + cwd = self._setup_repo(tmp_path, monkeypatch) + _patch_pipeline_for_progress(monkeypatch, emit=True) + + out, err = io.StringIO(), io.StringIO() + with contextlib.redirect_stdout(out), contextlib.redirect_stderr(err): + rc = run_install( + non_interactive=True, + agents=["claude-code"], + scope="project", + model="auto", + source_root=cwd, + quiet=False, + ) + assert rc == 0 + err_text = err.getvalue() + out_text = out.getvalue() + # The raw structured protocol line is parsed, never raw-relayed. + assert "JCIRAG_PROGRESS kind=vectors" not in err_text + # But indexing progress IS rendered on stderr (non-TTY concise fallback + # prints a "vectors ..." line; the patched coco helper emitted a vectors + # event). A graph event is emitted by the patched graph helper too. + assert "vectors" in err_text.lower() + # The wizard's conversational stdout is preserved (it writes the YAML + # config path when not quiet). + assert "Configuration written" in out_text or ".java-codebase-rag.yml" in out_text + + def test_update_emits_indexing_progress_on_stderr(self, tmp_path, monkeypatch): + """update is no longer silent: the patched cocoindex + incremental + graph helpers drive the renderer, and progress surfaces on stderr.""" + import io + import contextlib + from java_codebase_rag.installer import run_update + + cwd = self._setup_repo(tmp_path, monkeypatch) + # A configured host + a real-looking index so run_update reaches indexing. + index_dir = cwd / ".java-codebase-rag" + index_dir.mkdir(exist_ok=True) + (index_dir / "code_graph.lbug").write_text("", encoding="utf-8") + + _patch_pipeline_for_progress(monkeypatch, emit=True) + monkeypatch.delenv("JAVA_CODEBASE_RAG_SOURCE_ROOT", raising=False) + monkeypatch.delenv("JAVA_CODEBASE_RAG_INDEX_DIR", raising=False) + + out, err = io.StringIO(), io.StringIO() + with contextlib.redirect_stdout(out), contextlib.redirect_stderr(err): + rc = run_update(force=False, dry_run=False, cwd=cwd) + assert rc in (0, 1) + err_text = err.getvalue() + # Progress reached the renderer (coco + incremental both emitted). + assert "JCIRAG_PROGRESS kind=vectors" not in err_text + assert "vectors" in err_text.lower() + + def test_update_runs_indexing_without_quiet_true(self, tmp_path, monkeypatch): + """Regression: update no longer forces quiet=True on the indexing + helpers (the reason it was silent today). In the default path both + helpers are called with quiet=False.""" + from java_codebase_rag.installer import run_update + + cwd = self._setup_repo(tmp_path, monkeypatch) + index_dir = cwd / ".java-codebase-rag" + index_dir.mkdir(exist_ok=True) + (index_dir / "code_graph.lbug").write_text("", encoding="utf-8") + + calls = _patch_pipeline_for_progress(monkeypatch, emit=False) + monkeypatch.delenv("JAVA_CODEBASE_RAG_SOURCE_ROOT", raising=False) + monkeypatch.delenv("JAVA_CODEBASE_RAG_INDEX_DIR", raising=False) + + rc = run_update(force=False, dry_run=False, cwd=cwd) + assert rc in (0, 1) + # Both indexing helpers ran and were NOT silenced. + assert calls["coco"], "run_cocoindex_update was not called" + assert calls["incremental"], "run_incremental_graph was not called" + assert calls["coco"][-1]["quiet"] is False + assert calls["incremental"][-1]["quiet"] is False + + def test_install_update_stdout_contract_preserved(self, tmp_path, monkeypatch): + """The wizard's human-readable stdout shape is unchanged: NO + JCIRAG_PROGRESS line leaks to stdout, and the indexing chatter that + used to live on stdout ("Creating index..." / "Updating index...") + no longer appears there.""" + import io + import contextlib + from java_codebase_rag.installer import run_install, run_update + + cwd = self._setup_repo(tmp_path, monkeypatch) + _patch_pipeline_for_progress(monkeypatch, emit=True) + + # --- install --- + out, err = io.StringIO(), io.StringIO() + with contextlib.redirect_stdout(out), contextlib.redirect_stderr(err): + run_install( + non_interactive=True, agents=["claude-code"], scope="project", + model="auto", source_root=cwd, quiet=False, + ) + install_out = out.getvalue() + # No structured progress line on stdout (stdout is the wizard payload). + assert "JCIRAG_PROGRESS" not in install_out + # The old stdout indexing chatter is gone (moved to stderr framing). + assert "Creating index..." not in install_out + assert "Index created successfully." not in install_out + + # --- update --- + index_dir = cwd / ".java-codebase-rag" + index_dir.mkdir(exist_ok=True) + (index_dir / "code_graph.lbug").write_text("", encoding="utf-8") + _patch_pipeline_for_progress(monkeypatch, emit=True) + monkeypatch.delenv("JAVA_CODEBASE_RAG_SOURCE_ROOT", raising=False) + monkeypatch.delenv("JAVA_CODEBASE_RAG_INDEX_DIR", raising=False) + + out2, err2 = io.StringIO(), io.StringIO() + with contextlib.redirect_stdout(out2), contextlib.redirect_stderr(err2): + run_update(force=False, dry_run=False, cwd=cwd) + update_out = out2.getvalue() + assert "JCIRAG_PROGRESS" not in update_out + # The old stdout indexing chatter moved off stdout. + assert "Updating index (Lance + graph)..." not in update_out diff --git a/tests/test_java_codebase_rag_cli.py b/tests/test_java_codebase_rag_cli.py index aa8f3b9..cbac80d 100644 --- a/tests/test_java_codebase_rag_cli.py +++ b/tests/test_java_codebase_rag_cli.py @@ -1361,3 +1361,96 @@ def test_cli_graph_progress_absent_when_quiet( # In quiet mode there is no header/footer framing either. assert "java-codebase-rag init" not in err + +# --------------------------------------------------------------------------- +# PR-4 — wire --quiet/--verbose through update / install +# --------------------------------------------------------------------------- + + +def test_cmd_update_forwards_quiet_flag( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """`_cmd_update --quiet` forwards quiet=True to run_update. + + Until PR-4 _cmd_update ignored both --quiet and --verbose entirely. + """ + import java_codebase_rag.installer as _installer + + captured: dict = {} + + def _fake_run_update(*, force=False, dry_run=False, cwd=None, + quiet=False, verbose=False): + captured["quiet"] = quiet + captured["verbose"] = verbose + captured["force"] = force + captured["dry_run"] = dry_run + return 0 + + monkeypatch.setattr(_installer, "run_update", _fake_run_update) + monkeypatch.chdir(tmp_path) + + rc = cli_mod.main(["update", "--quiet"]) + assert rc == 0 + assert captured["quiet"] is True + + +def test_cmd_update_forwards_verbose_flag( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """`_cmd_update --verbose` forwards verbose=True to run_update.""" + import java_codebase_rag.installer as _installer + + captured: dict = {} + + def _fake_run_update(*, force=False, dry_run=False, cwd=None, + quiet=False, verbose=False): + captured["quiet"] = quiet + captured["verbose"] = verbose + return 0 + + monkeypatch.setattr(_installer, "run_update", _fake_run_update) + monkeypatch.chdir(tmp_path) + + rc = cli_mod.main(["update", "--verbose"]) + assert rc == 0 + assert captured["verbose"] is True + # And the default path (no flag) forwards both as False. + rc2 = cli_mod.main(["update"]) + assert rc2 == 0 + assert captured["quiet"] is False + assert captured["verbose"] is False + + +def test_cmd_install_forwards_verbose_flag( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """`_cmd_install --verbose` forwards verbose=True to run_install. + + Until PR-4 _cmd_install wired only --quiet through. + """ + import java_codebase_rag.installer as _installer + + captured: dict = {} + + def _fake_run_install(*, non_interactive, agents, scope, model, + source_root=None, quiet=False, verbose=False): + captured["quiet"] = quiet + captured["verbose"] = verbose + captured["non_interactive"] = non_interactive + return 0 + + monkeypatch.setattr(_installer, "run_install", _fake_run_install) + monkeypatch.chdir(tmp_path) + + rc = cli_mod.main( + ["install", "--non-interactive", "--agent", "claude-code", "--verbose"] + ) + assert rc == 0 + assert captured["verbose"] is True + # quiet still flows through too. + rc2 = cli_mod.main( + ["install", "--non-interactive", "--agent", "claude-code", "--quiet"] + ) + assert rc2 == 0 + assert captured["quiet"] is True + From 19073929a8f0c824ea218e8009a0b0cfe5cd39d6 Mon Sep 17 00:00:00 2001 From: Dmitry Teryaev Date: Mon, 15 Jun 2026 05:46:14 +0300 Subject: [PATCH 11/11] fix PR-4: best-effort graph catch-up in update; truthful failure footer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit run_update's graph catch-up is best-effort again (graph failure -> exit 0 + Warning, not exit 1) — matches the original intent and the output/UX- only scope. Both installer brackets catch BaseException to set the failed footer marker before re-raising. Footer uses perf_counter like the operator footer. Co-Authored-By: Claude --- java_codebase_rag/installer.py | 29 ++++++++++---- tests/test_installer.py | 73 ++++++++++++++++++++++++++++++++++ 2 files changed, 94 insertions(+), 8 deletions(-) diff --git a/java_codebase_rag/installer.py b/java_codebase_rag/installer.py index 1f0ef82..3ea835d 100644 --- a/java_codebase_rag/installer.py +++ b/java_codebase_rag/installer.py @@ -825,7 +825,7 @@ def _index_progress_footer(subcommand: str, started: float, *, ok: bool) -> None """Print the stderr footer closing the indexing sub-step framing.""" from java_codebase_rag.cli_format import bold, styled_check, styled_cross - elapsed = time.time() - started + elapsed = time.perf_counter() - started marker = styled_check() if ok else styled_cross() print( f"{marker} {bold(f'java-codebase-rag {subcommand} · finished in {elapsed:.2f}s')}", @@ -893,7 +893,7 @@ def run_init_if_needed( renderer, on_progress, on_progress_console = build_index_progress_context() - started = time.time() + started = time.perf_counter() if renderer is not None: _index_progress_header("install", cfg.source_root, cfg.index_dir) renderer.start() @@ -929,6 +929,12 @@ def run_init_if_needed( file=sys.stderr, ) index_ok = False + except BaseException: + # An exception from cocoindex/graph means the index did not succeed; + # flip the footer marker before re-raising so it renders a red cross + # (mirrors cli._run_with_pipeline_progress's BaseException handler). + index_ok = False + raise finally: if renderer is not None: renderer.stop() @@ -1375,7 +1381,7 @@ def run_update( renderer, on_progress, on_progress_console = build_index_progress_context() - started = time.time() + started = time.perf_counter() if renderer is not None: _index_progress_header("update", cfg.source_root, cfg.index_dir) renderer.start() @@ -1406,16 +1412,23 @@ def run_update( on_progress_console=on_progress_console, ) if g.returncode != 0: - # Artifacts above already refreshed; the graph catch-up is - # best-effort here. Surface a truthful, actionable message - # instead of leaving the graph silently stale or claiming - # the feature is unimplemented. Goes to stderr (indexing - # progress framing), not the wizard's stdout summary. + # The graph catch-up is best-effort: `update`'s primary job + # is refreshing shipped artifacts + vectors (cocoindex). A + # graph failure surfaces a truthful, actionable Warning on + # stderr but does NOT flip index_ok (which drives both the + # footer marker and the return code) — exit 0 with a green + # check + the Warning line carrying the graph caveat. print( f"\nWarning: incremental graph update failed (exit {g.returncode}). " "Run `java-codebase-rag reprocess` for a full rebuild.", file=sys.stderr, ) + except BaseException: + # An exception from cocoindex/graph means the index did not succeed; + # flip the footer marker before re-raising so it renders a red cross + # (mirrors cli._run_with_pipeline_progress's BaseException handler). + index_ok = False + raise finally: if renderer is not None: renderer.stop() diff --git a/tests/test_installer.py b/tests/test_installer.py index 716c45a..bf858f7 100644 --- a/tests/test_installer.py +++ b/tests/test_installer.py @@ -1581,3 +1581,76 @@ def test_install_update_stdout_contract_preserved(self, tmp_path, monkeypatch): assert "JCIRAG_PROGRESS" not in update_out # The old stdout indexing chatter moved off stdout. assert "Updating index (Lance + graph)..." not in update_out + + def test_update_graph_catchup_failure_is_best_effort_exit_0(self, tmp_path, monkeypatch): + """run_update's graph catch-up is best-effort: a graph-only failure must + NOT flip the exit code. Vectors (cocoindex) succeeded, so exit 0 with a + Warning on stderr carrying the graph caveat — matches the original + semantics and the output/UX-only scope of PR-4.""" + import io + import contextlib + import subprocess + from java_codebase_rag.installer import run_update + + cwd = self._setup_repo(tmp_path, monkeypatch) + index_dir = cwd / ".java-codebase-rag" + index_dir.mkdir(exist_ok=True) + (index_dir / "code_graph.lbug").write_text("", encoding="utf-8") + monkeypatch.delenv("JAVA_CODEBASE_RAG_SOURCE_ROOT", raising=False) + monkeypatch.delenv("JAVA_CODEBASE_RAG_INDEX_DIR", raising=False) + + # Patch at the installer import site (java_codebase_rag.pipeline). + # cocoindex succeeds; the incremental graph returns a non-zero exit. + def coco_ok(env, *, full_reprocess, quiet, verbose=True, + lance_project_root=None, on_progress=None, on_progress_console=None): + return subprocess.CompletedProcess(args=["stub"], returncode=0, stdout="", stderr="") + + def graph_fail(**kwargs): + return subprocess.CompletedProcess(args=["stub"], returncode=3, stdout="", stderr="") + + monkeypatch.setattr("java_codebase_rag.pipeline.run_cocoindex_update", coco_ok) + monkeypatch.setattr("java_codebase_rag.pipeline.run_incremental_graph", graph_fail) + + out, err = io.StringIO(), io.StringIO() + with contextlib.redirect_stdout(out), contextlib.redirect_stderr(err): + rc = run_update(force=False, dry_run=False, cwd=cwd) + + assert rc == 0, f"graph-only failure must be best-effort (exit 0), got {rc}" + err_text = err.getvalue() + assert "Warning:" in err_text + assert "incremental graph update failed" in err_text + + def test_install_indexing_exception_renders_failed_footer(self, tmp_path, monkeypatch): + """If run_cocoindex_update raises during install's indexing sub-step, + the renderer bracket must render a failed (red cross) footer before the + exception propagates — not a green check right before the traceback. + Mirrors cli._run_with_pipeline_progress's BaseException handler.""" + import io + import contextlib + from java_codebase_rag import cli_format + from java_codebase_rag.installer import run_install + + cwd = self._setup_repo(tmp_path, monkeypatch) + + def boom(env, *, full_reprocess, quiet, verbose=True, + lance_project_root=None, on_progress=None, on_progress_console=None): + raise RuntimeError("boom from cocoindex") + + monkeypatch.setattr("java_codebase_rag.pipeline.run_cocoindex_update", boom) + + out, err = io.StringIO(), io.StringIO() + with contextlib.redirect_stdout(out), contextlib.redirect_stderr(err): + with pytest.raises(RuntimeError, match="boom from cocoindex"): + run_install( + non_interactive=True, + agents=["claude-code"], + scope="project", + model="auto", + source_root=cwd, + quiet=False, + ) + + err_text = err.getvalue() + # The footer rendered the failure marker (red cross), not the green check. + assert cli_format.styled_cross() in err_text + assert cli_format.styled_check() not in err_text