Skip to content

fix(cli): pass RichProgressCallback to workspace persistence phase - #2158

Open
Shivang9983 wants to merge 1 commit into
repowise-dev:mainfrom
Shivang9983:fix/workspace-persist-progress
Open

fix(cli): pass RichProgressCallback to workspace persistence phase#2158
Shivang9983 wants to merge 1 commit into
repowise-dev:mainfrom
Shivang9983:fix/workspace-persist-progress

Conversation

@Shivang9983

Copy link
Copy Markdown
Contributor

Summary

  • Adds explicit progress feedback to the workspace initialization persistence phase by passing RichProgressCallback with an indeterminate spinner (on_phase_start("persist", None)).
  • Wraps persist_result in workspace.py inside a try...finally block to guarantee on_phase_done triggers cleanly across per-repo loops even if persistence fails.
  • Resolves silent execution after page generation during repowise workspace init.

Related Issues

Fixes #2063

Test Plan

  • Verified local installation via pip install -e packages/core -e packages/cli.
  • Manually executed workspace init to confirm live spinner/elapsed timer renders after ✓ Generated X pages.
  • Verified linting passes (ruff check packages/cli/src/repowise/cli/commands/init_cmd/workspace.py).
  • Ran test suite (pytest).

Checklist

  • My code follows the project's code style
  • I have added tests for new functionality
  • All existing tests still pass
  • I have updated documentation if needed

@RaghavChamadiya

Copy link
Copy Markdown
Member

Thanks @Shivang9983, and thanks for copying the single-repo pattern rather than inventing a second one. callback.rebind(...), on_phase_start("persist", None) with an indeterminate total, and callback.table passed through as timings are all exactly right, and the try/finally is better than the reference site, which leaves the phase open if persist_result raises.

The blocker is the None you hand RichProgressCallback. In the single-repo flow the callback is built inside a live with Progress(...) as persist_bar: block and gets that bar (init_cmd/command.py:1622-1623). Here it gets nothing, and on_phase_start has no guard for that:

# packages/cli/src/repowise/cli/ui/progress.py:179-185
def on_phase_start(self, phase: str, total: int | None) -> None:
    label = _PHASE_LABELS.get(phase, f"{phase}…")
    if phase in self._tasks:
        self._progress.update(self._tasks[phase], total=total, visible=True)
    else:
        self._tasks[phase] = self._progress.add_task(label, total=total, visible=True, cost=0.0)

self._tasks is empty on a fresh instance, so the else branch runs and calls .add_task on None. Reproduced against your change:

>>> pc = PhaseTimingRecorder(None).rebind(RichProgressCallback(None, Console()))
>>> pc.on_phase_start("persist", None)
AttributeError: 'NoneType' object has no attribute 'add_task'

That line sits on the main per-repo path in _index_one_repo, not a fallback branch, so as it stands repowise init on a workspace raises on the first repo it tries to persist. Nothing in the unit suite drives that path, which is why CI is green.

Two ways out. Either build a real Progress here the way command.py:1615-1622 does, which also gets you the spinner the issue was asking for, or give RichProgressCallback a self._progress is None guard so a bar-less callback degrades to timings plus messages. I would take the first: #2063 is about the stage being silent, and a recorder that records but draws nothing does not fix that.

One non-blocking nit while you are in there: the new line 476 is a blank line carrying two trailing spaces.

Ping me once it draws a real bar and I will take another look.

@RaghavChamadiya

Copy link
Copy Markdown
Member

One more thing while you are rebuilding this, and it is one you already named yourself. On #2063 you said you would "ensure persist_callback.warnings are folded into the repo record", and this version does not do that yet.

It matters for the same reason the phase announcement does. Persistence runs on its own callback, so any warning it emits is invisible to the run record unless someone moves it across deliberately. The single-repo flow does exactly that, one line after the block you copied:

# init_cmd/command.py:1632
run_warnings.extend(persist_callback.warnings)

Without it, a workspace repo whose persistence degraded writes a state.json that looks clean, which is the same silent-degradation shape #1369 was about.

So the persist block wants three things rather than two: a real Progress so the stage is visibly working, the phase open/close you already have, and the warnings folded into whatever this repo's state write uses. Sorry for splitting the feedback across two comments.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Feature] Workspace init is silent while persisting generated pages

2 participants