From 592745118f60974ba273cd0fdb9a7fab4bb47896 Mon Sep 17 00:00:00 2001 From: Dmitry Teryaev Date: Sun, 14 Jun 2026 23:15:23 +0300 Subject: [PATCH 1/5] 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 2/5] 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 3/5] 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 4/5] 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 5/5] 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