Repository navigation
fix(skill): preserve persisted scan options across skill runs - #4250
deepanshupal wants to merge 1 commit into
Conversation
Read existing CLI build-config options in fresh and incremental skill scan fragments, keeping corpus boundaries consistent without changing standalone detection API defaults. Regenerate all host artifacts and snapshots. Add rendered-snippet regressions and explicit-override tests. Fixes Graphify-Labs#4240. AI-Assisted-By: AI coding assistant
|
Thanks for the pull request, @deepanshupal. A maintainer will review it soon. Want to talk it through while it is in review? Come join us on our Discord server. For longer-form discussion there is also GitHub Discussions. A couple of things that speed up review: make sure the test suite passes on Python 3.10 and 3.13, and that the change keeps extraction deterministic. |
![graphify-labs[bot]](https://avatars.githubusercontent.com/in/4356270?s=60&v=4){kind=small}
There was a problem hiding this comment.
Graphify reviewed this change.
Looks safe to merge — no coupling regressions and no blocking issues, checked against the code graph (not a self-assessment).
Graphify review — findings
Makes the skill templates' detect and detect_incremental steps read the build's saved exclude patterns and gitignore setting through _read_build_excludes and _read_build_gitignore against out_path(). A skill-driven run now skips the same files the CLI build does. Before, it re-scanned paths the build had already excluded.
Review partial — this diff was larger than one review pass covers, so later files were not reviewed; some findings may be missing.
Analysis details — impact, health, verification
Impact & health
Graphify review
Impact — 1228 functions depend on the 1160 functions this change touches.
Health — this change adds coupling hotspots:
- new:
render()— 13 callers, 5 callees - new:
audit_coverage()— 8 callers, 6 callees - new:
main()— 3 callers, 11 callees - new:
monolith_roundtrip()— 3 callers, 5 callees - new:
test_audit_catches_a_dropped_non_allowlisted_heading()— 0 callers, 6 callees
Verification — 1228 functions in the blast radius were not formally verified this run (proofs are advisory here).
Gate & verification
graphify gate
PASS — objectively clean (no health regressions, tests not run — proofs not run this pass (advisory)). Grounded, not self-assessed.
Advisory (not blocking):
- verification_scope: 1228 function(s) in the blast radius were not formally verified this run
Test selection
Test selection
345 of 345 test file(s) selected (100%) via static blast radius.
Escalated to a full run for safety — the selection is not trustworthy on its own (see below). CI should run the whole suite.
tests/test_affected_cli.py— full-run-safetytests/test_affected_member_seed.py— full-run-safetytests/test_agents_platform.py— full-run-safetytests/test_analyze.py— full-run-safetytests/test_anthropic_custom_endpoint.py— full-run-safetytests/test_antigravity_install.py— full-run-safetytests/test_apm_fallback_version.py— full-run-safetytests/test_architecture_doc.py— full-run-safetytests/test_astro_extraction.py— full-run-safetytests/test_astro_import_ids.py— full-run-safetytests/test_atomic_canvas_export.py— full-run-safetytests/test_atomic_version_stamp.py— full-run-safetytests/test_atomic_writes.py— full-run-safetytests/test_backend_env_isolation.py— full-run-safetytests/test_backend_extras.py— full-run-safetytests/test_benchmark.py— full-run-safetytests/test_benchmark_raw_graph.py— full-run-safetytests/test_blade_extractor.py— full-run-safetytests/test_build.py— full-run-safetytests/test_build_located_semantic_identity.py— full-run-safetytests/test_build_merge_dedup_scope.py— full-run-safetytests/test_build_merge_hyperedges_and_prune.py— full-run-safetytests/test_build_merge_shrink_guard.py— full-run-safetytests/test_builtin_global_type_refs.py— full-run-safetytests/test_cache.py— full-run-safetytests/test_cache_stale_import_target.py— full-run-safetytests/test_callflow_html.py— full-run-safetytests/test_cargo_introspect.py— full-run-safetytests/test_cargo_missing_manifest.py— full-run-safetytests/test_carried_hyperedge_remap.py— full-run-safetytests/test_case_sensitive_resolution.py— full-run-safetytests/test_charmap_encoding.py— full-run-safetytests/test_chunking.py— full-run-safetytests/test_cjs_module_extension.py— full-run-safetytests/test_claude_cli_backend.py— full-run-safetytests/test_claude_md.py— full-run-safetytests/test_cli_broken_pipe.py— full-run-safetytests/test_cli_export.py— full-run-safetytests/test_cli_help.py— full-run-safetytests/test_cluster.py— full-run-safetytests/test_cluster_exclude_hubs.py— full-run-safetytests/test_cobol_extractor.py— full-run-safetytests/test_codebuddy.py— full-run-safetytests/test_community_hub_labels.py— full-run-safetytests/test_community_labels_skill.py— full-run-safetytests/test_confidence.py— full-run-safetytests/test_corrupt_graph_json.py— full-run-safetytests/test_cpp_method_declarations.py— full-run-safetytests/test_cpp_nested_and_cli.py— full-run-safetytests/test_cpp_objc_cross_file_calls.py— full-run-safety- … and 295 more
non-code file(s) changed (
graphify/skill-agents.md,graphify/skill-aider.md,graphify/skill-amp.md,graphify/skill-claw.md,graphify/skill-codex.md…) → running the full suite for safety (a code graph can't see config/fixture/data deps)
changed code file(s) with no mapped test (
graphify/skill-agents.md,graphify/skill-aider.md,graphify/skill-amp.md,graphify/skill-claw.md,graphify/skill-codex.md…) — a coverage gap or a missing link — running the full suite rather than only the selected tests
Selection is safe under the controlled-regression assumption; always-run tests + a periodic full run are the backstops. Advisory — it never changes the check verdict.
· 5 more finding(s) on lines outside this diff (see the check run).
|
Landed in v0.9.83 via an authorship-preserving cherry-pick, so your commit is on |
2 participants
What does this PR do?
Fixes #4240. Skill full/incremental detection now reads the persisted exclusion and gitignore options from the same output config that CLI rebuilds use. This keeps opted-in gitignored files in the graph and avoids reintroducing explicitly excluded files.
The change is in source fragments, with regenerated skill artifacts and snapshots.
detect()/detect_incremental()defaults are unchanged, so a direct caller's explicit options still win.Type of change
Verification & Invariants
Persisted corpus boundaries must agree across CLI and skill runs. A fixture with ignored
notes/a.mdand excludeddocs/skip.mdfails on the old rendered snippets and passes with this change. The regression runs fresh/incremental scan snippets from 31 host artifacts, tests missing/malformed config defaults, and checks that the direct detection API does not override explicit caller options.How was this tested?
Python 3.10, isolated environment. All test files run serially in two halves: 6,444 passed, 272 skipped. The optional dependencies behind the skips were not installed; this is not an all-extras CI-matrix claim.
pytest tests/ -q --tb=short(split by test file into two serial runs)ruff check .python -m tools.skillgen --checkpython -m tools.skillgen --audit-coveragepython -m tools.skillgen --schema-singletonpython -m tools.skillgen --monolith-roundtrippython -m tools.skillgen --always-on-roundtripgit diff --checkPyright completed on both this tree and an untouched HEAD worktree in the same environment. Both produced the same 638 errors/4 warnings, with no new or removed diagnostics. This is a baseline comparison, not a clean type-check result.
Graphify-specific checklist
No real Windows execution was performed. The scan snippets use existing platform-neutral path helpers and config readers; cross-platform CI remains necessary.