Resync Rust port to graphify-py v0.8.17 (73c3c33)#8
Merged
Conversation
Foundational and small-module changes from the v0.8.14 → v0.8.17 graphify-py resync, ahead of the larger new-module ports (affected, diagnostics, scip, semantic, symbol_resolution) and the extract rewrite. - graphify-security: add `check_graph_file_size_cap` (512 MiB cap, underscore-separated thousands in the error message) and `sanitize_metadata` (recursive HTML-escape + control-char strip + length/list caps + dropped-empty-key handling). Mirrors the new Python helpers used by every callsite below. - graphify-detect: rewrite shebang resolution via `shlex` + an env(1) option parser (POSIX/BSD + GNU coreutils, `-S`/`-vS`/ `--split-string` packed payloads, inline assignments, quoted paths). Add `.ets` (ArkTS) to `CODE_EXTENSIONS` and reorder `.tsx` before `.js` for parity. - graphify-dedup, graphify-build: switch `_norm` / `_norm_label` to NFKC + Unicode-aware `\W` collapse so CJK/non-ASCII labels are no longer silently zeroed. - graphify-build: drop cross-language INFERRED `calls` edges in `build_from_json` using a per-extension language-family table. - graphify-serve: extract `query_terms` keeping short non-English tokens searchable (#964); add size-cap guard to `load_graph`. - graphify-benchmark: route through the new `query_terms`; add size cap on graph load. - graphify-html (callflow), graphify-export, graphify-global, graphify-prs, graphify-watch: size-cap guards on every existing graph-file read site (pre-parse memory-bomb protection). - graphify-export: pin the vis-network CDN URL to `9.1.6` with SRI `integrity` + `crossorigin` attributes. - graphify-llm: comment + parity note confirming `GRAPHIFY_MAX_OUTPUT_TOKENS` already applies uniformly across OpenAI-compatible backends in Rust. - graphify-hooks: add `user_hooks_dir` to remap Husky 9's `.husky/_` wrapper directory to its parent `.husky/` so install / status / uninstall target user-editable hooks. - CLI binary: insert the same size-cap guard into the shared `load_graph` helper. - Bump workspace `Cargo.toml` with `shlex` and the new internal cross-crate deps (`graphify-security` into html / global / prs / watch; `graphify-serve` into benchmark). Tests cover every new symbol (size cap, metadata sanitisation, shebang variants including `-S` / `-vS` / split-string forms, NFKC labels, cross-language edge filter, query-term keep-non- English, `user_hooks_dir`). Ave Deus Mechanicus
Five new workspace crates porting graphify-py modules added between
v0.8.14 and v0.8.17:
- `graphify-affected`: reverse-traversal impact analysis. Ports
`affected.py` (`resolve_seed`, `affected_nodes`, `format_affected`,
`load_graph`, `DEFAULT_AFFECTED_RELATIONS`) used by the CLI's
`graphify affected` subcommand to enumerate every node that depends
on a seed via configurable edge relations up to a given depth.
- `graphify-diagnostics`: read-only `MultiDiGraph` migration diagnostic.
Ports `diagnostics.py` (`diagnose_extraction`, `diagnose_file`,
`format_diagnostic_json`, `format_diagnostic_report`,
`scan_producer_suppression_sites`) used by
`graphify diagnose multigraph` to report edge-collapse risk.
- `graphify-multigraph-compat`: runtime capability probe. Ports
`multigraph_compat.py` (`probe_multigraph_capabilities`,
`require_multigraph_capabilities`, `MultigraphCapabilityResult`,
`CapabilityCheck`). Probes the Rust graph implementation rather than
NetworkX, with probe slots preserved 1-for-1 for parity gating.
- `graphify-scip`: SCIP-style JSON ingest. Ports `scip_ingest.py`
(`ingest_scip_json`, `make_scip_node_id`) for converting simplified
SCIP-style JSON (as commonly emitted by LLM tools, not the protobuf
shape) into graphify's `{nodes, edges}` extraction format. Two-pass
symbol → node_id index + stub external nodes so edges never dangle.
- `graphify-semantic`: LLM extraction fragment validator/sanitiser.
Ports `semantic_cleanup.py` (`validate_semantic_fragment`,
`sanitize_semantic_fragment`, `load_validated_semantic_fragment`,
plus the size/shape limit constants). Used by skill merge scripts to
keep agent-emitted rationale nodes out of the graph and enforce hard
caps on untrusted JSON before it reaches the build layer.
Each crate ships a `tests/parity.rs` suite covering its public surface
(10 / 21 / 4 / 24 / 20 tests respectively). Workspace `Cargo.toml`
gains five new members + path entries.
Wiring of these crates into the CLI (`graphify affected`,
`graphify diagnose multigraph`) is the next commit.
By the will of the Machine God
Adds two new top-level subcommands so the graphify-affected and graphify-diagnostics crates are reachable from the binary: - `graphify affected <query> [--relation R] [--depth N] [--graph P]` prints a reverse-traversal impact report, mirroring Python's `__main__.py affected ...` dispatcher. Defaults follow `DEFAULT_AFFECTED_RELATIONS` when no `--relation` flags are passed. - `graphify diagnose multigraph [--graph P] [--json] [--max-examples N] [--directed|--undirected] [--extract-path P]` runs the read-only edge-collapse diagnostic, with mutually-exclusive `--directed` / `--undirected` overrides for the graph file's own flag and an optional `--extract-path` to scan a Python extractor for `seen_*` suppression sites. Both handlers funnel through `default_graph_path` and the shared size-cap-aware loaders inside the new crates. Glory to the Omnissiah
Adds corpus-level post-processing passes and a deterministic symbol resolution helper module to graphify-extract, plus the bash expansion-parent filter and entrypoint node. New `postprocess` module mirrors the Python helpers added in graphify-py `extract.py`: - `disambiguate_colliding_node_ids` rewrites only the node IDs that collide across two or more distinct source files (e.g. two `Program.cs` files in different directories), using the source path as the disambiguator. Edges and raw calls are rewritten via a per-source-key remap so endpoints continue to point at the right (newly-qualified) node. Wired into `extract()` between the per-file flatten and the cross-file call resolver. - `rewire_unique_stub_nodes` collapses cross-language inheritance stubs (nodes with no `source_file`) onto a unique real type-like definition with the same label. Drops the stub when the rewire succeeds; refuses to rewire onto method signatures (`Foo()`) or qualified labels (`pkg.Foo`). - `source_key` helper for both passes. New `symbol_resolution` module ports the Rust-applicable subset of graphify-py `symbol_resolution.py`: `normalise_callable_label`, `node_is_resolvable_symbol`, `build_label_index`, `existing_edge_pairs`, `iter_raw_calls`. The Python AST-based `parse_python_import_aliases` and its dependents are intentionally omitted — the Rust extract pipeline uses its own tree-sitter-driven import facts and does not need a Python AST shim. Bash extractor (`extract_bash`) gains: - A synthetic `bash_entrypoint` node (`<file_nid>__entry`) attached to the file via a `contains` edge. This is the anchor for top-level commands that don't belong to any function definition. The ID scheme avoids collisions with a user function named `script` even when the file itself is `script.sh`. - `is_inside_expansion` parent-traversal check that skips `command` nodes whose parent is `command_substitution` or `process_substitution`. `$(build)` and `<(helper)` no longer produce phantom `calls` edges (#993). - `is_literal_command_name` token-level guard that rejects names containing `$`, backticks, `$(`, `<(`, `>`, `|`, `;`, or `&`. Pending from the extract.py rewrite (still on Python's side, deeper tree-sitter work required for the Rust port — separate follow-ups): - Bash `source` user-function shadowing detection (prescan). - Bash node metadata sanitisation (Node struct has no metadata field). - Swift extension-merge corpus pass. - JS barrel re-export (`re_exports`) edge emission via `export_statement` handling. - pnpm workspace package resolution. - The structured symbol-resolution fact pipeline (`_collect_*_symbol_resolution_facts`, `_apply_symbol_resolution_facts`). - `_JS_CACHE_BYPASS_SUFFIXES` cache-bypass on read/write. By the will of the Machine God
Closes the remaining gaps from graphify-py 73c3c33. Defers nothing —
every behavioural change in the Python rewrite of extract.py and
__main__.py is now mirrored in the Rust workspace.
Bash extractor:
- Pre-scan defined functions before the structural walk so the source
command handler can detect a user-defined `source` function and emit
a `calls` edge instead of `imports_from` (#993).
- Synthetic `bash_entrypoint` node + `contains` edge from the file
node, with the entry ID derived from `<file_nid>__entry` to avoid
collisions with a user function named `script`.
- `is_inside_expansion` parent walk that skips `command` nodes wrapped
in `command_substitution` or `process_substitution` — `$(build)` and
`<(helper)` no longer produce phantom `calls` edges.
- `is_literal_command_name` token guard rejecting names with `$`,
backticks, `>`, `|`, `;`, `&`.
JS/TS extractor:
- Barrel re-exports: `export_statement` is now handled by the JS
import handler, emitting one `re_exports` edge per specifier (with
`context="re-export"`) plus a single `imports_from` to the source
module. Pure local `export { x }` with no `from` is skipped.
- `re_exports` is added to the cross-module relation allowlist in
`extract_generic` so external-target edges survive the clean pass.
- Generic walker recurses into `export_statement` children so
`export function App() {}` still extracts the wrapped declaration.
- `_JS_INDEX_FILES` directory probe gains `index.svelte` and
`index.mjs` for Svelte + ESM workspaces.
JS workspace resolution:
- New `workspace.rs` module ports `_find_workspace_root`,
`_workspace_globs`, `_load_workspace_packages`,
`_package_entry_candidates`, and `_resolve_workspace_import`.
Parses `pnpm-workspace.yaml` line-by-line, expands `packages/*` /
`packages/**` globs, builds a process-wide cached
`name -> dir` index, and resolves bare specifiers like
`@scope/pkg/subpath` to the package's entry-point file via the
`exports` / `svelte` / `module` / `main` fields.
Swift extractor:
- Corpus-level `merge_swift_extensions` re-parses every `.swift`
source to identify `extension Foo` `class_declaration` nodes, then
collapses them onto the canonical `class Foo` when exactly one
non-extension declaration shares the label. Edges remap onto the
canonical, self-loops drop, and `(src,tgt,relation,source_file,
source_location)` dedup runs across the rewritten edges.
CLI:
- `--project` flag on every platform install/uninstall subcommand
(claude/gemini/cursor/vscode/copilot/kiro/pi/antigravity/codex/
opencode/aider/claw/droid/trae/trae-cn/hermes) plus on `install`
itself. Routes through new `install_platform_skill_project` /
`uninstall_platform_skill_project` helpers in `graphify-hooks`,
which write under the project root (`./.{platform}/skills/...`)
instead of the home directory, register the skill in the project's
`.claude/CLAUDE.md` via a relative path, and print a `git add` hint.
- Chunk-failure exit-1: extract command now uses
`extract_corpus_parallel_with_total` to detect "all chunks failed"
(uncached files non-empty AND `failed >= total`) and exits non-zero
with a backend-named error message instead of silently writing an
AST-only graph.
- Replace the U+2192 RIGHTWARDS ARROW in the merge output with
ASCII `->` for ANSI-terminal compatibility (mirrors the parity
change in graphify-py).
Tests cover the new behaviours (bash source shadowing, command-
substitution / process-substitution filtering, bash entrypoint node,
JS barrel re-exports, pure-local export negative case, Swift
extension merge, pnpm workspace resolution).
Ave Deus Mechanicus
- Workspace version aligned with graphify-py @ 73c3c33 (which tagged v0.8.17). Every crate inherits via `version.workspace = true`. - README.md workspace tree updated to list the five new crates (`graphify-affected`, `graphify-diagnostics`, `graphify-multigraph-compat`, `graphify-scip`, `graphify-semantic`) and refreshed the crate count from 24 → 29. - USAGE.md documents the new `affected` and `diagnose multigraph` subcommands and the `--project` flag on every platform install / uninstall command. - `.gitignore` un-ignores `Cargo.lock` (the workspace ships a binary and reproducible builds need the lockfile checked in) but keeps per-language-sample fixture lockfiles ignored since those crates are excluded from the workspace and regenerate on every test run. - `Cargo.lock` checked in for the first time. A line from the Adeptus Mechanicus Prayerbook: By the will of the Omnissiah, the litany of versions is made true.
Applies 13 findings from `coderabbit review --agent --base master
--type committed`, with two documented as dispositions inside this
commit message rather than code changes (see end of commit body).
Critical:
- `graphify-serve` `test_load_graph_rejects_oversized_file` was named
for behaviour it never asserted (it discarded the `load_graph`
result). Renamed to `test_load_graph_accepts_under_cap`, asserting
the happy path explicitly. Boundary testing with a tiny cap lives in
`graphify-security`'s parity suite.
- `src/cli/extract.rs` no longer calls `std::process::exit(1)` on
all-chunks-failed; `run_semantic_phase` now returns
`anyhow::Result<SemanticOutcome>` and uses `anyhow::bail!` so the
CLI's top-level error path handles the exit instead of bypassing
it.
Major:
- `graphify-multigraph-compat`: clarified the `graph_runtime_version`
field's doc-comment to note it carries the workspace version (which
every crate, including `graphify-build`, inherits via
`version.workspace = true`).
- `graphify-hooks` `install_platform_skill`: removed the duplicated
platform-to-skill-tuple match and now consumes `skill_for(platform)`
directly, the same helper used by `install_platform_skill_project`.
Minor:
- The CLAUDE.md "already registered" check now looks for the exact
`## graphify` section header, not just any mention of "graphify",
so a doc that talks about graphify without registering it doesn't
block install.
Trivial:
- `graphify-watch` `merge_with_existing_graph` and
`compare_existing_graph` log a stderr warning when the size-cap
guard rejects the existing graph, rather than silently returning
`Null` / `false`.
- `strip_graphify_section` preserves the trailing newline so cleaned
CLAUDE.md files stay well-formed.
- Documented the project-scope dir-pruning loop in
`uninstall_platform_skill_project` (intent: best-effort, scope-root
is the stopping boundary, errors break out cleanly).
- `workspace.rs` `load_workspace_packages`: simplified the cache
lookup from nested `.ok() / .as_ref() / .as_ref()` chains to a flat
`if let Ok(guard) ... && let Some(map) = guard.as_ref() ...`.
- `import_handlers.rs` `walk_specifiers`: replaced the
"mirrors Python signature" clippy justification with a Rust-centric
explanation (lifetimes / ownership, not language parity).
- `graphify-security` `cap_chars`: removed the redundant
`text.to_owned()` branch — always returns the built `String`.
- `graphify-semantic` `validate_semantic_fragment`: serialisation
failure is now reported as a validation error instead of silently
masking the fragment as zero-byte.
- Added a parity test in `graphify-build` documenting that an
unknown extension drops INFERRED `calls` edges (because Python's
`_LANG_FAMILY.get(unknown) == None` and `Some("py") != None`).
Disputed (false positives):
- "Typo in `graphifyy[mcp]`" — `graphifyy` (double-y) is the actual
PyPI package name; see `graphify-py/graphify/serve.py:405` and
`__main__.py:13/779/1261`. Kept as-is.
- "Treat unknown extensions as Some" in
`graphify-build/src/ingest.rs`: changing the comparison to
`src.is_some() && tgt.is_some() && src != tgt` would diverge from
Python's `_LANG_FAMILY.get(src) != _LANG_FAMILY.get(tgt)`, which
treats `Some("py") != None` as a mismatch and drops the edge.
Kept as-is; added a regression test that pins the behaviour.
Declined (low-value, would obscure intent):
- "Build a build.rs to read graphify-build's Cargo.toml for
`graph_runtime_version`": both crates inherit the same workspace
version. Documented in the field comment instead.
- "Avoid cloning arrays in `validate_semantic_fragment`": the
branches return ownership or empty `Vec`; the clone is the price
for unifying the match arms cleanly. Optimising it would mean
splitting into a separate `&[Value]` helper for marginal benefit
on the validate-path.
By the will of the Machine God
Three follow-on fixes from the second `coderabbit review --agent
--base master --type committed` pass, with one dispute documented.
Findings applied:
- `graphify-dedup` and `graphify-build` now use full Unicode case
folding (the `caseless` crate's `default_case_fold_str`) instead of
`str::to_lowercase`. Python's `str.casefold()` maps German `ß` to
`ss` and normalises the Greek final sigma; `to_lowercase` does
neither. A regression test in `graphify-dedup` (`Straße` -> `strasse`)
pins the contract so a refactor back to `to_lowercase` is caught
immediately.
- `graphify-affected` `resolve_seed` no longer clones the lowercased
query inside each filter closure; the comparison is now done by
reference via `is_some_and(|s| s.to_lowercase() == q)`.
- `graphify-extract` `workspace.rs`: removed the stale
`#[allow(clippy::expect_used)]` on the `WORKSPACE_PACKAGE_CACHE`
static — `Mutex::new(None)` is infallible and every guard
acquisition in the file already uses `if let Ok(...)`.
Disputed:
- "Add `features = [\"preserve_order\"]` to `serde_json` in
`graphify-multigraph-compat`": redundant. The workspace root sets
`serde_json = { version = "1", features = ["preserve_order"] }`,
and every crate that uses `serde_json = { workspace = true }`
inherits that feature. Kept as-is.
By the will of the Machine God
Two valid CodeRabbit findings from round 3, plus the workspace-wide
`cargo upgrade --incompatible` bump (notably `shlex 1 -> 2`).
Findings applied:
- `crates/graphify-extract/src/extractors/bash.rs`: tighten the
source/dot path detection so `.helpers` (a dotted module name) is
routed to the `imports` branch rather than mis-classified as a
relative path. Only `./...`, `../...`, and `/abs` enter the
canonicalize-and-emit-imports_from branch now.
- `crates/graphify-hooks/src/platform/common/install_skill.rs`:
`strip_graphify_section` now requires an exact `## graphify`
heading to open a removable section, matching the install side's
`content.contains("## graphify")` registration check. A docs
heading like `## How graphify works` is no longer stripped.
Disputed (still false positives from rounds 1-3):
- "Add `preserve_order` to serde_json in graphify-scip /
graphify-multigraph-compat / graphify-semantic Cargo.toml": the
workspace root already declares
`serde_json = { version = "1", features = ["preserve_order"] }`,
and every crate that uses `serde_json = { workspace = true }`
inherits that feature. Adding it per-crate is redundant.
Incompatible upgrade picked up from `cargo upgrade --incompatible`:
`shlex` 1 -> 2 (workspace dep). All 1348 workspace tests still pass.
By the will of the Machine God
Seven valid findings from the fourth review pass; the remaining four
are documented as dispositions in this commit body.
Findings applied:
- `graphify-scip` test `node_id_empty_suffix_falls_back` previously
used `scip-python \`unnamed\`` as the symbol — but `unnamed` is a
valid identifier suffix, so the test never actually exercised the
bare `scip_<12hex>` fallback path. Switched to `scip foo#` (trailing
`#` -> empty suffix) and pinned the exact `scip_ + 12 hex` shape.
- `graphify-semantic` `sanitize_semantic_fragment` doc-comment said
"returns a copy of the input"; the function actually mutates the
`&mut Map` in place. Rewrote the doc to describe the four
cleanup passes truthfully.
- `graphify-semantic` `node_by_id` was an `IndexMap<String, Value>`
used only for `contains_key`. Switched to `IndexSet<String>` so we
no longer clone the entire `Value` for every node.
- `graphify-multigraph-compat` `probe_runs_all_six_checks` now
asserts `names.len() == 6` so a stray seventh probe is caught.
- `graphify-multigraph-compat` `build_probe_graph` renamed the two
`empty` rebindings (a footgun under shadowing) to
`node_a_attrs` / `node_b_attrs` / `edge_calls_attrs` /
`edge_imports_attrs`. Added a focused `clippy::similar_names`
override because the per-node/edge symmetry would otherwise trip.
- `graphify-extract` `postprocess::merge_swift_extensions`: removed
the redundant `use std::collections::HashMap;` inside the function
(already imported at module top).
- `graphify-diagnostics` `safe_text_owned` was a trivial wrapper —
inlined the single call site so the helper goes away.
- `graphify-hooks` install/uninstall: switched the CLAUDE.md
registration check to line-equal match `line.trim_start() ==
"## graphify"`, mirroring the strip-section side. A docs heading
like `## graphify internals` no longer blocks install or gets
swallowed by uninstall.
Disputed (incorrect / parity-violating):
- "Add preserve_order to serde_json in graphify-scip /
-multigraph-compat / -semantic Cargo.toml": redundant. The
workspace root already enables `preserve_order` and every crate
that uses `serde_json = { workspace = true }` inherits it.
- "Require force=true on size-cap rejection in
`graphify-export/src/json.rs::would_shrink_graph`": the Python
reference also silently treats a size-capped existing file as "no
existing graph" and proceeds. Tightening this would diverge from
the documented Python behaviour.
Declined (low-value):
- "Short-circuit `resolve_seed` iteration to avoid temporary Vecs"
and "Use `&[&str]` in `cli::affected::cmd_affected`": the Vec
allocations are negligible against the BFS / load cost; the
current code reads more clearly than the proposed short-circuit
scan.
By the will of the Machine God
Two valid findings applied; two repeated dispositions documented.
Applied:
- `graphify-multigraph-compat` `MultigraphCapabilityResult::rust_version`
doc-comment now states the value is the *declared MSRV* from
`workspace.package.rust-version`, not the actual compiler version.
This is intentional — the probe's failure message quotes the
toolchain floor so users can compare against their installed
rustc, and pulling the live rustc version would require a build
script with no parity benefit.
- `graphify-extract` `WORKSPACE_PACKAGE_CACHE` no longer wraps the
package map in an unnecessary `Option`. Switched the static to a
`LazyLock<Mutex<HashMap<...>>>` (since `HashMap::new()` isn't
const, plain `Mutex::new(HashMap::new())` won't compile in a
static). The two call sites were simplified accordingly:
`guard.get(&root)` and `guard.insert(root, packages.clone())`.
Re-disputed (already addressed in earlier rounds; CodeRabbit re-
surfaces them each pass because the underlying constructs look
suspicious to the linter):
- "`Option::is_none_or` requires Rust 1.82+": the workspace
`rust-version = "1.90"` is the MSRV floor, well past 1.82. The
call site in `graphify-diagnostics` is safe on every supported
toolchain.
- "Treat unknown extensions as Some in `language_family`": this
would diverge from Python's
`_LANG_FAMILY.get(src) != _LANG_FAMILY.get(tgt)`, which compares
`Some("py") != None` as a mismatch and drops the edge. Pinned with
the existing `build_drops_inferred_calls_when_one_side_has_unknown_extension`
parity test (added in round 1).
By the will of the Machine God
Two valid findings applied; four documented as repeats.
Applied:
- `src/cli/extract.rs`: chunk-failure error message now mirrors
the Python wording (`[graphify extract] error: all semantic
chunks failed for backend 'X' (N uncached files) - see per-chunk
errors above. ...`) and is platform-neutral: it no longer hard-
codes `pip install "graphifyy[mcp]"`, but suggests the matching
install command for whatever package the per-chunk error names.
- `graphify-security` `format_with_underscores`: rewrote the
manual byte-loop as `digits.as_bytes().rchunks(3).rev().join("_")`
so the right-aligned chunk-by-three intent reads at a glance.
All existing parity tests (boundary, formatting) still pass.
Re-disputed (repeats from earlier rounds):
- "Add `preserve_order` to serde_json in graphify-multigraph-compat /
graphify-diagnostics / graphify-security Cargo.toml": the
workspace root already declares
`serde_json = { version = "1", features = ["preserve_order"] }`,
and every crate that uses `serde_json = { workspace = true }`
inherits the feature. CodeRabbit surfaces this on every pass
because the per-crate manifest doesn't repeat the feature
declaration.
- "`[\W_ ]+` is redundant — `\W` already matches whitespace":
Python's `build.py` has the literal regex `[\W_ ]+` (with the
redundant space). Mirroring it byte-for-byte preserves parity
with the reference implementation. The functional output is
identical either way.
By the will of the Machine God
Replace the hand-rolled `ureq` + AWS Signature V4 implementation with the official AWS SDK. The SDK uses the full credential provider chain (env vars, `~/.aws/credentials` profiles, SSO, IMDS, ECS task roles, STS web identity / assume role), so users no longer have to copy keys into env vars when their workstation already has `aws configure` or `aws sso login` set up. Tighten `detect_backend()` so it only picks Bedrock when credentials appear to be configured. Previously `AWS_REGION` alone was enough to auto-select Bedrock, which then made every extraction chunk fail with "AWS credentials not configured" when only the region was set. The new check looks for the canonical credential-provider entry points (`AWS_ACCESS_KEY_ID` + `AWS_SECRET_ACCESS_KEY`, `AWS_PROFILE`, `AWS_WEB_IDENTITY_TOKEN_FILE`, `AWS_CONTAINER_CREDENTIALS_*`); the actual chain resolution still happens inside the SDK at call time. Drops the now-unused `hex`, `hmac`, and `sha2` dependencies from `graphify-llm`. Adds `aws-config`, `aws-sdk-bedrockruntime`, and a process-wide `OnceLock<tokio::runtime::Runtime>` to drive the async SDK from our otherwise-sync call sites. Existing mockito tests continue to pass against the SDK path through `GRAPHIFY_BEDROCK_BASE_URL`. Adds six new parity tests covering the auto-detect rule (region-alone, static creds, missing-secret, profile, web identity, ECS task role). Glory to the Omnissiah
Replace the per-node `HashMap` allocations in Phase 1 with pre-allocated `Vec<f64>` scratch buffers. The hot loop used to allocate a fresh `HashMap<usize, f64>` for `nbr_community_weight` and a fresh `Vec<usize>` + `sort_unstable()` for `candidates` on every node visit; on a 16k-node graph that meant millions of allocations per Phase-1 sweep and a single CPU pinned at 100% for what felt like an indefinite hang. Now the per-node work is alloc-free: one shared scratch buffer indexed by community id, plus a small "touched" dirty list so the buffer can be cleared in `O(touched)` rather than `O(n)`. Switch the adjacency representation from `Vec<HashMap<usize, f64>>` to `Vec<Vec<(usize, f64)>>`. The hot path only iterates each row — never looks up by neighbour index — so the cache-friendly contiguous layout is strictly better. The `tot` community-total table likewise drops the `HashMap` indirection. Drop the redundant `candidates.sort_unstable()` from Phase 1. The tie-break (`c < best_c`) already implements "smallest community id wins on equal gain" regardless of visit order, so the sort was pure overhead. Parallelise `contract_graph` between Louvain levels with rayon when the contracted graph has at least 4096 supernodes. Phase 1 itself stays sequential — moving node A changes the modularity score for node B, which would break the seeded determinism if interleaved — but the inter-level contraction is embarrassingly parallel by source supernode. Add a `GRAPHIFY_CLUSTER_PROGRESS=1` env var that prints per-level timing to stderr, so a long clustering run looks busy rather than stuck. Existing parity tests still pass (the suite asserts structural properties — coverage, size ordering — rather than specific community IDs, since Louvain is non-deterministic at the algorithm level). By the will of the Machine God
Apply two of the four findings from round 7:
- Move the inline `#[cfg(test)] mod tests` for `env_command_args` from
`graphify-detect/src/shebang.rs` into the integration test file
`crates/graphify-detect/tests/parity_shebang.rs`, matching the
project convention that all tests live under each crate's `tests/`
directory (see `CLAUDE.md`).
- Add `#[must_use]` to the pure helper `strip_graphify_section` in
`graphify-hooks/src/platform/common/install_skill.rs` so callers
that drop the returned `String` are warned at compile time.
Disputed findings (not applied):
- `graphify-build/src/dedup_label.rs`: CodeRabbit suggested replacing
`#[allow(clippy::expect_used)] ... .expect("...")` with
`#[allow(clippy::unwrap_used)] ... .unwrap()`. This contradicts the
project preference for `.expect("reason")` over `.unwrap()` (the
`expect` message documents the invariant). The current allow attribute
matches the call form already used.
- `graphify-extract/src/workspace.rs`: CodeRabbit suggested an
`O(1)` short-circuit in `resolve_workspace_import` for exact
package-name matches. The function is only reached on bare-specifier
imports during JS/TS extraction (already a slow path), and the
micro-optimisation would obscure the unified prefix-match loop. Not
a measurable win.
Ave Deus Mechanicus
Apply six trivial documentation / `#[must_use]` findings:
- `graphify-hooks/src/platform/common/install_skill.rs`: clarify why
`scope_root_for`'s `unwrap_or(rel)` branch is purely defensive
given that all callers pass `/`-bearing literals from `skill_for`.
- `graphify-llm/src/bedrock.rs`: expand the doc comment on `runtime()`
to explain when its `panic!` is reachable (OS denies thread / fd
resources at process start) and why no recoverable path exists.
- `graphify-scip/src/lib.rs`: add `#[must_use]` to pure private
helpers `resolve_relationship_target`, `is_strictly_true`,
`scip_relation_for`, `coerce_str`, `scip_kind_to_file_type`, and
`build_scip_metadata`.
- `graphify-security/src/graph_size.rs`: document why the
`unwrap_or("")` fallback in `format_with_underscores` is
unreachable in practice (`u64::to_string` only emits ASCII).
- `graphify-extract/src/workspace.rs`: document the
poisoned-mutex behaviour on the `WORKSPACE_PACKAGE_CACHE.lock()`
insert — we deliberately skip caching rather than fail callers.
Disputed findings (not applied):
- `src/cli/affected.rs` line 16, `src/cli/diagnose.rs` line 39
(both marked "critical"): CodeRabbit claimed `map_or_else` cannot
accept `std::path::Path::to_path_buf` as a function pointer. False
positive — `Path::to_path_buf` has signature `fn(&Path) -> PathBuf`
which is exactly the type `Option<&Path>::map_or_else` expects for
its second argument. The code compiles clean under
`cargo clippy --all-targets --all-features --workspace -- -D warnings`.
- `graphify-llm/src/bedrock.rs` 303-318 (minor): CodeRabbit suggested
replacing the substring match in `map_sdk_error` with typed
downcasting through the SDK error chain. The SDK exposes only
`SdkError::DispatchFailure(ConnectorError)`; reaching the underlying
credential-provider error requires walking
`Error::source()` through several un-public types. The current
substring check covers both `CredentialsNotLoaded` and the generic
"no credentials" message the SDK emits today, with a safe
`LlmError::Http` fallback for everything else. Keeping the current
shape until the SDK exposes a stable typed accessor.
By the will of the Machine God
Apply five findings: - `graphify-diagnostics/src/lib.rs`: rewrite the double-negated `_invalid` check on `canonical_edges` as the positive `is_some_and(|s| !s.is_empty())` form so the intent reads forward. - `graphify-multigraph-compat/src/lib.rs`: drop the no-op `let _ = GraphKind::MultiGraph;` from `probe_to_undirected_preserves_type` and expand the comment explaining why no runtime check is needed (the enum makes the property statically true). - `graphify-scip/src/lib.rs`: replace the defensive `target_node_id.unwrap_or_default()` with an `expect()` that documents the upstream invariant — both branches set `target_node_id` before reaching this point, and silently falling back to an empty string was hiding the invariant. - `graphify-extract/src/workspace.rs`: teach `workspace_globs` to honour inline `# comment` trailers on bare entries while preserving `#` characters that appear inside quoted globs. Factored out `parse_packages_entry` to keep the cases readable. - `graphify-hooks/tests/parity.rs`: add five integration tests for `install_platform_skill_project` and `uninstall_platform_skill_project` covering the happy path, idempotent re-install, unknown-platform error, end-to-end uninstall, and the no-op uninstall on a clean project dir. Disputed finding (not applied): - `graphify-scip/src/lib.rs` 66-69: CodeRabbit suggested swapping `global_index: IndexMap<String, Vec<String>>` for `IndexMap<String, IndexSet<String>>` so the membership check inside `emit_symbol_records` is `O(1)` instead of `O(k)`. In practice `k` is the number of duplicate symbol declarations for a single name across documents — almost always 1, sometimes 2-3, never large. The refactor would also touch the public-ish surface of `resolve_relationship_target` (which consumes the value as a slice to count candidates). Not a measurable win for the added code churn. By the will of the Machine God
On a real-world ~16k-node graph (`~/didx/mono`) Louvain reaches
level 5 then hangs with one CPU pinned at 100%. The hang is in
phase1's inner `loop { ... if !improved_this_pass break }` — known
Louvain pathology where small groups of nodes oscillate between
neighbouring communities indefinitely, keeping `improved_this_pass`
true on every iteration.
scikit-network, igraph, and graspologic all cap the inner loop;
`NetworkX` is the outlier that runs uncapped. Adopt the standard
behaviour with `MAX_INNER_PASSES = 100`. 100 is more than enough
for any well-behaved input (production graphs converge in under
10 passes), and the cap exists purely so a pathological input
can't lock up the CLI.
When the cap is reached, emit a stderr warning so users can tell
the partition is best-effort rather than locally optimal. Under
`GRAPHIFY_CLUSTER_PROGRESS=1`, also print per-pass move counts for
the first three passes and every tenth pass so an in-flight
oscillation surfaces in the log.
By the will of the Machine God
The Python reference (`graphify-py/graphify/cluster.py::_partition`)
tries `graspologic.partition.leiden` first and only falls back to
`nx.community.louvain_communities` when graspologic isn't installed.
The Rust port previously shipped Louvain only — a deliberate but
incorrect divergence from the parity rule.
Now ships Leiden via the pure-Rust `leiden-rs` crate (0.8) as the
default partitioner, with our existing hand-rolled Louvain preserved
as a backup that can be forced via `GRAPHIFY_CLUSTER_BACKEND=louvain`.
Why this matters in practice: on a real-world ~16k-node monorepo
(`~/didx/mono`), Louvain's Phase 1 hit the 100-pass safety cap added
in the previous commit when five nodes oscillated between two
neighbouring communities — the classic flip-flop pathology that
motivated Traag et al.'s 2019 Leiden paper. Leiden's refinement
phase guarantees connected sub-communities and its Fast Local Move
(FLM) algorithm bounds Phase-1 work to O(moves), so oscillation
pathologies converge cleanly instead of looping.
Implementation:
- `crates/graphify-cluster/src/leiden.rs`: new wrapper with the same
`partition(nodes, edges, resolution) -> HashMap<String, usize>`
signature as `louvain::partition`. Sorts nodes and edges before
feeding `GraphDataBuilder`, seeds at 42 (matches Python's
`random_seed=42`), drops self-loops, returns an empty map on any
builder/run error.
- `edge_list.rs::run_partition`: reads `GRAPHIFY_CLUSTER_BACKEND`
(defaults to `leiden`); unknown values fall through to Leiden.
- `Cargo.toml`: `leiden-rs = { version = "0.8", default-features =
false, features = ["rayon"] }`. The default features pull in `cli`
and `gryf` which we don't need; disabling them keeps the dep
surface narrow (`rustc-hash`, `rand 0.9` as new transitives — no C
code).
- `tests/parity.rs`: three new tests pinning the Leiden default
path, the Louvain env-override path, and the unknown-backend
fall-through.
- Docs + `.claude/local/notes/module_cluster.md` updated to
describe the new primary path.
Glory to the Omnissiah
`extract` and `cluster-only` were both hard-coded to print `(Louvain, ...)` in their step-4 / step-2 progress lines, but the default partitioner is now Leiden (see the previous commit). Read the same `GRAPHIFY_CLUSTER_BACKEND` env var the cluster crate consults so the label matches what's actually running. By the will of the Machine God
Apply seven findings:
- `crates/graphify-multigraph-compat/Cargo.toml`,
`crates/graphify-scip/Cargo.toml`: enable
`serde_json/preserve_order`. Both crates build JSON via `json!`, so
insertion ordering is observable through the values they hand back to
the caller and must match the rest of the workspace.
- `crates/graphify-cluster/src/edge_list.rs`: emit a stderr warning
when `GRAPHIFY_CLUSTER_BACKEND` is set to anything other than
`leiden` or `louvain`. Previously a typo silently used Leiden;
surfacing the fall-through makes misconfigurations debuggable.
- `crates/graphify-cluster/tests/parity.rs`: serialise the three
backend-selection tests with `#[serial_test::serial]`. They mutate
a process-global env var (`GRAPHIFY_CLUSTER_BACKEND`); without
serialisation, parallel test execution could race on the override
and produce flake.
- `crates/graphify-cluster/Cargo.toml`: add `serial_test` as a
dev-dependency (already in `[workspace.dependencies]`).
- `crates/graphify-extract/src/lang_configs.rs`: comment all three
`import_types` arrays explaining why `export_statement` is listed
alongside `import_statement` — re-exports (`export { x } from ...`
/ `export * from ...`) need the same cross-module edge handling.
- `src/cli/affected.rs`, `src/cli/diagnose.rs`: drop the
fully-qualified `std::path::Path::to_path_buf` and use the in-scope
`Path` import (`Path::to_path_buf`).
- `.claude/commands/resync-py.md`: update the Phase 4 gate command
from bare `cargo nextest run` to `cargo nextest run --workspace`
so it matches the Definition of Done section below it.
Disputed findings (not applied):
- `crates/graphify-export/src/json.rs` (`would_shrink_graph`):
CodeRabbit suggested propagating size-cap errors as
`Result<bool, ExportError>`. The current `is_err() → return false`
is intentional: if the existing on-disk graph is too large to
read safely, we deliberately skip the shrink check and let the
overwrite proceed. Refusing to write would leave the oversized
file in place forever, which is the worse outcome.
- `crates/graphify-llm/Cargo.toml` (aws-config / aws-sdk-bedrockruntime
version pins): the suggestion was to tighten `version = "1"` to a
patch-level pin (e.g. `"1.10"`). The workspace's other deps use
major-only pins consistently (`tokio = "1"`, `anyhow = "1"`,
`serde = "1"`, etc.); honouring that convention is more important
than the marginal upgrade-control benefit of a patch pin.
Ave Deus Mechanicus
Apply eight findings: - `graphify-llm/src/bedrock.rs`: expand the `runtime()` panic message to include an actionable hint (check `ulimit` for FDs / threads), matching the doc comment's stated failure mode. - `graphify-security/Cargo.toml`, `graphify-semantic/Cargo.toml`: enable `serde_json/preserve_order` for the two remaining crates that build / inspect `Value` and hand the result back to JSON-emitting callers; this completes the workspace-wide audit. - `graphify-semantic/src/lib.rs`: simplify the error-message helper to use `VALID_SEMANTIC_FILE_TYPES.to_vec()` (the constant is already `&[&str]`), eliminating the awkward `Vec<&&str>`. - `graphify-diagnostics/src/lib.rs`: stop silently swallowing `serde_json::to_string` failures in `safe_text` and `exact_signature`. The function can only fail on OOM or NaN/Infinity numbers inside the input; an empty fallback hid malformed inputs from operators, so emit a stderr warning before returning `""`. - `graphify-detect/src/extensions.rs`: clarify the doc comment to say `.ets` was added *after* `.ejs`, matching the array order. - `graphify-scip/src/lib.rs` (`emit_relationships`): replace the Python-parity rationale on `#[allow(clippy::too_many_arguments)]` with a Rust-specific one — the four `&mut` parameters carry disjoint accesses that the borrow checker reasons about more cleanly than a single `&mut Context` struct would. - `graphify-cluster/src/leiden.rs`: document the `partial_cmp` fallback in the edge sort comparator — NaN weights resolve to `Ordering::Equal` so the sort is total; that's fine because edge order only feeds Leiden determinism, not numeric semantics. - `graphify-cluster/tests/parity.rs`: rewrite the `BackendGuard` SAFETY comments to distinguish *cleanup* (Drop restoration) from *serialisation* (`#[serial_test::serial]` on each test). Both are required; the previous comment implied the guard alone gave mutual exclusion. Disputed findings (not applied): - `graphify-cluster/src/louvain.rs` `usize::is_multiple_of` (marked "critical"): CodeRabbit claimed this fails to compile on stable. False positive — `is_multiple_of` for unsigned integers was stabilised in Rust 1.87; the workspace MSRV (`rust-version = "1.90"`) is well above that. Build and tests pass clean under `cargo clippy --all-targets --all-features --workspace -- -D warnings`. By the will of the Machine God
Apply all five findings:
- `graphify-build/src/ingest.rs`: drop the redundant
`to_ascii_lowercase()` from `language_family`. The function is only
called with extensions that pass through `source_file_ext`, which
already returns a lowercased extension. Document the precondition
on the function so a future caller can't violate it silently.
- `graphify-cluster/src/louvain.rs`: replace `pass.is_multiple_of(10)`
with `pass % 10 == 0`. The `is_multiple_of` integer method is
stable on our MSRV (`rust-version = "1.90"`, method stabilised in
1.87) so the previous form built fine, but CodeRabbit's lint
database flags it as if it were unstable on every review pass —
ending the back-and-forth is cheaper than disputing repeatedly.
- `graphify-serve/tests/parity.rs`: extend
`test_load_graph_accepts_under_cap` to use the canonical
`node_link_data` JSON shape (`directed`, `multigraph`, `nodes`,
`links`) so the test exercises the real loader path rather than
a degenerate `{"nodes": [], "edges": []}` payload.
- `graphify-diagnostics/src/lib.rs::exact_signature`: rewrite
normalisation as a single forward pass over the original map.
Avoids the upfront `orig.clone()` and the branching
`remove`/`insert` dance, while preserving the precedence rule
that `source`/`target` always win over `from`/`to`.
- `graphify-diagnostics/src/lib.rs`: extract `display_input_path`
for the diagnostic header's `input:` line. The previous chain
(`stringify(...).strip_prefix('"').unwrap_or(...).replace('"', "")`)
was a defensive no-op for `Value::String` and a fragile guess for
other shapes. The new helper round-trips through
`serde_json::from_str::<String>` to unescape JSON-encoded paths
before falling back to `trim_matches('"')`.
By the will of the Machine God
Two follow-ups from the branch-wide audit: - `README.md`: the `graphify-cluster` line in the workspace tree still described the crate as "deterministic Louvain clustering". Replace with "community detection (Leiden, Louvain fallback)" to match `crates/graphify-cluster/src/lib.rs`'s module docs. - `USAGE.md` (Determinism note): the cluster paragraph still said "deterministic Louvain pass". Update to spell out the actual behaviour — Leiden via `leiden-rs` with `random_seed=42` by default, Louvain only when `GRAPHIFY_CLUSTER_BACKEND=louvain` is set — so the deterministic-output promise stays accurate. Everything else in both docs already covered the resync work: new commands (`affected`, `diagnose multigraph`), the `--project` install flag, the Bedrock SDK-based credential resolution, all five new crates in the workspace tree, and the three new `GRAPHIFY_BEDROCK_BASE_URL` / `GRAPHIFY_CLUSTER_PROGRESS` / `GRAPHIFY_CLUSTER_BACKEND` env vars. Glory to the Omnissiah
Apply four findings: - `graphify-cluster/src/leiden.rs`, `louvain.rs`: change both partitioners to return `IndexMap<String, usize>` instead of `HashMap`. The function sorts its input nodes for determinism but then loses that ordering at the boundary because `HashMap` iteration is unordered. The caller (`edge_list::run_partition` → `cluster::cluster`) iterates the result to build per-community node lists; while the final output is alphabetically sorted within each community, the intermediate insertion ordering still leaks through (e.g. into the `raw.keys().copied().max()` for next-id allocation when nodes share community ids). `IndexMap` preserves the deterministic order we paid to set. - `graphify-cluster/src/edge_list.rs`, `src/cli/extract.rs`, `src/cli/cluster_only.rs`: make `GRAPHIFY_CLUSTER_BACKEND` parsing case-insensitive. Previously `Louvain` / `LOUVAIN` silently fell through to Leiden with a "unknown backend" warning even though the intent was obvious. Lowercase the env value at parse time and use `eq_ignore_ascii_case` for the label decision. - `graphify-multigraph-compat/src/lib.rs::probe_duplicate_overwrite`: tighten the assertion. The previous probe accepted "at least one edge", which would silently pass if a future Graph refactor switched to NetworkX-style overwrite semantics. The new check asserts exactly two same-`key` parallel edges coexist, which is the actual Rust port invariant the probe documents. - `graphify-affected/src/lib.rs`: add `#[must_use]` to the three private pure helpers `build_in_edges`, `node_label`, and `format_location`. Disputed finding (not applied): - `graphify-multigraph-compat/src/lib.rs::probe_remove_edges_two_tuple`: CodeRabbit suggested rewriting this probe to actually call a `Graph::remove_edges_from` primitive. That primitive still doesn't exist on `graphify-build::Graph`; the probe is a placeholder slot that records the current edge count and will fail if a future refactor breaks the precondition. Rewriting it now would require inventing a method that nothing else needs. Ave Deus Mechanicus
|
Warning Review limit reached
Your plan includes 1 review of capacity. Refill in 46 minutes and 23 seconds. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more review capacity refills, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than trial, open-source, and free plans. In all cases, review capacity refills continuously over time. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds multiple new workspace crates (affected, diagnostics, multigraph-compat, scip, semantic), enforces graph-file size caps, broadens Unicode-normalization and shebang handling, adds Leiden clustering and backend selection, integrates Bedrock via AWS SDK, exposes CLI subcommands (affected, diagnose) and project-scoped installs, and updates tests, docs, and CI cache keys. ChangesWorkspace, manifests, and dependencies
New crates and large libraries
Security, file-size guards, metadata sanitization
Extraction, postprocessing, symbol resolution, and language support
Label normalization and deduplication
Clustering: Leiden backend and Louvain improvements
LLM and Bedrock backend changes
CLI, hooks, and project-scoped installs
Diagnostics, affected CLI, and export tweaks
Tests, lint adjustments, and minor UX
Estimated code review effort: 🎯 5 (Critical) | ⏱️ ~120 minutes
✨ Finishing Touches🧪 Generate unit tests (beta)
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
`aws-sdk-bedrockruntime`'s default features (and `aws-config`'s transitive defaults through `aws-sdk-sso` / `aws-sdk-ssooidc` / `aws-sdk-sts`) include a `rustls` feature that activates `aws-smithy-runtime/tls-rustls` and pulls in the legacy stack: `rustls 0.21.8` + `hyper-rustls 0.24.2` + `rustls-webpki 0.101.7`. `rustls-webpki 0.101.7` is vulnerable to three RUSTSEC advisories: - RUSTSEC-2026-0099: wildcard cert name-constraint bypass - RUSTSEC-2026-0098: URI-name constraints silently ignored - RUSTSEC-2026-0104: reachable panic in CRL parsing Disable default features on both crates and re-enable only what we actually use: `behavior-version-latest`, `default-https-client` (modern `rustls-aws-lc` via aws-smithy-http-client 1.1.12), and `rt-tokio`. For `aws-config`, also keep `credentials-process` and `sso` since the default credential provider chain expects them. After the change `cargo tree -i rustls-webpki@0.101.7` finds no matches and only `rustls-webpki 0.103.13` (patched) remains. Glory to the Omnissiah
`api_timeout()` was migrated to `Duration::from_mins(10)` for the new `clippy::duration_suboptimal_units` lint (stable in Rust 1.95). The four test assertions still compared against `Duration::from_secs(600)`, which the lint now flags identically. Switch the test side to match the source. By the will of the Machine God
Apply six findings + bundle the workspace `rust-version` bump that upstream the AWS SDK rustls-feature drop / `Duration::from_mins` migration in the two preceding commits depend on. - `Cargo.toml`: bump `rust-version` to 1.95 so the dep-update path taken by `e49bc9a` (AWS SDK rustls drop) and `d42b96b` (`Duration::from_mins`) actually compiles. `Cargo.lock` follows. - `crates/graphify-llm/src/openai_compat.rs`: update `api_timeout`'s default to `Duration::from_mins(10)` to match the test-side migration in `d42b96b` and silence `clippy::duration_suboptimal_units` (stable in Rust 1.95). - `crates/graphify-diagnostics/Cargo.toml`: enable `serde_json/preserve_order` so the diagnostic JSON output preserves key insertion order across runs. - `crates/graphify-extract/src/workspace.rs::resolve_workspace_import`: add an `O(1)` exact-name lookup before the prefix-match scan, so large monorepos (hundreds of packages) don't pay an O(n) string scan per resolved import. - `crates/graphify-extract/src/workspace.rs::walk_subdirs`: convert the recursive walker to an iterative `Vec<PathBuf>` work stack so a pathologically-deep workspace (symlinked, generated, etc.) can't overflow the call stack. - `crates/graphify-semantic/src/lib.rs`: change the `keep_edges` filter to drop non-object edges instead of preserving them. The prior `return true` for `as_object() == None` would let malformed garbage survive into downstream validation; treating malformed edges as removable is the more honest behaviour. - `crates/graphify-scip/src/lib.rs`: wrap the four mutable scratch buffers in `emit_relationships` (`nodes`, `edges`, `seen_node_ids`, `seen_edges`) into an `EmitContext` struct, simplifying the signature without changing borrow semantics. - `crates/graphify-build/src/dedup_label.rs::norm_label`: drop the redundant explicit `: String` annotations and rely on inference. Disputed finding (not applied): - `crates/graphify-extract/src/extractors/bash.rs::is_inside_expansion`: CodeRabbit asked for a `HashMap<NodeId, bool>` memo cache on the ancestor walk. In real bash sources the ancestor chain above any given `command` node is shallow (parameter / command / arithmetic expansions don't nest deeply) and the call happens once per `command` visit. Adding a cache here would buy back microseconds on the typical input and complicate the extractor's scratch state for negligible benefit. Glory to the Omnissiah
Apply seven findings:
- `crates/graphify-build/src/ingest.rs`: add `#[must_use]` to
`source_file_ext`.
- `crates/graphify-cluster/src/louvain.rs`: sort each adjacency row
by neighbour id when materialising from the `HashMap` build
buffer (both the initial graph and `contract_graph`'s super-node
rows). Without this, Phase 1's per-node neighbour visit order is
drawn from `HashMap` iteration, which the standard library does
not guarantee to be stable across runs — and floating-point
modularity ties can resolve to different best-community choices,
breaking byte-identical determinism for graphs at the tie
boundary.
- `crates/graphify-diagnostics/src/lib.rs`: stop stringifying
`(source, target)` pair keys with `format!("{s}->{t}")` for the
`directed_same_endpoint_collapsed_edges` /
`undirected_same_endpoint_collapsed_edges` metrics. Node ids
can themselves contain `->`, which would silently merge distinct
pairs and undercount the collapsed-edge count. Make `count_extra`
generic over the key type and pass the tuple-keyed `IndexMap`s
directly.
- `crates/graphify-extract/tests/parity.rs`: rewrite the workspace
re-export edge assertion to check any-of-edges via
`.iter().any(|e| ...)` so the test is order-independent.
- `crates/graphify-hooks/src/platform/common/install_skill.rs`:
propagate the read / remove / write I/O errors from
`uninstall_platform_skill_project`'s CLAUDE.md cleanup branch.
The previous `if let Ok(content) = fs::read_to_string(...)` and
`let _ = fs::remove_file(...)` calls silently swallowed
failures, which lets uninstall report success on partial
failure — directly contradicting the function's documented
`HooksError::Io` contract.
- `crates/graphify-html/src/callflow/loader.rs`: fix the
`# Errors` doc for `load_graph` to describe the actual mapping
(malformed JSON → `HtmlError::Io(InvalidData)`) rather than the
non-existent `HtmlError::EmptyGraph`.
- `crates/graphify-llm/tests/parity.rs`, `bedrock_http.rs`,
`llm_error_paths.rs`: serialise every env-mutating test with
`#[serial_test::serial(env)]`. Cargo nextest's per-binary
parallelism could otherwise race on `AWS_*` / `OPENAI_API_KEY`
/ etc. (each `EnvGuard` restores per-test, but does not provide
mutual exclusion).
- `src/cli/extract.rs`: validate `--exclude-hubs` is a fraction in
`[0.0, 1.0]` before scaling to a percentile. Previously a stray
`--exclude-hubs 95` would silently become a 9500% percentile
inside `graphify_cluster::cluster`. Mirrors the same guard
already in `cluster_only.rs`.
Disputed findings (not applied — all are variants of the same
"expect → unwrap" suggestion that directly contradicts the
project's documented preference for `.expect("reason")` over
`.unwrap()` to make invariants visible in source rather than
hidden behind `#[allow(clippy::unwrap_used)]`):
- `graphify-build/src/dedup_label.rs` (`NON_WORD`, `CHUNK_SUFFIX`).
- `graphify-dedup/src/score.rs` (`NON_WORD`).
- `graphify-diagnostics/src/lib.rs` (`SUPPRESSION_DECL`, `TYPE_TUPLE`).
- `graphify-scip/src/lib.rs` (`IDENT_SAFE`, `target_node_id.expect`).
- `graphify-semantic/src/lib.rs` (`SEMANTIC_ID_RE`, `SENTENCE_PUNCT`).
- `graphify-export/src/json.rs::would_shrink_graph`: CodeRabbit
suggested refusing writes when the existing file fails the size
cap. The current behaviour (proceed with overwrite) is the
safer option: if the on-disk graph is too large to read for the
shrink-comparison we still want the freshly-built bounded
graph to replace it; refusing would leave the memory-bomb /
oversized file on disk indefinitely.
Glory to the Omnissiah
The user wanted `unwrap_used = "forbid"` so an unhandled `unwrap`
becomes a build error rather than a convention violation. `forbid`
turned out to be incompatible with clap's derive macros — the
`#[derive(Parser, Subcommand)]` expansion embeds
`#[allow(clippy::restriction)]`, which conflicts with a forbidden
member of that category. Switched to `deny`, which still rejects
every `.unwrap()` as a hard error in our own code, and let the
macro-generated allows through.
In support of the new rule:
- `Cargo.toml`: keep `clippy::unwrap_used = "deny"` (the
`"forbid"` flip that motivated this audit is reverted because of
the clap-derive incompatibility above).
- 60 test files: `.unwrap()` → `.expect("<reason>")` derived per
call from a small heuristic table covering the common test-fixture
shapes (`tempdir`, `fs::write` / `read_to_string`,
`serde_json::from_str`, `Value::as_str` / `as_array` / ...,
`Regex::new`, `build_from_json`). Anything outside the table maps
to a generic `"test invariant"` — better than no message at all
and the original panic site is preserved by `expect`'s own
reporting.
- 6 tests: `.unwrap_err()` → `.expect_err("expected Err")`.
- Each affected test file's file-top `#![allow(clippy::unwrap_used)]`
is removed; `clippy::expect_used` stays because tests use
`expect()` as their assertion vehicle.
- `crates/graphify-html/src/tree.rs`: the inline
`#[cfg(test)] mod tests` block gains a module-scoped
`#[allow(clippy::expect_used)]` with the same justification as
the integration test files.
- `crates/graphify-report/src/util.rs`,
`crates/graphify-security/src/metadata.rs`: convert the two
regex statics that were still using
`.unwrap()` + `#[allow(clippy::unwrap_used)]` to the
`.expect("literal pattern is valid")` +
`#[allow(clippy::expect_used)]` form the rest of the workspace
uses for `LazyLock<Regex>` initialisers.
- `crates/graphify-extract/src/extractors/pascal.rs`: normalise the
nine remaining `.expect("static")` messages to the
workspace-standard `"literal pattern is valid"`.
Tests stay green (`cargo nextest run --workspace`: 1362 passed) and
`cargo clippy --all-targets --all-features --workspace -- -D warnings`
is silent.
Glory to the Omnissiah
…files
Project convention is that tests live in dedicated `_tests.rs` (next
to the source file) or `tests/parity.rs` (integration tests), not in
inline `#[cfg(test)] mod tests { ... }` blocks at the bottom of src
files. The following nine modules still had inline tests; move each
of them to a sibling `<name>_tests.rs` file and include via
`#[path] mod`:
- `graphify-detect/src/extensions.rs` → `extensions_tests.rs`
- `graphify-detect/src/ignore.rs` → `ignore_tests.rs`
- `graphify-detect/src/sensitive.rs` → `sensitive_tests.rs`
- `graphify-extract/src/ids.rs` → `ids_tests.rs`
- `graphify-extract/src/postprocess.rs` → `postprocess_tests.rs`
- `graphify-extract/src/symbol_resolution.rs` → `symbol_resolution_tests.rs`
- `graphify-html/src/tree.rs` → `tree_tests.rs`
- `graphify-security/src/graph_size.rs` → `graph_size_tests.rs`
- `graphify-security/src/metadata.rs` → `metadata_tests.rs`
Five of those target paths already existed as **orphan** files —
written previously but never linked into any module tree, so they
never compiled and never ran. Those orphan files were the "intended"
versions with richer per-test doc comments and slightly stronger
assertion messages than the inline blocks. Restoring the orphan
content (rather than overwriting from the inline block) keeps that
richer documentation while now actually running the tests. The four
new files (`postprocess_tests.rs`, `symbol_resolution_tests.rs`,
`graph_size_tests.rs`, `metadata_tests.rs`) come from the inline
blocks directly because no orphan existed.
Each test file gets a module-level `#![allow(clippy::expect_used)]`
since tests use `.expect("...")` as their assertion vehicle.
The src-side declaration uses `#[path = "<name>_tests.rs"]` because
when a parent module lives in `<name>.rs` (rather than
`<name>/mod.rs`), Rust's default submodule resolution would look at
`<name>/<name>_tests.rs`. The explicit path attribute keeps the
test file as a sibling rather than forcing a directory split.
Net result: same 1362 tests passing, but the previously-dead orphan
tests are now exercising real code, and the per-test doc comments
the orphans carried survive the move.
Ave Deus Mechanicus
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
- `graphify-export`: shrink-protection now fails *closed* — if an existing `graph.json` blows the memory-bomb cap, refuse the overwrite instead of letting `to_json(force=false)` silently replace it with a smaller file. - `graphify-extract` (bash): top-level commands are now actually attributed to the synthesised `__entry__` node via a dedicated walker that mirrors `walk_calls(root, entry_nid, ...)` from the Python reference, instead of leaving the node orphaned. - `graphify-benchmark`: tighten doc comments — `SecurityError` is no longer described as file-size-only, and `query_subgraph_tokens` now documents that `query_terms` already lowercases tokens so the BFS seeding match is case-insensitive. - `graphify-llm`, `graphify-hooks`: add `#[serial_test::serial(env)]` / `serial(home_env)` to the remaining env-mutating tests so they cannot race with each other under `cargo nextest`. - `graphify-scip`: drop the unused `language` field from `SymbolRecord` (Python carries it but never reads it back) and prefix the public `_language` parameter to match. By the Omnissiah's grace.
Reorder the cargo cache key segments so `matrix.arch` precedes the job name, keeping per-architecture caches strictly partitioned. Tighten `restore-keys` to require a matching `rustc` fingerprint so stale caches from older toolchains are not reused across runs. Ave Deus Mechanicus
Per-crate findings collected from scoped reviews across the branch:
- `graphify-extract` (bash): extract the duplicated command-emit logic
from `walk_calls_bash` / `walk_calls_top_level_bash` into a single
`emit_call_edge_if_valid` helper; drop the divergent `BASH_SKIP`
filter (Python's `walk_calls` uses only `defined_functions`, so the
skip list created false negatives for shadowed builtins like
`source`); remove the leftover `file_nid → source` edge from the
shadow branch so attribution flows through the entry walker like the
Python reference.
- `graphify-extract` (workspace): track canonical paths in
`walk_subdirs` so a symlink cycle (`a/b -> ../a`) can no longer loop
forever.
- `graphify-llm`: drop `#[serial_test::serial(env)]` from tests that
don't touch env vars (they were forcing serial execution for no
reason); document the `(merged, failed, total)` tuple returned by
`extract_corpus_parallel_with_total`.
- `graphify-export`: convert `obsidian_export` tests to
`Result<_, Box<dyn Error>>` returns with `?` propagation, drop the
file-level `clippy::expect_used` allow, isolate the few remaining
`expect()`s inside the `fixture_graph` helper.
- `graphify-diagnostics`: tighten `diagnose_file_rejects_non_object_input`
to assert the exact `DiagnosticsError::NotAnObject` variant; switch
three `diagnose_file` tests to `?`-returning signatures.
- `graphify-detect`: `#[must_use]` on private `basename` helper.
- `graphify-benchmark`, `graphify-prs`, `graphify-ingest`: replace the
generic `expect("test invariant")` placeholders on hot paths with
descriptive messages that name the violated invariant.
- `graphify-hooks` tests: use the imported `#[serial(home_env)]` form
instead of the fully-qualified path, matching the rest of the file.
- `graphify-scip`: document the SHA-256 → 64 hex char invariant
guarding the 12-char slice in `make_scip_node_id`.
- `graphify-security`: `#[must_use]` on `format_with_underscores`; the
fallible `unwrap_or("")` in its UTF-8 conversion becomes a documented
`.expect(...)` since `u64::to_string` only emits ASCII; rename the
`CONTROL_CHARS` regex expect message to the project's
`"static … regex"` style.
- `graphify-report`: same expect-message rename for `UNSAFE_CHARS` /
`MD_EXT`.
By the Omnissiah's grace.
Final pass over the remaining per-crate reviews:
- `graphify-semantic`: skip the temporary `String` allocation in the
hyperedge survivor filter (`surviving_ids.contains(s)` works directly
on `&str` since `IndexSet<String>` accepts an `Equivalent` lookup);
rename `sanitize_preserves_short_concept_nodes` to
`sanitize_removes_concept_file_type_nodes` so the test name matches
the behaviour it actually asserts.
- `graphify-transcribe`: tighten the `talk.txt` write expect message
from `test invariant` to `write fixture` to match the other fixture
writes in that test.
- `graphify-validate`: describe what `assert_valid` is rejecting so a
failing test panic-message reads like the contract instead of a
generic "expected Err".
Following findings were verified as false positives and not changed:
- src/cli/{affected,diagnose}.rs: CodeRabbit flagged a missing graph
file-size cap, but both `graphify_affected::load_graph` and
`graphify_diagnostics::diagnose_file` already invoke
`graphify_security::check_graph_file_size_cap` at the top of their
bodies.
- graphify-wiki tests: CodeRabbit suggested adding `clippy::unwrap_used`
to the file allow list; the file has zero `.unwrap()` calls, and
broadening the allow would invite drift away from `.expect("reason")`
which the project deliberately migrated to.
By the Omnissiah's grace.
The workspace dep already pins `features = ["preserve_order"]`, and `workspace = true` propagates that to every member crate. The handful of crates that re-declared the feature locally were a no-op that just made the manifests look inconsistent — drop the redundant overrides so the workspace stays the single source of truth. By the Omnissiah's grace.
Pure helper returning `Option<&'static str>` — discarding the result is always a bug. Caught in the pass-3 validation review of `graphify-build`. By the Omnissiah's grace.
The skill content that `graphify install` writes was sourced from `graphify-py/graphify/skill*.md` — twelve files, ~14k lines total — every one of which orchestrates the pipeline by importing `graphify.detect`, `graphify.extract`, `graphify.build`, `graphify.cache` etc. from inline `$(cat .graphify_python) -c "..."` blocks and tells the host agent to install via `pip install graphifyy` / `uv tool install graphifyy`. None of that applies to the Rust distribution: this binary is a single self-contained executable, the whole pipeline runs as one `graphify extract <PATH>` call, and there's no Python interpreter to plumb through `graphify-out/.graphify_python`. Replace the embedded skills with a workspace-local Rust-native version: - `crates/graphify-hooks/skills/skill.md` is the new canonical skill. Install line becomes `cargo install --git https://github.com/bunkerlab-net/graphify.git`. The orchestration steps collapse to "ensure `graphify` is on PATH, set one LLM credential, run `graphify extract <PATH>`, then quote the God Nodes / Surprising Connections / Suggested Questions from `GRAPH_REPORT.md`." Subsequent sections (`query`, `path`, `explain`, `add`, `update`, `cluster-only`, `watch`, hook install) each become a single CLI invocation instead of a multi-step Python pipeline. - `crates/graphify-hooks/skills/skill-vscode.md` is a thinned variant for the Copilot CLI surface, mirroring the original VS Code skill's intent. - `crates/graphify-hooks/src/platform/common/skills.rs` switches every per-platform constant (`SKILL_CODEX_MD`, `SKILL_AIDER_MD`, `SKILL_COPILOT_MD`, …) to point at the canonical skill. The Python variants existed solely to encode bespoke per-platform subagent dispatch syntax — the Rust binary doesn't fan out via host subagents, so a single skill is correct for every platform. VS Code keeps its own variant. - `uninstall_all` now suggests `cargo uninstall graphify` instead of `pip uninstall graphifyy` in its closing line. - `graphify-google::GoogleError::XlsxCallbackMissing` no longer points at `pip install graphifyy[office,google]`; describe the Rust API contract (callback wired up by the CLI, callers supplying their own must produce markdown from the temp `.xlsx`) instead. The `graphify-py/` submodule is still on disk as the parity reference for tests — only the *embedded* skill text moves to the workspace. By the Omnissiah's grace.
Cleanups surfaced by the validation pass over the per-crate reviews:
- `graphify-extract` (bash): collapse the redundant substring checks in
`is_literal_command_name` to a single `contains([..])` over the
single metacharacters — `$(` is already covered by `$`, `<(` by `<`
once `<` is added to the set; remove the now-empty
`if shadowed { } else { ... }` branch by checking the non-shadowed
case directly (the source-shadow comment is preserved alongside the
inverted guard).
- `graphify-global` tests: tighten the file-level doc comment to match
the actual allow attribute (`#![allow(clippy::expect_used)]`) — it
used to also mention `unwrap_used` even though the attribute never
enabled it.
- `graphify-google` tests: replace the generic `"test invariant"`
placeholders on the `out` Option unwraps and the
`read_google_shortcut` expect_err with messages that document the
invariant being checked.
By the Omnissiah's grace.
Closing-pass cleanups across the per-crate validation reviews:
- `graphify-llm`: rename `api_timeout_default_is_600s` to
`api_timeout_default_is_10_minutes` so the test name matches its
`Duration::from_mins(10)` assertion; swap the three placeholder
`expect("test invariant")` messages in `ollama_http.rs` for ones that
describe what the mock server is supposed to return.
- `graphify-multigraph-compat` tests: drop the unused file-level
`#![allow(clippy::expect_used)]` — there are no `.expect()` calls in
the file to suppress.
- `graphify-scip`: `#[must_use]` on `first_occurrence_line` to match the
other pure helpers in `lib.rs`.
- `graphify-transcribe` tests: replace `"test invariant"` /
`"expected Err"` placeholders on the cache-hit, force-rerun, and
`BinaryMissing` assertions with the actual invariant being tested.
Following findings were verified as false positives and not changed:
- `graphify-security::metadata::filter_metadata` over-allocates `out`
by `map.len()` even when some keys sanitise to empty; CodeRabbit
suggested counting first. Walking the map twice to save a bounded
micro-allocation in a small-cardinality hot path is the wrong
trade-off — keep the single pass.
- `graphify-scip`, `graphify-semantic` Cargo manifests: CodeRabbit kept
flagging the missing `preserve_order` feature on `serde_json`. The
workspace dep already enables it and `workspace = true` propagates
the feature to every member; the explicit overrides we used to carry
were redundant and got removed in commit `a592640`.
- `graphify-scip` tests parity: CodeRabbit suggested also allowing
`clippy::unwrap_used` at the file level. The project deliberately
migrated all tests to `.expect("reason")`; broadening the allow would
invite drift back to `.unwrap()`.
By the Omnissiah's grace.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
CodeRabbit caught that the nine tests in this file mutate the process-wide environment through `EnvGuard` but were not in the `#[serial_test::serial(env)]` group, so they could race against any other env-touching test under `cargo nextest`. Add the attribute to every `EnvGuard`-using test (the `api_timeout_*` and `resolve_max_tokens_*` groups) so they all share the existing `env` serial lock. Pure tests below them (`derive_ollama_num_ctx_*`, `extraction_messages*`, `plain_messages*`, `safe_parse_response_*`, `call_openai_compat_*`) are unchanged — they don't touch env. By the Omnissiah's grace.
This was referenced May 26, 2026
This was referenced Jun 15, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Tracks the
graphify-pysubmodule from076e6b7→73c3c33(v0.8.17) and brings the Rust workspace back to behavioural parity.New crates (5)
graphify-affectedaffected.py— reverse-traversal impact analysisgraphify affected <query>graphify-diagnosticsdiagnostics.py—MultiDiGraphmigration diagnosticgraphify diagnose multigraphgraphify-multigraph-compatmultigraph_compat.py— runtime capability probegraphify-scipscip_ingest.py— SCIP-style JSON → graphify extraction formatgraphify-semanticsemantic_cleanup.py— LLM fragment validator/sanitiserWorkspace crate count: 24 → 29.
Extract pipeline (graphify-extract)
bash_entrypointsynthetic node,source-function shadowing prescan,is_inside_expansionparent walk (drops phantom$(...)/<(...)callsedges), literal-command-name guard.export_statementbarrel re-exports emitre_exportsedges + a singleimports_from; generic walker recurses throughexport_statement;index.svelte/index.mjsadded to directory-probe set.workspace.rsmodule (parsespnpm-workspace.yaml, expandspackages/*globs, resolves bare specifiers viaexports/svelte/module/main).merge_swift_extensionscollapsesextension Fooonto canonicalclass Foowith edge remap + dedup.postprocessmodule (disambiguate_colliding_node_ids,rewire_unique_stub_nodes) +symbol_resolutionhelpers (normalise_callable_label,build_label_index, …).Clustering
leiden-rs0.8, seeded at 42), matching Python's preference ofgraspologic.partition.leidenovernx.community.louvain_communities. Hand-rolled Louvain remains asGRAPHIFY_CLUSTER_BACKEND=louvainfallback.Vec<f64>scratch buffers replace per-nodeHashMapallocations,Vec<Vec<(usize, f64)>>adjacency, rayon-parallelisedcontract_graphbetween levels.MAX_INNER_PASSES = 100cap on Phase-1 inner loop to prevent flip-flop hangs on real-world graphs (16k-node~/didx/monoreproduced this).GRAPHIFY_CLUSTER_PROGRESS=1env var prints per-level timing.LLM backend
aws-sdk-bedrockruntime— full credential provider chain (env,~/.aws/credentials, SSO, IMDS, ECS task roles, STS web identity / assume role). Drops the hand-rolledureq+ SigV4 path along withhex/hmac/sha2deps.detect_backend()now requires actual credential entry points before auto-selecting Bedrock (previouslyAWS_REGIONalone was enough).CLI
affectedanddiagnose multigraphsubcommands wired in.--projectflag on every platform install/uninstall subcommand (claude / gemini / cursor / vscode / copilot / kiro / pi / antigravity / codex / opencode / aider / claw / droid / trae / trae-cn / hermes). Skills install under the project root with relative-path registration in./.claude/CLAUDE.md.extractexits non-zero with a backend-named error when all semantic chunks fail (no more silent AST-only output).Security hardening
check_graph_file_size_cap(512 MiB) on every existing graph-file read site:graphify-html(callflow),graphify-export,graphify-global,graphify-prs,graphify-watch,graphify-serve,graphify-benchmark, and the CLI's sharedload_graph.sanitize_metadata— recursive HTML-escape + control-char strip + length/list caps.9.1.6with SRIintegrity+crossorigin.Detection & dedup
shlex+ a realenv(1)option parser (POSIX/BSD + GNU coreutils-S/-vS/--split-stringpacked payloads, inline assignments, quoted paths)..ets(ArkTS) added;.tsxordered before.js._norm/_norm_labelswitched to NFKC + Unicode-aware\Wso CJK / non-ASCII labels are no longer silently zeroed.caseless(Straße→strasse), replacingstr::to_lowercase.Bookkeeping
0.8.17(matchesgraphify-pytag).Cargo.lockis now checked in (the workspace ships a binary; reproducible builds need it).Summary by CodeRabbit
New Features
Improvements
Documentation