GitHub Actions YAML extraction, ci-select, metadata.json contract doc - #3087
GitHub Actions YAML extraction, ci-select, metadata.json contract doc#3087eliorerz wants to merge 6 commits into
Conversation
There was a problem hiding this comment.
Graphify reviewed this change.
Worth a look — the grounded gate found no coupling regressions or blocking issues, but 5 advisory finding(s) below merit a look before merge.
Formal verification. 1 change(s) alter behavior, breaking input(s) attached.
Behavior changes: \_obsidian\_tag changes behavior, here is the input that shows it.
The verifier found a concrete input on which \_obsidian\_tag behaves differently before and after the change. If that change is intended, ship it; if not, this is your bug.
Guarantee: This difference was REPRODUCED, the verifier actually ran both versions on that input and saw them disagree. It is real, not an artifact.
Evidence: On input \{"name":"'h\\u00e9llo w\\u00f6rld'"\}, the old code produced 'héllo\_wörld' but the new code produces 'hllo\_wrld'. Paste that input straight into a regression test.
Graphify review — findings
Adds a graphify ci-select subcommand that picks CI test jobs from a diff by walking the knowledge graph, documented via new --repo/--diff/--graph/--depth help entries in the CLI. Documents the published-bundle contract in a new docs/graph-bundle-metadata.md plus a draft-2020-12 JSON Schema, establishing this repo as the single source of truth for the metadata.json writer and reader that live in another repo. Widens the corrupt-cache-entry catch in load_cached to also swallow UnicodeDecodeError so a truncated write that cuts a multi-byte UTF-8 char counts as corrupt rather than being re-extracted forever, and clarifies in ARCHITECTURE.md that extract() has its own process-level side effects (recursion limit, cache clears, stderr warnings) invisible to other stages.
Worth a look
- User-controlled --diff-cmd is executed through the shell —
graphify/ci_select.py· Escalate · high- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
- Repo-relative source files with directories are treated as cross-repo —
graphify/ci_select.py:352· Escalate · high- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
- check_semantic_cache does not catch UnicodeDecodeError on corrupt entry —
graphify/cache.py· Escalate · medium- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
- ci-select ignores failed diff command exit status —
graphify/ci_select.py· Escalate · medium- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
- Timed-out diff command can leave child processes running —
graphify/ci_select.py· Escalate · medium- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
Analysis details — impact, health, verification
Impact & health
Graphify review
Impact — 2606 functions depend on the 883 functions this change touches.
Health — this change adds coupling hotspots:
- new:
extract()— 458 callers, 41 callees - new:
_rebuild_code()— 95 callers, 51 callees - new:
detect()— 93 callers, 14 callees - new:
build_merge()— 41 callers, 14 callees - new:
save_semantic_cache()— 52 callers, 9 callees - new:
save_manifest()— 34 callers, 11 callees - new:
to_obsidian()— 29 callers, 12 callees - new:
extract_files_direct()— 17 callers, 20 callees - …and 55 more — each is listed as a finding
Verification — 2606 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: 2574 function(s) in the blast radius were not formally verified this run
Formal verification
Behavior changes: \_obsidian\_tag changes behavior, here is the input that shows it.
The verifier found a concrete input on which \_obsidian\_tag behaves differently before and after the change. If that change is intended, ship it; if not, this is your bug.
Guarantee: This difference was REPRODUCED, the verifier actually ran both versions on that input and saw them disagree. It is real, not an artifact.
Evidence: On input \{"name":"'h\\u00e9llo w\\u00f6rld'"\}, the old code produced 'héllo\_wörld' but the new code produces 'hllo\_wrld'. Paste that input straight into a regression test.
Could not verify: Could not verify \_as\_repo\_relative.
The verifier did not have enough to check \_as\_repo\_relative, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: parameter `root` is annotated `Path | None` — outside the synthesizable primitive/collection set
No difference found (not proven): No behavior difference found in format\_affected (not a proof).
The verifier ran both versions of format\_affected on many inputs and saw identical behavior every time. Strong evidence the change is safe, but evidence, not a proof.
Guarantee: Empirical: differential testing (both versions run on many generated inputs). A divergence on an untested input remains possible, so this is 'no counterexample found', not 'proven equivalent'.
Note: An input the sampler did not try could still differ.
Could not verify: Could not verify resolve\_seed.
The verifier did not have enough to check resolve\_seed, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: parameter `root` is annotated `Path | None` — outside the synthesizable primitive/collection set
Could not verify: Could not verify build.
The verifier did not have enough to check build, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: not verifiable: all 9 sampled inputs raised on both versions — the function never executed, so 'no divergence' would be vacuous (mostly AttributeError — names the real obstacle, not a sampling gap)
Could not verify: Could not verify build\_from\_json.
The verifier did not have enough to check build\_from\_json, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: not verifiable: all 6 sampled inputs raised on both versions — the function never executed, so 'no divergence' would be vacuous (mostly NameError — names the real obstacle, not a sampling gap)
Could not verify: Could not verify build\_merge.
The verifier did not have enough to check build\_merge, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: parameter `graph_path` is annotated `str | Path | None` — outside the synthesizable primitive/collection set
No difference found (not proven): No behavior difference found in \_fold\_edge\_aliases (not a proof).
The verifier ran both versions of \_fold\_edge\_aliases on many inputs and saw identical behavior every time. Strong evidence the change is safe, but evidence, not a proof.
Guarantee: Empirical: differential testing (both versions run on many generated inputs). A divergence on an untested input remains possible, so this is 'no counterexample found', not 'proven equivalent'.
Note: An input the sampler did not try could still differ.
Could not verify: Could not verify cache\_dir.
The verifier did not have enough to check cache\_dir, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: parameter `root` is annotated `Path` — outside the synthesizable primitive/collection set
Could not verify: Could not verify file\_hash.
The verifier did not have enough to check file\_hash, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: parameter `path` is annotated `Path` — outside the synthesizable primitive/collection set
Could not verify: Could not verify load\_cached.
The verifier did not have enough to check load\_cached, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: parameter `path` is annotated `Path` — outside the synthesizable primitive/collection set
Could not verify: Could not verify save\_semantic\_cache.
The verifier did not have enough to check save\_semantic\_cache, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: parameter `root` is annotated `Path` — outside the synthesizable primitive/collection set
Could not verify: Could not verify dispatch\_command.
The verifier did not have enough to check dispatch\_command, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: not verifiable: all 23 sampled inputs raised on both versions — the function never executed, so 'no divergence' would be vacuous (mostly SystemExit — names the real obstacle, not a sampling gap)
No difference found (not proven): No behavior difference found in \_run\_hook\_guard (not a proof).
The verifier ran both versions of \_run\_hook\_guard on many inputs and saw identical behavior every time. Strong evidence the change is safe, but evidence, not a proof.
Guarantee: Empirical: differential testing (both versions run on many generated inputs). A divergence on an untested input remains possible, so this is 'no counterexample found', not 'proven equivalent'.
Note: An input the sampler did not try could still differ.
Could not verify: Could not verify \_stamped\_manifest\_files.
The verifier did not have enough to check \_stamped\_manifest\_files, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: parameter `root` is annotated `Path` — outside the synthesizable primitive/collection set
Could not verify: Could not verify deduplicate\_entities.
The verifier did not have enough to check deduplicate\_entities, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: not verifiable: all 9 sampled inputs raised on both versions — the function never executed, so 'no divergence' would be vacuous (mostly TypeError — names the real obstacle, not a sampling gap)
Could not verify: Could not verify classify\_file.
The verifier did not have enough to check classify\_file, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: parameter `path` is annotated `Path` — outside the synthesizable primitive/collection set
Could not verify: Could not verify detect.
The verifier did not have enough to check detect, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: parameter `root` is annotated `Path` — outside the synthesizable primitive/collection set
Could not verify: Could not verify ignored\_predicate.
The verifier did not have enough to check ignored\_predicate, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: parameter `root` is annotated `Path` — outside the synthesizable primitive/collection set
Could not verify: Could not verify \_load\_dir\_own\_ignore.
The verifier did not have enough to check \_load\_dir\_own\_ignore, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: parameter `d` is annotated `Path` — outside the synthesizable primitive/collection set
Could not verify: Could not verify \_load\_graphifyignore.
The verifier did not have enough to check \_load\_graphifyignore, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: parameter `root` is annotated `Path` — outside the synthesizable primitive/collection set
No difference found (not proven): No behavior difference found in save\_manifest (not a proof).
The verifier ran both versions of save\_manifest on many inputs and saw identical behavior every time. Strong evidence the change is safe, but evidence, not a proof.
Guarantee: Empirical: differential testing (both versions run on many generated inputs). A divergence on an untested input remains possible, so this is 'no counterexample found', not 'proven equivalent'.
Note: An input the sampler did not try could still differ.
No difference found (not proven): No behavior difference found in attach\_hyperedges (not a proof).
The verifier ran both versions of attach\_hyperedges on many inputs and saw identical behavior every time. Strong evidence the change is safe, but evidence, not a proof.
Guarantee: Empirical: differential testing (both versions run on many generated inputs). A divergence on an untested input remains possible, so this is 'no counterexample found', not 'proven equivalent'.
Note: An input the sampler did not try could still differ.
Could not verify: Could not verify \_git\_head.
The verifier did not have enough to check \_git\_head, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: parameter `cwd` is annotated `'str | Path | None'` — outside the synthesizable primitive/collection set
No difference found (not proven): No behavior difference found in \_obsidian\_safe\_stem (not a proof).
The verifier ran both versions of \_obsidian\_safe\_stem on many inputs and saw identical behavior every time. Strong evidence the change is safe, but evidence, not a proof.
Guarantee: Empirical: differential testing (both versions run on many generated inputs). A divergence on an untested input remains possible, so this is 'no counterexample found', not 'proven equivalent'.
Note: An input the sampler did not try could still differ.
· 4 grounded finding(s) anchored inline below; 59 more finding(s) on lines outside this diff (see the check run).
| return ", ".join(labels) if labels else "(no connections)" | ||
|
|
||
|
|
||
| def ci_select( |
There was a problem hiding this comment.
ci_select()
fans out to 8 callees (efferent coupling); 8 callers depend on it (afferent coupling).
Grounded coupling-delta finding (deterministic), not an LLM guess.
| return plan | ||
|
|
||
|
|
||
| def cli_main(argv: list[str] | None = None) -> None: |
There was a problem hiding this comment.
cli_main()
high coupling complexity (Ca·Ce = 20).
Grounded coupling-delta finding (deterministic), not an LLM guess.
| @@ -4989,6 +4994,17 @@ def _is_cpp_header(path: Path) -> bool: | |||
|
|
|||
| def _get_extractor(path: Path) -> Any | None: | |||
There was a problem hiding this comment.
_get_extractor()
fans out to 8 callees (efferent coupling); 29 callers depend on it (afferent coupling).
Grounded coupling-delta finding (deterministic), not an LLM guess.
| return "jobs" in keys and ("on" in keys or is_github_actions_workflow_path(path)) | ||
|
|
||
|
|
||
| def extract_github_actions(path: Path) -> dict: |
There was a problem hiding this comment.
extract_github_actions()
fans out to 10 callees (efferent coupling); 16 callers depend on it (afferent coupling).
Grounded coupling-delta finding (deterministic), not an LLM guess.
4b59ebb to
14adeac
Compare
… CI test selection Add a new subcommand that uses the knowledge graph to determine which tests need to run based on changed files. Supports cross-repository test selection when changes in one repo affect dependencies in another repo.
Add a JSON Schema for the metadata.json file that accompanies published graph bundles. This schema serves as the single source of truth for both the CI job that generates the bundle and the fetch script that consumes it, preventing drift between writer and reader implementations. The schema defines fields for version tracking (source_sha, graphify_version), staleness detection (generated_at), and bundle file paths (graph.json, GRAPH_REPORT.md, manifest.json).
- Add looks_like_workflow_shape() content check to prevent non-workflow files at workflow paths from being misclassified - Fix UnicodeDecodeError handling in cache corruption detection - Harden install_references write probe for root/elevated contexts - Gate test platform capabilities (mkfifo, AF_UNIX, symlinks) on actual availability - Fix whitespace and comment clarity throughout These are real bug fixes caught in code review, not just style improvements.
… bugs - Move workflow-shape gate to top of _get_extractor() before manifest checks - Fix comment-before-value parsing in block sequence items (prevents comments from being read as dependency names) - Consolidate .yaml/.yml dispatch logic
14adeac to
e398e7b
Compare
1. Check diff command exit status and report failures (was silently using empty output) 2. Kill timed-out diff subprocess to prevent orphaned processes 3. Fix repo-relative path detection - paths like 'internal/servers/file.go' were incorrectly split as repo='internal', file='servers/file.go'. Now only treats single-component prefixes (no slashes) as repo names. 4. Add inline security note that --diff-cmd executes through shell Addresses findings from PR Graphify-Labs#3087 review: - 'ci-select ignores failed diff command exit status' - 'Timed-out diff command can leave child processes running' - 'Repo-relative source files with directories are treated as cross-repo' - 'User-controlled --diff-cmd is executed through the shell' (documented, by design)
…lect - graphify ci-select subcommand for graph-informed CI test selection - GitHub Actions workflow YAML extraction (jobs/needs/uses) - metadata.json schema for published graph bundles PRs: Graphify-Labs#3087 (upstream), #5 (fork)
There was a problem hiding this comment.
Graphify reviewed this change.
Worth a look — the grounded gate found no coupling regressions or blocking issues, but 5 advisory finding(s) below merit a look before merge.
Formal verification. 1 change(s) tested, no difference found (not proven).
Graphify review — findings
Adds a documented metadata.json bundle contract (docs/graph-bundle-metadata.md plus a draft-2020-12 JSON Schema) defining schema_version, source_sha, graphify_version, generated_at, and bundle file paths, so the CI publisher and the fetch/reader script that pull a graphify --update output validate against one source of truth. Introduces a ci-select CLI command for graph-informed CI test selection from a diff, wired into _run_cli help with --repo, --diff-cmd/--diff/--files, --graph, --test-jobs, and --depth options. Treats a UnicodeDecodeError from load_cached reading a truncated cache entry as a corrupt (counted, re-extracted) entry rather than crashing.
Worth a look
- Repo-relative subdirectory files are misclassified as cross-repo —
graphify/ci_select.py· Escalate · high- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
- Full-suite fallback returns no jobs —
graphify/ci_select.py:314· Escalate · high- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
- Timed-out shell diff commands can leave descendant processes running —
graphify/ci_select.py· Escalate · medium- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
- Fallback YAML parser drops list values —
graphify/ci_select.py:159· Escalate · medium- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
- File lookup can seed nodes from the wrong repository —
graphify/ci_select.py:186· Escalate · medium- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
Analysis details — impact, health, verification
Impact & health
Graphify review
Impact — 2959 functions depend on the 927 functions this change touches.
Health — this change adds coupling hotspots:
- new:
extract()— 494 callers, 42 callees - new:
_rebuild_code()— 98 callers, 50 callees - new:
detect()— 108 callers, 15 callees - new:
build_merge()— 46 callers, 14 callees - new:
to_obsidian()— 36 callers, 13 callees - new:
save_semantic_cache()— 52 callers, 9 callees - new:
save_manifest()— 40 callers, 11 callees - new:
to_json()— 50 callers, 7 callees - …and 60 more — each is listed as a finding
Verification — 2959 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: 2886 function(s) in the blast radius were not formally verified this run
Formal verification
Could not verify: Could not verify load\_cached.
The verifier did not have enough to check load\_cached, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: parameter `path` is annotated `Path` — outside the synthesizable primitive/collection set
Could not verify: Could not verify dispatch\_command.
The verifier did not have enough to check dispatch\_command, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: not verifiable: all 23 sampled inputs raised on both versions — the function never executed, so 'no divergence' would be vacuous (mostly SystemExit — names the real obstacle, not a sampling gap)
Could not verify: Could not verify classify\_file.
The verifier did not have enough to check classify\_file, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: parameter `path` is annotated `Path` — outside the synthesizable primitive/collection set
Could not verify: Could not verify \_git\_head.
The verifier did not have enough to check \_git\_head, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: parameter `cwd` is annotated `'str | Path | None'` — outside the synthesizable primitive/collection set
Could not verify: Could not verify \_get\_extractor.
The verifier did not have enough to check \_get\_extractor, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: parameter `path` is annotated `Path` — outside the synthesizable primitive/collection set
No difference found (not proven): No behavior difference found in \_run\_cli (not a proof).
The verifier ran both versions of \_run\_cli on many inputs and saw identical behavior every time. Strong evidence the change is safe, but evidence, not a proof.
Guarantee: Empirical: differential testing (both versions run on many generated inputs). A divergence on an untested input remains possible, so this is 'no counterexample found', not 'proven equivalent'.
Note: An input the sampler did not try could still differ.
· 4 grounded finding(s) anchored inline below; 64 more finding(s) on lines outside this diff (see the check run).
| return ", ".join(labels) if labels else "(no connections)" | ||
|
|
||
|
|
||
| def ci_select( |
There was a problem hiding this comment.
ci_select()
fans out to 8 callees (efferent coupling); 8 callers depend on it (afferent coupling).
Grounded coupling-delta finding (deterministic), not an LLM guess.
| return plan | ||
|
|
||
|
|
||
| def cli_main(argv: list[str] | None = None) -> None: |
There was a problem hiding this comment.
cli_main()
high coupling complexity (Ca·Ce = 20).
Grounded coupling-delta finding (deterministic), not an LLM guess.
| @@ -5375,6 +5380,17 @@ def _is_cpp_header(path: Path) -> bool: | |||
|
|
|||
| def _get_extractor(path: Path) -> Any | None: | |||
There was a problem hiding this comment.
_get_extractor()
fans out to 8 callees (efferent coupling); 29 callers depend on it (afferent coupling).
Grounded coupling-delta finding (deterministic), not an LLM guess.
| return "jobs" in keys and ("on" in keys or is_github_actions_workflow_path(path)) | ||
|
|
||
|
|
||
| def extract_github_actions(path: Path) -> dict: |
There was a problem hiding this comment.
extract_github_actions()
fans out to 10 callees (efferent coupling); 16 callers depend on it (afferent coupling).
Grounded coupling-delta finding (deterministic), not an LLM guess.
Fixes confirmed broken code from automated review of PR Graphify-Labs#3087: 1. **Subdirectory misclassification (CONFIRMED BUG)** - Issue: Same-repo files like "internal/servers/file.go" wrongly classified as cross-repo to a nonexistent repo named "internal" - Root cause: Tautological check `"/" not in potential_repo` was always true since potential_repo is extracted before the first slash - Fix: Build set of known repos from test-jobs.yaml + graph analysis; only treat prefixes as cross-repo if they match known repos - Test: TestBugFixes::test_subdirectory_not_misclassified_as_cross_repo 2. **Full-suite fallback returns no jobs (CONFIRMED BUG)** - Issue: When no changed files match graph nodes, returned empty must_run list despite reasoning "Falling back to full test suite" - Fix: Load all jobs from test-jobs.yaml and add to must_run before early return - Test: TestBugFixes::test_full_suite_fallback_schedules_all_jobs 3. **Timed-out shell diff commands leave descendants running (CONFIRMED BUG)** - Issue: TimeoutExpired exception doesn't expose process handle; ffc72c3's fix tried to call .kill() on cmd string (has no such method) - Fix: Use subprocess.Popen with os.setsid() for process group management; properly kill process group on timeout - Note: shell=True with user-controlled --diff-cmd is intentional (local CLI tool, no injection risk) 4. **Fallback YAML parser drops list values (CONFIRMED BUG)** - Issue: When parsing "key:\n - item1\n - item2", created empty dict for key, then loop over parent.keys() found nothing, dropped items - Fix: When encountering list item with empty-dict parent, replace dict with list in grandparent - Test: TestBugFixes::test_yaml_parser_preserves_list_values All fixes verified against real code (not eyeballed) with reproduction tests. Existing test suite (26 tests) still passes. 3 new regression tests added. Supersedes incomplete ffc72c3 fix commit.
Fixes confirmed broken code from automated review of PR Graphify-Labs#3087: 1. **Subdirectory misclassification (CONFIRMED BUG)** - Issue: Same-repo files like "internal/servers/file.go" wrongly classified as cross-repo to a nonexistent repo named "internal" - Root cause: Tautological check `"/" not in potential_repo` was always true since potential_repo is extracted before the first slash - Fix: Build set of known repos from test-jobs.yaml + graph analysis; only treat prefixes as cross-repo if they match known repos - Test: TestBugFixes::test_subdirectory_not_misclassified_as_cross_repo 2. **Full-suite fallback returns no jobs (CONFIRMED BUG)** - Issue: When no changed files match graph nodes, returned empty must_run list despite reasoning "Falling back to full test suite" - Fix: Load all jobs from test-jobs.yaml and add to must_run before early return - Test: TestBugFixes::test_full_suite_fallback_schedules_all_jobs 3. **Timed-out shell diff commands leave descendants running (CONFIRMED BUG)** - Issue: TimeoutExpired exception doesn't expose process handle; ffc72c3's fix tried to call .kill() on cmd string (has no such method) - Fix: Use subprocess.Popen with os.setsid() for process group management; properly kill process group on timeout - Note: shell=True with user-controlled --diff-cmd is intentional (local CLI tool, no injection risk) 4. **Fallback YAML parser drops list values (CONFIRMED BUG)** - Issue: When parsing "key:\n - item1\n - item2", created empty dict for key, then loop over parent.keys() found nothing, dropped items - Fix: When encountering list item with empty-dict parent, replace dict with list in grandparent - Test: TestBugFixes::test_yaml_parser_preserves_list_values All fixes verified against real code (not eyeballed) with reproduction tests. Existing test suite (26 tests) still passes. 3 new regression tests added. Supersedes incomplete ffc72c3 fix commit.
There was a problem hiding this comment.
Graphify reviewed this change.
Worth a look — the grounded gate found no coupling regressions or blocking issues, but 5 advisory finding(s) below merit a look before merge.
Formal verification. 1 change(s) tested, no difference found (not proven).
Graphify review — findings
Adds a graph-informed CI test-selection command: ci-select takes a diff (via --diff-cmd, --diff/stdin, or --files), walks the graph outward to a --depth (default 3) via BFS from changed files, and maps reached nodes to test jobs through an auto-detected or explicit test-jobs.yaml, with the new usage block wired into CLI dispatch and help. Documents the published-bundle contract: a new metadata.json schema (schema_version, source_sha, graphify_version, generated_at, bundle) plus prose explaining that a graphify_version mismatch hard-fails the fetch while source_sha/generated_at only gate staleness. Treats a cache entry that fails to decode as UTF-8 as corrupt rather than a miss by catching UnicodeDecodeError alongside JSONDecodeError in load_cached, so truncated multi-byte writes get counted and re-extracted instead of silently failing forever.
Worth a look
- Same-repo directories are inferred as cross-repo prefixes —
graphify/ci_select.py· Escalate · high- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
- Broken assertion references undefined variable —
tests/test_github_actions.py:379· Escalate · high- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
- Popen uses preexec_fn, which can deadlock in multithreaded callers —
graphify/ci_select.py· Escalate · medium- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
- preexec_fn=None passed on platforms without setsid still valid but signal.killpg unguarded —
graphify/ci_select.py· Escalate · medium- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
- User-controlled diff command is executed through a shell —
graphify/ci_select.py· Escalate · medium- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
Analysis details — impact, health, verification
Impact & health
Graphify review
Impact — 2968 functions depend on the 936 functions this change touches.
Health — this change adds coupling hotspots:
- new:
extract()— 494 callers, 42 callees - new:
_rebuild_code()— 98 callers, 50 callees - new:
detect()— 108 callers, 15 callees - new:
build_merge()— 46 callers, 14 callees - new:
to_obsidian()— 36 callers, 13 callees - new:
save_semantic_cache()— 52 callers, 9 callees - new:
save_manifest()— 40 callers, 11 callees - new:
to_json()— 50 callers, 7 callees - …and 60 more — each is listed as a finding
Verification — 2968 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: 2895 function(s) in the blast radius were not formally verified this run
Formal verification
Could not verify: Could not verify load\_cached.
The verifier did not have enough to check load\_cached, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: parameter `path` is annotated `Path` — outside the synthesizable primitive/collection set
Could not verify: Could not verify dispatch\_command.
The verifier did not have enough to check dispatch\_command, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: not verifiable: all 23 sampled inputs raised on both versions — the function never executed, so 'no divergence' would be vacuous (mostly SystemExit — names the real obstacle, not a sampling gap)
Could not verify: Could not verify classify\_file.
The verifier did not have enough to check classify\_file, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: parameter `path` is annotated `Path` — outside the synthesizable primitive/collection set
Could not verify: Could not verify \_git\_head.
The verifier did not have enough to check \_git\_head, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: parameter `cwd` is annotated `'str | Path | None'` — outside the synthesizable primitive/collection set
Could not verify: Could not verify \_get\_extractor.
The verifier did not have enough to check \_get\_extractor, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: parameter `path` is annotated `Path` — outside the synthesizable primitive/collection set
No difference found (not proven): No behavior difference found in \_run\_cli (not a proof).
The verifier ran both versions of \_run\_cli on many inputs and saw identical behavior every time. Strong evidence the change is safe, but evidence, not a proof.
Guarantee: Empirical: differential testing (both versions run on many generated inputs). A divergence on an untested input remains possible, so this is 'no counterexample found', not 'proven equivalent'.
Note: An input the sampler did not try could still differ.
· 4 grounded finding(s) anchored inline below; 64 more finding(s) on lines outside this diff (see the check run).
| return ", ".join(labels) if labels else "(no connections)" | ||
|
|
||
|
|
||
| def ci_select( |
There was a problem hiding this comment.
ci_select()
fans out to 9 callees (efferent coupling); 10 callers depend on it (afferent coupling).
Grounded coupling-delta finding (deterministic), not an LLM guess.
| return plan | ||
|
|
||
|
|
||
| def cli_main(argv: list[str] | None = None) -> None: |
There was a problem hiding this comment.
cli_main()
high coupling complexity (Ca·Ce = 20).
Grounded coupling-delta finding (deterministic), not an LLM guess.
| @@ -5375,6 +5380,17 @@ def _is_cpp_header(path: Path) -> bool: | |||
|
|
|||
| def _get_extractor(path: Path) -> Any | None: | |||
There was a problem hiding this comment.
_get_extractor()
fans out to 8 callees (efferent coupling); 29 callers depend on it (afferent coupling).
Grounded coupling-delta finding (deterministic), not an LLM guess.
| return "jobs" in keys and ("on" in keys or is_github_actions_workflow_path(path)) | ||
|
|
||
|
|
||
| def extract_github_actions(path: Path) -> dict: |
There was a problem hiding this comment.
extract_github_actions()
fans out to 10 callees (efferent coupling); 16 callers depend on it (afferent coupling).
Grounded coupling-delta finding (deterministic), not an LLM guess.
…tion (#498) ## Summary Enable semantic extraction (docs, images, etc.) in the graphify brain refresh workflow by integrating Vertex AI backend with Workload Identity Federation authentication. ## Changes ### 1. Workload Identity Federation Authentication - Added `google-github-actions/auth@v2` step to authenticate to `osac-ci` GCP project - Added `id-token: write` permission for OIDC token generation - Service account: `osac-ci@osac-ci.iam.gserviceaccount.com` - WIF provider: `projects/1008600636152/locations/global/workloadIdentityPools/github-actions-pool/providers/github-actions-provider` ### 2. Vertex AI Backend Configuration - Set `GOOGLE_CLOUD_PROJECT=osac-ci` - Set `GOOGLE_CLOUD_LOCATION=us-central1` - Backend auto-detection works via these env vars (no `--backend vertex` flag needed) ### 3. Install graphify from Fork (Temporary) **Before:** `pip install --user "graphifyy[sql]==0.9.41"` **After:** `pip install --user "git+https://github.com/eliorerz/graphify@v8#egg=graphifyy[sql,vertex]"` This is temporary until upstream PRs merge: - Graphify-Labs/graphify#3083 (Vertex AI backend) - Graphify-Labs/graphify#3087 (ci-select + fixes) Once merged and a new `graphifyy` release is published, revert to: `pip install --user "graphifyy[sql,vertex]==${GRAPHIFY_VERSION}"` ### 4. Enable Semantic Extraction - **Removed** `--code-only` flag from full extraction (`graphify extract .`) - Now extracts semantic content from docs, images, markdown, etc. via Gemini 2.5 Flash - Incremental updates (`graphify update .`) unchanged (still code-only) ## Rationale Red Hat's GCP org policy **disallows raw API keys entirely** (confirmed live against the osac-ci project). This blocks the existing `gemini` backend (requires `GEMINI_API_KEY`). Vertex AI uses **Application Default Credentials** (service account via Workload Identity Federation) — no static secrets, aligns with security policy. ## Infrastructure Already Deployed - ✅ GCP project: `osac-ci` (project number `1008600636152`) - ✅ Service account with `roles/aiplatform.user` - ✅ WIF pool/provider configured, org-scoped to `osac-project` repos, main branch only - ✅ Vertex backend tested live end-to-end with real extraction ## Testing - [x] YAML validates (`python3 -c "import yaml; yaml.safe_load(...)"`) - [ ] Will test on first scheduled run or manual workflow_dispatch trigger - [ ] Confirm graph.json bundle includes semantic nodes from docs/images ## Rollback Plan If issues occur: 1. Revert this PR 2. Falls back to code-only extraction (AST-only graph, no semantic content) 3. No data loss — graph.json just won't have doc/image nodes <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Improvements** * Improved automated knowledge refresh reliability with more secure cloud authentication. * Refreshes now perform complete content extraction when existing semantic data is incomplete, while continuing to use incremental updates when appropriate. * Enhanced semantic processing delivers more complete and meaningful results. * **Documentation** * Clarified that knowledge reflects committed project content and may not reflect current repository or workflow status. * Added guidance to verify live repository and workflow details directly. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
Summary
ci-selectsubcommand: graph-informed CI test selection. BFS traversal (configurable depth, default 3 hops) from changed files, maps reachable nodes to CI job names via atest-jobs.yamlmapping. Confidence tiers (>=0.8 use as-is, 0.5-0.8 log, <0.5 full-suite fallback), cross-repo impact detection viasource_fileprefixes, structured JSON output. Sub-second on 51K-node graphs. Includes 351 lines of tests.docs/graph-bundle-metadata.md+ JSON Schema: documents themetadata.jsonmanifest that accompanies a publishedgraphify --updatebundle (graph.json/GRAPH_REPORT.md/manifest.json) when consumed by a separate machine/CI job rather than generated locally — gives bundle producers and consumers one source of truth for the contract instead of two independently-evolving assumptions.--code-only: graphify has no YAML support at all today (.yaml/.ymlareDOC_EXTENSIONSwith no extractor), so.github/workflows/*.yamlcontent is invisible to the graph regardless of corpus scope — including on a code-only, no-LLM-backend first run. Extracts jobneeds/usesrelationships into graph nodes/edges. Ships with 3 follow-up fix commits (CodeRabbit review findings, git executable resolution viashutil.which(), extractor-precedence/comment-in-sequence bugs).Test plan
tests/test_ci_select.py, 351 lines) covers the ci-select feature