Skip to content

refactor(infrastructure): remove viz_client, a read client with no caller (#724, slice 5) - #900

Merged
github-actions[bot] merged 1 commit into
mainfrom
fix/724-remove-dead-viz-client
Nov 6, 2026
Merged

refactor(infrastructure): remove viz_client, a read client with no caller (#724, slice 5)#900
github-actions[bot] merged 1 commit into
mainfrom
fix/724-remove-dead-viz-client

Conversation

@cdeust

@cdeust cdeust commented Nov 6, 2026

Copy link
Copy Markdown
Owner

Summary

Slice 5 of #724: removes mcp_server/infrastructure/viz_client.py and its test. The module was the read side of the query_workflow_graph handler, which left mcp_server/ when the visualization stack moved to cortex-viz (docs/audits/visualization-companion-backends-2026-11-05.md §6). No module imports it. The writer of the registry it read (mcp_server/server/viz_instance.py) is gone too. Only its own test called it. CLAUDE.md forbids compatibility shims, and §9 of the template forbids dead code, so the module is removed rather than given a caller.

Refs #724 (slice 5 of 6; not Closes: slices 1–4 wait for the owner's decision on the companion's SQLite support, and slice 6 lives in the companion repository).

Root cause: the extraction of the visualization stack (commits 77dba678 / 67286929 per the audit, not verifiable in this shallow clone) removed the handler but not its infrastructure client.

Type of change

  • Refactor (no functional change; rules/coding-standards.md compliance)

Test plan

  • grep -rn viz_client mcp_server tests_py scripts benchmarks --include=*.py: no match after removal.
  • python scripts/check_craftsmanship.py: before, it reports the stale baseline entry [method-size] viz_client.py: fetch_live_graph. After --write-baseline it reports OK, with only that entry dropped (diff −6/+1).
  • pytest tests_py/core/test_wiki_drift.py tests_py/scripts tests_py/infrastructure/test_project_wiki_mirror.py tests_py/test_shared_host_decisions.py: 1903 passed, 15 skipped. PostgreSQL is absent in this session.
  • ruff check (0.17.3, the uv.lock version) reports 8 E501 errors. All 8 are on main too, in files this PR does not touch (scripts/ci_pipeline_source_contract.py, scripts/tests/test_no_main_comparison.py, tests_py/scripts/test_ci_review_required.py).

Not verified here: the full suite. torch could not be installed through this session's proxy, so the environment was synced with --no-install-package torch. CI is the receipt.

Audit notes

  • ADR-0623 (wiki/adr/migration/0623-viz-client.md and its mirror) and the viz_client sentence of ADR-1112 are kept unchanged. They are historical decision records, and the migration ADRs preserve evidence verbatim. Whether to mark ADR-0623 superseded is left to the owner.
  • No CHANGELOG entry: this removes no user-visible behavior.

Coding-standards compliance

  • §2.2 Layer dependency direction preserved.
  • §9 No dead code.

Breaking changes

None: no caller existed.

Generated by Claude Code

@claude
refactor(infrastructure): remove viz_client, a read client with no ca…
b8fed2f 
…ller (#724, slice 5)

viz_client.py was written for the query_workflow_graph handler, which left
mcp_server/ with the extraction of the visualization stack into cortex-viz
(docs/audits/visualization-companion-backends-2026-11-05.md §6). No module
imports it; only its own test exercised it. The craftsmanship baseline drops
its fetch_live_graph method-size entry.

Refs #724

Co-Authored-By: Claude Opus 5.5 <[email protected]>
Claude-Session: https://claude.ai/code/session_018kyZYtBmbg1yQFN984DbSe
@cdeust cdeust mentioned this pull request Nov 6, 2026
Open
6 tasks
@cdeust
cdeust marked this pull request as ready for review November 6, 2026 14:44
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Nov 6, 2026 •
edited
Loading

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-11-06T14:45:22.719861Z b8fed2f Draft marked ready
🔒 Security Review ✅ Completed 2026-11-06T14:46:37.618091Z b8fed2f Draft marked ready
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@claude

claude Bot commented Nov 6, 2026 •
edited
Loading

Copy link
Copy Markdown

Claude finished @cdeust's task in 1m 41s —— View job

PR #900 review (review 1 of 3)

  • Gather context: diff of b8fed2f and remaining viz_client references
  • Run five reviews: code quality, documentation accuracy, test coverage, security, performance
  • Check the deleted test's imports
  • Post findings with gh pr comment

Result: no Blocker found. No code, tests, scripts, workflows or configs still reference viz_client. The baseline entry had to go: if it had stayed, the craftsmanship gate would have failed on a stale entry. The deleted test imported only viz_client, so no coverage of code that still exists is lost. There is no security or performance impact.

I didn't run pytest, ruff or the craftsmanship gate. These findings come from reading the code.

The three Notes (stale ADR-1112 sentence, ADR-0623 status, audit §6 checklist) and the verdict are in the review comment.

Verdict: approve

@claude

claude Bot commented Nov 6, 2026

Copy link
Copy Markdown

Review 1 of 3: PR #900 (b8fed2f)

Five reviews ran: code quality, documentation accuracy, test coverage, security and performance. None found a Blocker.

What was traced:

  • No remaining callers. Nothing in Python code, scripts, tests, workflows, pyproject.toml or the coverage and mutation configs imports or names viz_client. The only remaining mentions are in docs and ADRs.
  • The baseline change is required. scripts/check_craftsmanship.py:95-97 rescans every file in the baseline, and stale_entries (scripts/craftsmanship_baseline.py:68-77) fails on any entry that no longer matches. If the fetch_live_graph entry had stayed, the gate would have gone red. Nothing validates the generated date.
  • No test coverage is lost. The deleted test imported only mcp_server.infrastructure.viz_client and the standard library. Owner-only permissions on the cache root are still tested in tests_py/shared/test_private_paths.py.
  • No security property is weakened. The module only read from the registry. The ADR-1112 protection comes from mcp_server/shared/private_paths.py, which never used viz_client.
  • No performance or benchmark impact. Nothing under benchmarks/ used the module.
  • No docs check fails. scripts/craftsmanship_decisions.py:74-92 checks # source: IDs and the wiki↔docs/adr mirror, not the source paths ADRs cite. This PR edits no ADR page.

Notes, no action before merge:

  • The viz_client sentence in ADR-1112 (wiki/adr/cortex/1112-personal-data-is-stored-owner-only.md:41, mirrored at docs/adr/ADR-1112-…md:43) is now outdated about behaviour, but its security claim still holds. If you change it, add a dated addendum to the wiki page and regenerate the mirror.
  • ADR-0623 still says "Keep the source implementation linked", but that source no longer exists. Marking it superseded is a decision for the owner, as the PR body says.
  • Slice 5 of §6 in docs/audits/visualization-companion-backends-2026-11-05.md:119 still reads as not done.

Verdict: approve

@github-actions
github-actions Bot merged commit e6727e9 into main Nov 6, 2026
32 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@cdeust @claude