Repository navigation
Conversation
ajithksr
changed the title
|
Hello @safishamsi just checking in to see if you have bandwidth to review this PR. Let me know if you need any further context or changes to get this merged!" I intend to provide a featire where we can use graphify to extract all comment blocks from with in a function...with a traget to improve logging analysis in linux/android world. |
10f9055 to
56598f6
Compare
![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
This pull request adds an opt-in AST-based logging statement extraction feature. It introduces a new GenericLogExtractor class (in graphify/generic_logger.py) driven by a YAML config, plus a GraphifyBuilderWrapper in extract.py that hooks the extractor into both the single-file and sequential extraction paths to inject log nodes/edges into the graph result. The change also wires up new --enable-logging and --logging-config CLI options for the extract and update commands (setting corresponding environment variables), and updates the __main__.py help text and README.md to document them. The update command's argument parser was also refactored from a for-loop to an index-based while-loop to support options that take a value.
No blocking issues surfaced. 9 lower-confidence candidates did not survive cross-model review.
Analysis details — impact, health, verification
Impact & health
Graphify review
Impact — 1411 functions depend on the 288 functions this change touches.
Health — this change adds coupling hotspots:
- worse:
extract()— 376 callers, 39 callees
Verification — 1411 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: 1411 function(s) in the blast radius were not formally verified this run
· 1 more finding(s) on lines outside this diff (see the check run).
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 opt-in AST-based logging-statement extraction driven by GenericLogExtractor, wiring a new --enable-logging / --logging-config PATH flag pair (and GRAPHIFY_EXTRACT_LOGS / GRAPHIFY_LOGGING_CONFIG env vars) into both the extract and update commands. During extraction, each file is passed through inject_logs_to_graph via a GraphifyBuilderWrapper that appends log-type nodes and EXTRACTED edges to the result; the hook is wrapped in try/except so failures print to stderr without aborting extraction. Extraction stays disabled unless the flag is set and the config YAML exists, otherwise it warns and no-ops.
Worth a look
- Log extraction unconditionally runs regardless of --enable-logging flag —
graphify/extract.py:5533· Escalate · high- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
- test asserts nodes of type 'log' that are never created —
tests/test_generic_logging.py:90· Escalate · high- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
- test asserts edge fields 'relation'/'target' that do not match add_edge parameters —
tests/test_generic_logging.py:96· Escalate · high- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
- Logging extraction runs despite opt-in CLI contract —
graphify/extract.py:5533· Escalate · medium- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
- Enabled log extraction implicitly trusts logging_config.yaml from the current working directory —
graphify/generic_logger.py:24· 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 — 1804 functions depend on the 320 functions this change touches.
Health — this change adds coupling hotspots:
- new:
extract()— 501 callers, 42 callees - new:
_rebuild_code()— 98 callers, 50 callees - new:
extract_xaml()— 19 callers, 17 callees - new:
extract_js()— 85 callers, 3 callees - new:
dispatch_command()— 2 callers, 122 callees - new:
_get_extractor()— 26 callers, 6 callees - new:
run_pipeline()— 8 callers, 13 callees - new:
collect_files()— 17 callers, 6 callees - …and 25 more — each is listed as a finding
Verification — 1804 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: 1804 function(s) in the blast radius were not formally verified this run
Formal verification
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 \_extract\_sequential.
The verifier did not have enough to check \_extract\_sequential, 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 \_extract\_single\_file.
The verifier did not have enough to check \_extract\_single\_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: not verifiable: all 8 sampled inputs raised on both versions — the function never executed, so 'no divergence' would be vacuous (mostly ValueError — names the real obstacle, not a sampling gap)
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.
· 33 more finding(s) on lines outside this diff (see the check run).
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 an opt-in AST-based logging statement extractor gated behind --enable-logging (or GRAPHIFY_EXTRACT_LOGS=1) with a configurable YAML rules file via --logging-config/GRAPHIFY_LOGGING_CONFIG, wired into both extract and update argument parsing. During file extraction, GenericLogExtractor.inject_logs_to_graph runs after normal extraction and, through GraphifyBuilderWrapper, adds log-typed nodes and edges for detected logging calls in Java/Kotlin/C/C++ sources. When logging is disabled, the config is missing, or the hook raises, it degrades to a warning/error on stderr and leaves the graph untouched.
Worth a look
- Log extraction runs unconditionally, ignoring --enable-logging flag —
graphify/extract.py:5533· Escalate · high- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
- Parallel extraction also ignores the logging enable flag —
graphify/extract.py:5534· Escalate · medium- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
- Logging extraction runs even when the new flag is not enabled —
graphify/extract.py:5709· Escalate · medium- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
- Lazy config load is not thread-safe —
graphify/generic_logger.py:20· Escalate · medium- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
- config may be None if yaml file has no logging_rules key causing AttributeError —
graphify/generic_logger.py:45· 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 — 1805 functions depend on the 321 functions this change touches.
Health — this change adds coupling hotspots:
- new:
extract()— 502 callers, 42 callees - new:
_rebuild_code()— 98 callers, 50 callees - new:
extract_xaml()— 19 callers, 17 callees - new:
extract_js()— 85 callers, 3 callees - new:
dispatch_command()— 2 callers, 122 callees - new:
_get_extractor()— 26 callers, 6 callees - new:
run_pipeline()— 8 callers, 13 callees - new:
collect_files()— 17 callers, 6 callees - …and 25 more — each is listed as a finding
Verification — 1805 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: 1805 function(s) in the blast radius were not formally verified this run
Formal verification
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 \_extract\_sequential.
The verifier did not have enough to check \_extract\_sequential, 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 \_extract\_single\_file.
The verifier did not have enough to check \_extract\_single\_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: not verifiable: all 8 sampled inputs raised on both versions — the function never executed, so 'no divergence' would be vacuous (mostly ValueError — names the real obstacle, not a sampling gap)
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.
· 33 more finding(s) on lines outside this diff (see the check run).
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 3 advisory finding(s) below merit a look before merge.
Formal verification. 1 change(s) tested, no difference found (not proven).
Graphify review — findings
Adds opt-in AST-based logging-statement extraction gated behind --enable-logging (or GRAPHIFY_EXTRACT_LOGS=1) on both extract and update, with an optional --logging-config PATH pointing at a YAML rules file. When enabled, GenericLogExtractor parses each supported source (Java, Kotlin, C, C++) via tree-sitter and injects log nodes and edges into the extracted graph through GraphifyBuilderWrapper; if the config file is missing it warns and stays disabled, and any per-file extraction error is caught and logged to stderr without failing the run. Also fixes update argument parsing to consume the two-token --logging-config PATH form correctly.
Worth a look
- add_edge injects log node/edge but is bypassed on cache hits producing inconsistent graphs —
graphify/extract.py:5709· Escalate · medium- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
- Log injection bypasses cache invalidation, producing stale results when --enable-logging toggled —
graphify/extract.py:5709· Escalate · medium- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
- find_id returns first identifier which may be a parameter/type, not the function name —
graphify/generic_logger.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 — 1806 functions depend on the 322 functions this change touches.
Health — this change adds coupling hotspots:
- new:
extract()— 502 callers, 42 callees - new:
_rebuild_code()— 98 callers, 50 callees - new:
extract_xaml()— 19 callers, 17 callees - new:
extract_js()— 85 callers, 3 callees - new:
dispatch_command()— 2 callers, 122 callees - new:
_get_extractor()— 26 callers, 6 callees - new:
run_pipeline()— 8 callers, 13 callees - new:
collect_files()— 17 callers, 6 callees - …and 25 more — each is listed as a finding
Verification — 1806 functions in the blast radius were not formally verified this run (proofs are advisory here).
Health delta baseline: last indexed commit fe66389 (diverged from this PR's base — delta is approximate).
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: 1806 function(s) in the blast radius were not formally verified this run
Test selection
Test selection
233 of 233 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— impact, full-run-safetytests/test_affected_member_seed.py— full-run-safetytests/test_agents_platform.py— impact, 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— impact, full-run-safetytests/test_astro_import_ids.py— impact, full-run-safetytests/test_atomic_writes.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_build.py— impact, 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— impact, full-run-safetytests/test_cache.py— full-run-safetytests/test_callflow_html.py— full-run-safetytests/test_cargo_introspect.py— full-run-safetytests/test_carried_hyperedge_remap.py— full-run-safetytests/test_case_sensitive_resolution.py— impact, full-run-safetytests/test_charmap_encoding.py— full-run-safetytests/test_chunking.py— full-run-safetytests/test_cjs_module_extension.py— impact, 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_cluster.py— full-run-safetytests/test_codebuddy.py— impact, 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_nested_and_cli.py— impact, full-run-safetytests/test_cpp_objc_cross_file_calls.py— impact, full-run-safetytests/test_cpp_preprocess.py— full-run-safetytests/test_cross_extension_reexport_self_cycle.py— impact, full-run-safetytests/test_cross_language_call_resolution.py— impact, full-run-safetytests/test_cross_repo_shared_types.py— full-run-safetytests/test_csharp_call_site_generic_args.py— impact, full-run-safetytests/test_csharp_enum_members.py— impact, full-run-safetytests/test_csharp_field_generic_args.py— impact, full-run-safetytests/test_csharp_interface_dispatch.py— impact, full-run-safetytests/test_csharp_member_calls.py— impact, full-run-safetytests/test_csharp_member_nodes.py— impact, full-run-safetytests/test_csharp_object_creation.py— impact, full-run-safetytests/test_csharp_partial_classes.py— impact, full-run-safety- … and 183 more
non-code file(s) changed (
README.md,logging_config.yaml,workflow.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 (
README.md,workflow.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.
Formal verification
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 \_extract\_sequential.
The verifier did not have enough to check \_extract\_sequential, 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 \_extract\_single\_file.
The verifier did not have enough to check \_extract\_single\_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: not verifiable: all 8 sampled inputs raised on both versions — the function never executed, so 'no divergence' would be vacuous (mostly ValueError — names the real obstacle, not a sampling gap)
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.
· 33 more finding(s) on lines outside this diff (see the check run).
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 opt-in AST-based logging extraction via --enable-logging/GRAPHIFY_EXTRACT_LOGS (with --logging-config/GRAPHIFY_LOGGING_CONFIG for a custom YAML) on both extract and update, wiring GenericLogExtractor into _extract_single_file and _extract_sequential to inject log nodes and edges through a GraphifyBuilderWrapper; hook failures are caught and logged to stderr without aborting extraction. Namespaces the cache under ast--prefixed kinds so logging-enabled runs use a separate cache bucket from plain AST runs, keyed by get_cache_kind(). Reworks the update arg loop to an index-based scanner so the new value-taking flags parse alongside the existing ones.
Worth a look
- Unsafe YAML load of logging_config.yaml —
graphify/generic_logger.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
- Log-enabled cache is populated after failed log injection —
graphify/extract.py:5539· Escalate · medium- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
- extract in _extract_sequential still saves under default kind but sequential load path missing kind —
graphify/extract.py:5720· Escalate · medium- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
- cache_dir no longer honors prompt_fp for non-AST kinds prefixed with ast- —
graphify/cache.py:929· Escalate · medium- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
- Reload publishes new config path before config is loaded —
graphify/generic_logger.py:34· 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 — 2135 functions depend on the 412 functions this change touches.
Health — this change adds coupling hotspots:
- new:
extract()— 504 callers, 42 callees - new:
_rebuild_code()— 98 callers, 50 callees - new:
detect()— 108 callers, 15 callees - new:
save_semantic_cache()— 58 callers, 9 callees - new:
dispatch_command()— 4 callers, 122 callees - new:
load_cached()— 48 callers, 7 callees - new:
extract_xaml()— 19 callers, 17 callees - new:
file_hash()— 50 callers, 6 callees - …and 37 more — each is listed as a finding
Verification — 2135 functions in the blast radius were not formally verified this run (proofs are advisory here).
Health delta baseline: last indexed commit a5957aa (diverged from this PR's base — delta is approximate).
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: 2135 function(s) in the blast radius were not formally verified this run
Test selection
Test selection
233 of 233 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— impact, full-run-safetytests/test_affected_member_seed.py— full-run-safetytests/test_agents_platform.py— impact, 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— impact, full-run-safetytests/test_astro_import_ids.py— impact, full-run-safetytests/test_atomic_writes.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_build.py— impact, 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— impact, full-run-safetytests/test_cache.py— impact, full-run-safetytests/test_callflow_html.py— full-run-safetytests/test_cargo_introspect.py— full-run-safetytests/test_carried_hyperedge_remap.py— full-run-safetytests/test_case_sensitive_resolution.py— impact, full-run-safetytests/test_charmap_encoding.py— impact, full-run-safetytests/test_chunking.py— impact, full-run-safetytests/test_cjs_module_extension.py— impact, 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_cluster.py— full-run-safetytests/test_codebuddy.py— impact, 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_nested_and_cli.py— impact, full-run-safetytests/test_cpp_objc_cross_file_calls.py— impact, full-run-safetytests/test_cpp_preprocess.py— full-run-safetytests/test_cross_extension_reexport_self_cycle.py— impact, full-run-safetytests/test_cross_language_call_resolution.py— impact, full-run-safetytests/test_cross_repo_shared_types.py— full-run-safetytests/test_csharp_call_site_generic_args.py— impact, full-run-safetytests/test_csharp_enum_members.py— impact, full-run-safetytests/test_csharp_field_generic_args.py— impact, full-run-safetytests/test_csharp_interface_dispatch.py— impact, full-run-safetytests/test_csharp_member_calls.py— impact, full-run-safetytests/test_csharp_member_nodes.py— impact, full-run-safetytests/test_csharp_object_creation.py— impact, full-run-safetytests/test_csharp_partial_classes.py— impact, full-run-safety- … and 183 more
non-code file(s) changed (
README.md,logging_config.yaml,workflow.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 (
README.md,workflow.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.
Formal verification
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: no capturable inputs from the test suite; property tier: 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: no capturable inputs from the test suite; property tier: 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 extract.
The verifier did not have enough to check extract, 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: no capturable inputs from the test suite; property tier: parameter `cache_root` is annotated `Path | None` — outside the synthesizable primitive/collection set
Could not verify: Could not verify \_extract\_sequential.
The verifier did not have enough to check \_extract\_sequential, 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: no capturable inputs from the test suite; property tier: parameter `root` is annotated `Path` — outside the synthesizable primitive/collection set
Could not verify: Could not verify \_extract\_single\_file.
The verifier did not have enough to check \_extract\_single\_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: no capturable inputs from the test suite; property tier: not verifiable: all 8 sampled inputs raised on both versions — the function never executed, so 'no divergence' would be vacuous (mostly ValueError — names the real obstacle, not a sampling gap)
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.
· 45 more finding(s) on lines outside this diff (see the check run).
|
Update: Pushed commit
Ready for review when you have bandwidth! @safishamsi |
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.
Graphify review — findings
Adds an optional AST-based logging-statement extractor exposed as --enable-logging / --logging-config PATH (and GRAPHIFY_EXTRACT_LOGS / GRAPHIFY_LOGGING_CONFIG env vars) on extract and update, driven by GenericLogExtractor which parses each file with tree-sitter and injects log-typed nodes and edges into the graph via GraphifyBuilderWrapper. Routes cached results through a logging-aware kind in load_cached/save_cached (with cache_dir giving ast- kinds their own versioned directory) so log-augmented and plain extractions don't collide, and skips writing the cache for a file whose log injection fails so a rerun can self-heal. Rewrites the update arg loop to index-based parsing so it can consume the new two-token options without swallowing the path argument.
Worth a look
- generic_logger.py is syntactically invalid —
graphify/generic_logger.py:279· Escalate · high- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
- New test module has an incomplete function definition —
tests/test_generic_logging.py:588· Escalate · high- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
- save_cached/load_cached called with new kind= keyword arg not present in original signature —
graphify/extract.py:6994· Escalate · high- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
- AST cache with prompt fingerprint skips extractor/schema versioning —
graphify/cache.py:956· Escalate · medium- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
- cache_dir now reclassifies ast-prefixed custom cache kinds —
graphify/cache.py:956· 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 — 2658 functions depend on the 484 functions this change touches.
Health — this change adds coupling hotspots:
- new:
extract()— 719 callers, 47 callees - new:
_rebuild_code()— 144 callers, 55 callees - new:
detect()— 112 callers, 15 callees - new:
save_semantic_cache()— 63 callers, 9 callees - new:
dispatch_command()— 4 callers, 125 callees - new:
load_cached()— 56 callers, 7 callees - new:
extract_js()— 87 callers, 4 callees - new:
extract_xaml()— 19 callers, 17 callees - …and 49 more — each is listed as a finding
Verification — 2658 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: 2652 function(s) in the blast radius were not formally verified this run
Test selection
Test selection
302 of 302 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— impact, full-run-safetytests/test_affected_member_seed.py— full-run-safetytests/test_agents_platform.py— impact, 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— impact, full-run-safetytests/test_astro_import_ids.py— impact, 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_build.py— impact, 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— impact, full-run-safetytests/test_cache.py— impact, 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— impact, full-run-safetytests/test_charmap_encoding.py— impact, full-run-safetytests/test_chunking.py— impact, full-run-safetytests/test_cjs_module_extension.py— impact, 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_cobol_extractor.py— impact, full-run-safetytests/test_codebuddy.py— impact, 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_nested_and_cli.py— impact, full-run-safetytests/test_cpp_objc_cross_file_calls.py— impact, full-run-safetytests/test_cpp_preprocess.py— full-run-safetytests/test_cross_extension_reexport_self_cycle.py— impact, full-run-safetytests/test_cross_language_call_resolution.py— impact, full-run-safetytests/test_cross_repo_external_call_guards.py— impact, full-run-safetytests/test_cross_repo_member_calls.py— impact, full-run-safety- … and 252 more
non-code file(s) changed (
README.md,logging_config.yaml,workflow.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 (
README.md,workflow.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.
· 57 more finding(s) on lines outside this diff (see the check run).
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. 3 change(s) tested, no difference found (not proven).
Graphify review — findings
Adds an opt-in AST-based logging-statement extractor gated behind --enable-logging (and --logging-config PATH) on the extract and update commands, wired through env vars GRAPHIFY_EXTRACT_LOGS/GRAPHIFY_LOGGING_CONFIG. When enabled, _extract_single_file runs GenericLogExtractor over each file via GraphifyBuilderWrapper, appending log-typed nodes and edges to the graph, and routes caching to a separate ast-logging cache kind so logging results don't collide with plain AST entries. On injection failure it logs to stderr and skips the cache write so a rerun can self-heal rather than poisoning the cache with a partial result.
Worth a look
- generic_logger.py is syntactically invalid —
graphify/generic_logger.py:280· Escalate · high- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
- Incomplete test function makes test module unparsable —
tests/test_generic_logging.py:323· Escalate · high- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
- load_cached/save_cached called with new 'kind' kwarg not present in cache.py signature —
graphify/extract.py:7003· Escalate · high- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
- Parallel worker does not receive the caller-selected cache kind —
graphify/extract.py:6988· Escalate · medium- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
- Partial log injection leaves mutated result but skips caching, silently altering output vs cached runs —
graphify/extract.py:7003· 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 — 2658 functions depend on the 484 functions this change touches.
Health — this change adds coupling hotspots:
- new:
extract()— 719 callers, 47 callees - new:
_rebuild_code()— 144 callers, 55 callees - new:
detect()— 112 callers, 15 callees - new:
save_semantic_cache()— 63 callers, 9 callees - new:
dispatch_command()— 4 callers, 125 callees - new:
load_cached()— 56 callers, 7 callees - new:
extract_js()— 87 callers, 4 callees - new:
extract_xaml()— 19 callers, 17 callees - …and 49 more — each is listed as a finding
Verification — 2658 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: 2652 function(s) in the blast radius were not formally verified this run
Test selection
Test selection
302 of 302 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— impact, full-run-safetytests/test_affected_member_seed.py— full-run-safetytests/test_agents_platform.py— impact, 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— impact, full-run-safetytests/test_astro_import_ids.py— impact, 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_build.py— impact, 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— impact, full-run-safetytests/test_cache.py— impact, 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— impact, full-run-safetytests/test_charmap_encoding.py— impact, full-run-safetytests/test_chunking.py— impact, full-run-safetytests/test_cjs_module_extension.py— impact, 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_cobol_extractor.py— impact, full-run-safetytests/test_codebuddy.py— impact, 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_nested_and_cli.py— impact, full-run-safetytests/test_cpp_objc_cross_file_calls.py— impact, full-run-safetytests/test_cpp_preprocess.py— full-run-safetytests/test_cross_extension_reexport_self_cycle.py— impact, full-run-safetytests/test_cross_language_call_resolution.py— impact, full-run-safetytests/test_cross_repo_external_call_guards.py— impact, full-run-safetytests/test_cross_repo_member_calls.py— impact, full-run-safety- … and 252 more
non-code file(s) changed (
README.md,logging_config.yaml,workflow.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 (
README.md,workflow.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.
Formal verification
No difference found (not proven): No behavior difference found in cache\_dir (not a proof).
The verifier ran both versions of cache\_dir 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 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: no capturable inputs from the test suite; property tier: 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 extract.
The verifier did not have enough to check extract, 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: no capturable inputs from the test suite; property tier: not verifiable: all 90 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)
No difference found (not proven): No behavior difference found in \_extract\_sequential (not a proof).
The verifier ran both versions of \_extract\_sequential 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 \_extract\_single\_file.
The verifier did not have enough to check \_extract\_single\_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: no capturable inputs from the test suite; property tier: not verifiable: all 8 sampled inputs raised on both versions — the function never executed, so 'no divergence' would be vacuous (mostly ValueError — names the real obstacle, not a sampling gap)
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.
· 57 more finding(s) on lines outside this diff (see the check run).
|
Hi @safishamsi and the Graphify team, This PR has been updated and synchronized with upstream
Ready for your review when you have a moment! |
|
Hi @safishamsi and the Graphify team, Following up on the automated review from
Verification Summary
The PR is in a healthy, verified state and ready for review/merge when you have a moment! |
025c607 to
76ec832
Compare
- Implements GenericLogExtractor driven by YAML Tree-sitter query rules - Adds opt-in CLI flags --enable-logging and --logging-config for extract and update - Isolates logging cache under versioned ast-logging namespaces with grammar fingerprinting - Preserves AST parser precision for complex function names, destructors, and lambdas - Adds self-healing error handling preventing faulty extractions from polluting the cache - Includes comprehensive unit and integration tests across supported languages
76ec832 to
0a7e33f
Compare
|
Hi @safishamsi and the Graphify team, I have updated and squashed this PR directly on top of the latest
Ready for your review and merge when you have a moment! |
This branch has not been deployed
1 participant
This PR makes the AST-based logging extraction feature an opt-in CLI toggle, improving the default performance and usability for users who do not need logging analysis.
Changes:
GenericLogExtractorto load configuration lazily based on environment variables.--enable-loggingand--logging-config PATHarguments tographify extractandgraphify update.README.mdto document the new flags.All 128 tests pass successfully.