Repository navigation
skill: tell the user graphify-out/ is in their working tree (#3941) - #4308
eoinforker wants to merge 1 commit into
Conversation
Step 9's report never mentioned that graphify-out/ was written into the working tree, so a repository that does not ignore it was left with untracked machine-local files in git status and no explanation. The report now adds one sentence in a git repository, with the two ways to handle it. The note goes in the three core fragments (core, aider, devin); the devin monolith change is sanctioned by a new round-trip predicate.
|
Thanks for the pull request, @eoinforker. 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. |
|
Thanks @eoinforker! One small thing: our changelog and release notes are built from PR titles, so we use the conventional-commits format, for example: Prefixes we use: |
There was a problem hiding this comment.
Graphify reviewed this change.
No issues found by static checks — no coupling regressions or blocking issues in the code graph. That is not the same as safe to merge: see what was not checked.
Not checked: tests were not run; no formal proof of the changed code.
Graphify review — findings
Adds a step to every platform's skill instructions that has the agent warn, when the processed directory is inside a git repo, that graphify-out/ lands in the working tree and will show up in git status unless it's already ignored. The warning also points the user to add it to .gitignore or commit only shared outputs, linking the team-workflows guide.
No blocking issues surfaced.
Analysis details — impact, health, verification
Impact & health
Graphify review
Impact — 1111 functions depend on the 1040 functions this change touches.
Health — grade A; 5 existing hotspot(s) in the area this change touches (pre-existing, not introduced here):
render()— 13 callers, 5 callees (high)audit_coverage()— 8 callers, 6 callees (high)main()— 3 callers, 11 callees (medium)monolith_roundtrip()— 3 callers, 5 callees (medium)test_audit_catches_a_dropped_non_allowlisted_heading()— 0 callers, 6 callees (medium)
Verification — 1111 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: 1111 function(s) in the blast radius were not formally verified this run
Test selection
Test selection
361 of 361 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_cloud_cta.py— full-run-safetytests/test_cluster.py— full-run-safetytests/test_cluster_ambiguous_scale.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-safety- … and 311 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.
|
Closing in favour of #3977, which got here first and covers this more thoroughly. Thanks @azizur100389! |
Fixes #3941.
Step 9's report describes the graph but never says that
graphify-out/now exists in the user's working tree. In a repository that does not already ignore it, the run leaves machine-local files ingit statuswith no explanation.Change
One sentence added to the Step 9 report instructions, after the PATH_TO_DIR line:
tools/skillgen/fragments/core/{core,aider,devin}.md), then regenerated withpython -m tools.skillgen --blessandpython -m tools.skillgen. No generated file was hand-edited._is_worktree_visibility_note_lineto_SANCTIONED_MONOLITH_DIFFS, matching only this sentence. If you would rather keep devin untouched, I can drop that file and the predicate.graphify-out/.gitignore; that is Question — manifest.json absolute paths + missing graphify-out/.gitignore: intended or oversight? #722's decision.Checks
python -m tools.skillgen --check,--audit-coverage,--schema-singleton,--monolith-roundtrip,--always-on-roundtrip: all OKruff check tools/skillgen/gen.py: cleanpytest tests/test_skillgen.py tests/test_skill_*.py tests/test_community_labels_skill.py tests/test_agents_platform.py: 183 passed