Skip to content

Stabilize import-graph cycle canonicalization#497

Open
hiroshitanaka-creator wants to merge 1 commit intomainfrom
codex/implement-phase-4-cleanup-and-refactor
Open

Stabilize import-graph cycle canonicalization#497
hiroshitanaka-creator wants to merge 1 commit intomainfrom
codex/implement-phase-4-cleanup-and-refactor

Conversation

@hiroshitanaka-creator
Copy link
Copy Markdown
Owner

Motivation

  • import グラフ出力で同一のサイクルが回転した表現で複数行として報告される問題を解消し、ツールの出力を決定的にするための Phase 4 tooling cleanup を行いました。
  • この修正はインポート境界や runtime の振る舞いを変えず、import-guard の安定化と回帰保守性を高めることを目的としています。

Description

  • tools/import_graph.py_canonical_cycle() を追加し、find_cycles() が回転不変なキーでサイクルを正規化して重複報告を潰すようにしました。
  • tests/test_import_graph_tool.py を新規追加して、サイクル正規化の回転不変性、TYPE_CHECKING ブロック内の import 除外、重複サイクルの畳み込みを回帰テストで固定しました。
  • docs/status.md の Completed/Next を運用ルールに従って更新し、本作業(Phase 4 tooling cleanup)を記録しました。

Testing

  • black --check src tests tools/import_graph.py は当初未整形で失敗しましたが、整形後はチェックに合格しました。
  • isort --check-only src tests tools/import_graph.py は当初 import order エラーが出ましたが修正後は合格しました。
  • mypy src/po_core/domain/ src/po_core/experiments/ src/po_core/app/ src/po_core/ports/ は成功しました。
  • python tools/import_graph.py --check --print は実行で violations/cycles 共に 0 を報告し成功しました。
  • pytest tests/test_import_graph_tool.py -v および pytest tests/test_release_readiness.py -v は両方とも合格しました。
  • UNVERIFIED: リポジトリ全体の pytest -q(フルスイート)、security / build / installed-artifact smoke は本 PR で再実行していません。

Codex Task

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 3548d693cb

ℹ️ 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".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread tools/import_graph.py
Comment on lines +246 to +250
for cycle in cycles:
key = _canonical_cycle(cycle)
if key not in seen:
seen.add(key)
uniq.append(c)
uniq.append(list(key))
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Sort returned cycles after canonicalization

find_cycles() now normalizes each individual cycle, but it still returns them in raw DFS discovery order. Because the DFS walks Set[str] dependencies, the list of cycles is still hash-seed dependent: with a graph like {'a': {'b', 'c'}, 'b': {'a'}, 'c': {'a'}}, different PYTHONHASHSEED values produce [a→b→a, a→c→a] or the reverse order. That means python tools/import_graph.py --print is still non-deterministic across runs/filesystems even after this change, which undermines the stated stabilization goal.

Useful? React with 👍 / 👎.

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.

1 participant