Improve dev workflow, lint baseline, refactor CLI, and gate CI#1183
Open
morus246 wants to merge 10 commits into
Open
Improve dev workflow, lint baseline, refactor CLI, and gate CI#1183morus246 wants to merge 10 commits into
morus246 wants to merge 10 commits into
Conversation
When no .graphifyignore exists, .gitignore patterns must apply — the upstream landed this as part of safishamsi#945 (commit 9e6192a) but no test guarded it. Without this test, a future refactor could silently re-introduce the gap and cause docs/translations/ to re-enter the graph. The 27 README translations under docs/translations/ duplicate the primary README in graphify's nav; ignoring them keeps god nodes focused on real code abstractions.
Two long-standing pollution sources the upstream .gitignore missed: - docs/translations/ — 27 README translations that duplicate the primary README in graphify's nav. Adds 548 nodes / 0 signal. - .hermes/ — local planning artifacts from Claude Code / Hermes runs. Adds ~40 nodes / 0 signal and leaks session content into the architecture graph. Validation: graphify update . went from 9648 -> 7362 nodes (-23.7%), 16273 -> 12192 edges (-25%), 743 -> 574 communities (-22%).
…llers
graphify/__main__.py had grown to 4582 lines, mostly because every
supported AI platform (Claude Code, Gemini, VSCode, Kiro, Antigravity,
Cursor, Devin, Kilo, OpenCode, Codex, Amp, CodeBuddy) ships its own
install/uninstall code path. This is the Phase 5.1 refactor the long
plan called for: move the installer block (lines 1-2069 + _clone_repo)
out into graphify/cli/installers.py.
Why this split is safe:
1. Backward compatibility preserved by re-exporting every installer
name as a module attribute on __main__ (set via sys.modules), so
existing imports like `from graphify.__main__ import claude_install`
and `from graphify.__main__ import _CLAUDE_MD_SECTION` keep working.
2. The PEP 562 `_ALWAYS_ON_ALIASES` lazy block (which doesn't show up
in dir()) is also bound to the __main__ module, so install-string
tests pass without modification.
3. All Path(__file__).parent references in installers.py are bumped
to .parent.parent so packaged assets (always_on/*.md, skills/*,
command-kilo.md, skill-{vscode,copilot}.md) keep resolving from
graphify/ rather than the new graphify/cli/ location.
Results:
- __main__.py: 4582 -> 2617 lines (-43%, 1965 LOC out)
- New graphify/cli/{__init__.py,installers.py}: 2090 lines
- pytest: 1968 passed, 1 skipped (no regressions)
- ruff: same 1 pre-existing F821 in prs.py, zero new errors
- graphify --version / --help / test_install_* / test_claude_md /
test_install_roundtrip all pass
Closes Phase 6 of the long plan. - New `lint` job runs `ruff check .` in parallel with the existing `skillgen-check` and `test` jobs, so a future ruff regression fails the PR without waiting for the slower test job. Uses uv + ruff via pip (`uv pip install --system ruff`) rather than the dev extra to keep the job tight. - New `Build package` step in the `test` job runs `uv run --frozen python -m build` after the tests. Catches packaging regressions (broken MANIFEST.in, missing data files, setuptools deprecation warnings becoming errors) that the test suite never touches. Wheel build is sub-second on this repo. - New `Smoke-test detect` step invokes the core file-classification / .graphifyignore pipeline via the Python API (there is no `graphify detect` subcommand) and asserts the scan finds at least one code file. Non-mutating, runs in <1s, and guards the same code path that every `graphify update` depends on.
Documents the same two CI gates (ruff, build) as locally-runnable commands so contributors don't have to wait on CI to find out they tripped the lint or packaging checks. Closes Phase 7.1 of the long plan. The PyPI / CLI naming note (`graphifyy` on PyPI vs `graphify` as the CLI command) was already on line 80 of the v8 README, so 7.2 ships as-is from upstream.
Pins Phase 8.2 of the long plan: a happy-path test that exercises the named-ListObject code path and emits real column-header nodes, and a static-analysis test that catches a re-introduction of the F-035 bug (the `_re.sub` typo from commit 67eb547's history). The happy-path test uses a named table because the no-table fallback in xlsx_extract_structure is dead code in practice — every openpyxl Worksheet has a `tables` attribute (empty dict), so the `if hasattr(ws, "tables")` check always succeeds and the `else` branch never runs. Building a named ListObject exercises the header-emitting code path for real. The F-035 invariant reads detect.py and asserts `_re.sub` does not appear in executable code inside xlsx_extract_structure's body (comment-stripped, so the F-035 history comment that legitimately mentions the typo doesn't trip the assertion). Skips the test gracefully on lean installs via `pytest.importorskip('openpyxl')`.
Phase 8.1 of the long plan. Bandit is already declared in `[dependency-groups].dev` and configured under `[tool.bandit]`, but no workflow was running it. Wired into the existing `lint` job with `|| true` for now: bandit currently reports 5 Medium-confidence findings (3× B104 hardcoded_bind_all_interfaces in serve.py / mcp_ingest.py, 2× B314 xml.etree.ElementTree.fromstring in docx_to_markdown / xlsx_extract_structure) — all pre-existing in v8, none introduced by this branch. The TODO in the step body lists them so a follow-up commit can either fix the issues or annotate them with `# nosec` before the `|| true` is dropped.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
End-to-end polish on the
v8development loop. 10 commits, no upstream churn — every commit is additive or a safe refactor.What's in
.github/workflows/ci.yml): newlintjob runningruff check .in parallel with the existingskillgen-checkandtestjobs. NewBuild packagestep (uv run --frozen python -m build) andSmoke-test detectstep in the test job. New advisorybanditstep (with a TODO to make it blocking once the 5 pre-existing medium-confidence findings are triaged)..gitignore): excludesdocs/translations/(495 nodes of README duplicates) and.hermes/(40 nodes of local planning artefacts). Combined with the existingdocs/superpowers/rule, the graph dropped from 9648 → 7362 nodes (−23.7%), 16273 → 12192 edges (−25%), 743 → 574 communities (−22%) on a clean re-run.graphify/cli/installers.py+ slimmer__main__.py): moves lines 1-2069 of__main__.py(every per-platform installer plus the cross-platform helpers) into a newgraphify.cli.installerssubmodule.__main__.pydrops from 4582 → 2617 LOC (−43%) while keeping the full public API re-exported (incl. PEP 562_ALWAYS_ON_ALIASESlazy attributes).test_detect_falls_back_to_gitignore_for_translationspins the.gitignorefallback added in upstream commit 9e6192a (Feature: fall back to .gitignore when .graphifyignore is absent #945) against future regressions.test_xlsx_extract_structure_*(2 tests) cover the F-035_re.subtypo fix from commit 67eb547, plus a happy-path test for the named-ListObject code path.README.md): new "Lint and packaging" subsection under the existing "Development" section documenting the local commands that match the new CI gates. The PyPI vs CLI naming note (graphifyyvsgraphify) was already present on line 80 of the v8 README, so 7.2 ships as-is from upstream.v7baseline (3 of 4 commits): theCLAUDE.mdguidance, the ruff-discovered extraction fixes, and the post-rebase import style fix. The 4th commit (dev-extras) was skipped because v8 already ships a strictly better setup (PEP 735[dependency-groups].dev+uv sync --all-extras --frozen).What's not in
ruff F821(nxundefined ingraphify/prs.py:243, commitcc9e5816) is not in scope of this branch. It still fires locally and on CI; the lint job is expected to fail until that one-line fix lands upstream.banditMedium findings (B104 hardcoded_bind_all_interfaces×3,B314 xml.etree.ElementTree.fromstring×2) are documented in the newbandit (advisory)step as a follow-up TODO before the gate becomes blocking.Validation
uv run --group dev ruff check .→ 1 pre-existing error, no new onesuv run --group dev --extra mcp --extra pdf --extra watch --extra sql --extra office pytest tests/ -q→ 1970 passed, 1 skipped (v8 itself has 1968; +2 from the new xlsx regression tests)uv run --group dev python -m build→ buildsgraphifyy-0.8.35.tar.gz+.whlgraphify update .→ 7362 nodes / 12192 edges / 574 communities (down 23.7% / 25% / 22% vs unmodified v8)graphify --version/graphify --help/graphify installall passNotes for the reviewer
__main__.py→cli/installers.pymove preserves 100% of the public API: every install/uninstall entry point, every_ALWAYS_ON_*lazy attribute, every_PLATFORM_CONFIG/_clone_repo/ etc. helper is re-exported as a module attribute ongraphify.__main__so existing imports (from graphify.__main__ import claude_install,from graphify.__main__ import _CLAUDE_MD_SECTION, etc.) keep working unchanged.sys.modules[__name__]rather than just populating module globals, because tests liketest_install_strings.pyimport the private_FOOnames directly withfrom graphify.__main__ import _FOO— that form reads module attributes, not function-local globals.Path(__file__).parentreferences inside the extractedinstallers.pyare bumped to.parent.parentso the packaged assets (always_on/*.md,skills/*,command-kilo.md,skill-{vscode,copilot}.md) keep resolving from thegraphify/root instead ofgraphify/cli/.