Skip to content

fix(mcp): renumber deduped guided tour steps - #1889

Open
mikemikimike wants to merge 11 commits into
repowise-dev:mainfrom
mikemikimike:fix/1872-renumber-guided-tour
Open

fix(mcp): renumber deduped guided tour steps#1889
mikemikimike wants to merge 11 commits into
repowise-dev:mainfrom
mikemikimike:fix/1872-renumber-guided-tour

Conversation

@mikemikimike

Copy link
Copy Markdown

Summary

Implementation

The persisted overview metadata is left unchanged. Renumbering happens only while building the MCP projection, so other consumers of the stored tour retain their original ordering.

Compatibility

No API fields were added or removed. The order values are now contiguous for deduplicated MCP tour responses, matching the response hint's sequential-walk semantics.

Validation

  • python -m py_compile packages/server/src/repowise/server/mcp_server/tool_overview.py tests/unit/server/mcp/test_overview_outline.py — passed locally.
  • Full pytest/CI validation was not run locally because the repository clone was interrupted by the environment's network timeout; GitHub Actions should run the repository test matrix.

@repowise-bot

repowise-bot Bot commented Aug 23, 2026

Copy link
Copy Markdown

✅ Health of changed files: 10.0 (unchanged)

📋 At a glance
1 hotspot touched. Scoped to tests.

✅ Health gate: passed

🔎 More signals (1)

🔥 Hotspot touched (1)

  • .../mcp/test_overview_outline.py: 1 commits/90d, 0 dependents · primary owner: Raghav Chamadiya (100%)

👀 Suggested reviewers @RaghavChamadiya


📊 See the full report for this PR
Your repo map with this PR's blast radius lit up, every caller of the contracts it changes, and health before and after. No sign-in. · ⭐ Star Repowise · 📥 Install bot · Silence on a single PR with [skip repowise] in the title · Per-repo toggle on repowise.dev/settings?tab=bot · Updated 2026-08-27 17:50 UTC

@mikemikimike
mikemikimike force-pushed the fix/1872-renumber-guided-tour branch from 19bb722 to 2f6f577 Compare August 23, 2026 15:49
@RaghavChamadiya

Copy link
Copy Markdown
Member

Thanks @mikemikimike. I am taking #1878 for #1872, which landed about four hours earlier with the identical enumerate(tour, start=1) change.

Your test is the better one though, and I would rather have it than not. #1878's case feeds four steps that do not deduplicate, so the orders are 1, 2, 3, 4 before and after the fix and the assertion orders == list(range(1, len(orders) + 1)) passes on unfixed main. It documents the intent but cannot catch the regression. Yours feeds a tour that actually collapses and pins the result:

assert [step["order"] for step in result["guided_tour"]] == [1, 2, 3]
assert [step["target_path"] for step in result["guided_tour"]] == ["a.py", "b.py", "d.py"]
assert json.loads(page.metadata_json) == metadata

Checking the target paths as well as the numbers is what proves the renumbering follows the surviving steps rather than just producing a tidy sequence, and asserting metadata_json is unmutated covers a failure mode neither of us would have thought to look for until it happened.

So: once #1878 merges, send this as a test-only PR against test_overview_outline.py and I will take it. Two things to fix first, both of which are why this one is not green yet. The import block is not alphabetical (_build_outline then _build_guided_tour), which ruff will flag as I001, and the dict literals in the tour fixture run past the line limit.

test_guided_tour_is_omitted_when_not_requested is worth keeping in that PR too. Nothing currently pins the want_tour=False path.

@mikemikimike

Copy link
Copy Markdown
Author

Thanks @RaghavChamadiya. After the main-branch merge, this PR is now test-only as requested. I ran Ruff on tests/unit/server/mcp/test_overview_outline.py; the import order check passes and Ruff reformatted the long fixture literals in commit 73c67edcb39b1df4df647506f44db61649b3f263, which is pushed to the fork branch.

Verification:

  • uvx --from ruff==0.6.9 ruff check tests/unit/server/mcp/test_overview_outline.py — passed
  • uvx --from ruff==0.6.9 ruff format --check tests/unit/server/mcp/test_overview_outline.py — passed
  • pytest -q --basetemp=<isolated repo directory> tests/unit/server/mcp/test_overview_outline.py — 13 passed
  • git diff --check — passed

The full test suite was not run. uv run --frozen could not initialize the repository environment because the host Python/NumPy build hit a Windows locale decoding error, so the focused test used the available pytest with the repository's three source roots on PYTHONPATH and third-party plugin autoload disabled.

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.

[Bug] guided_tour skips step numbers after deduplication, so the walk its own hint promises is not walkable

2 participants