Skip to content

fix(detect): sort os.walk to ensure reproducible symbol IDs (#3407) - #3454

Open
nikhilsaxena04 wants to merge 1 commit into
Graphify-Labs:v8from
nikhilsaxena04:fix-os-walk-sort
Open

fix(detect): sort os.walk to ensure reproducible symbol IDs (#3407)#3454
nikhilsaxena04 wants to merge 1 commit into
Graphify-Labs:v8from
nikhilsaxena04:fix-os-walk-sort

Conversation

@nikhilsaxena04

Copy link
Copy Markdown

The bug:
Cross-platform extractions were not deterministic (yielding different output graphs between macOS and Linux CI) because the fallback ordering relied on os.walk, which processes directories in raw filesystem order (unordered).

The fix:
Explicitly sorted dirnames and filenames in the os.walk loop in graphify/detect.py to guarantee that discovery is 100% reproducible regardless of the underlying OS or filesystem.

Tests:
tests/test_detect_sort.py added to assert on full relative paths to rigorously prove that os.walk is traversing the directories in sorted order. This prevents regression of non-deterministic traversal order.

…-Labs#3407)

The bug:
Cross-platform extractions were not deterministic (yielding different output graphs between macOS and Linux CI) because the fallback ordering relied on `os.walk`, which processes directories in raw filesystem order (unordered).

The fix:
Explicitly sorted `dirnames` and `filenames` in the `os.walk` loop in `graphify/detect.py` to guarantee that discovery is 100% reproducible regardless of the underlying OS or filesystem.

Tests:
`tests/test_detect_sort.py` added to assert on full relative paths to rigorously prove that os.walk is traversing the directories in sorted order. This prevents regression of non-deterministic traversal order.

@graphify-labs graphify-labs Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Graphify reviewed this change.

Looks safe to merge — no coupling regressions and no blocking issues, checked against the code graph (not a self-assessment).

Formal verification. No changes could be formally verified in this run.


Graphify review — findings

Sorts dirnames and filenames in place during the os.walk traversal in detect, so scanned files are discovered in deterministic alphabetical order rather than filesystem-dependent order. Adds a test asserting detected code files come back in sorted repo-relative order.

No blocking issues surfaced.

Analysis details — impact, health, verification

Impact & health

Graphify review

Impact — 2164 functions depend on the 121 functions this change touches.

Health — this change adds coupling hotspots:

  • new: extract() — 563 callers, 43 callees
  • new: _rebuild_code() — 115 callers, 51 callees
  • new: detect() — 110 callers, 15 callees
  • new: _extract_generic() — 18 callers, 26 callees
  • new: save_manifest() — 40 callers, 11 callees
  • new: extract_files_direct() — 17 callers, 20 callees
  • new: extract_js() — 85 callers, 4 callees
  • new: extract_xaml() — 19 callers, 17 callees
  • …and 43 more — each is listed as a finding

Verification — 2164 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: 762 function(s) in the blast radius were not formally verified this run

Test selection

Test selection

51 of 263 test file(s) selected (19%) via static blast radius.

  • tests/test_astro_extraction.py — impact
  • tests/test_atomic_writes.py — impact
  • tests/test_build.py — impact
  • tests/test_charmap_encoding.py — impact
  • tests/test_chunking.py — impact
  • tests/test_cjs_module_extension.py — impact
  • tests/test_claude_cli_backend.py — impact
  • tests/test_cpp_nested_and_cli.py — impact
  • tests/test_detect.py — impact
  • tests/test_detect_sort.py — impact, changed-test
  • tests/test_dotnet.py — impact
  • tests/test_evidence_binding.py — impact
  • tests/test_extract.py — impact
  • tests/test_extract_cli.py — impact
  • tests/test_file_slice.py — impact
  • tests/test_ignore_file_encoding.py — impact
  • tests/test_image_vision.py — impact
  • tests/test_import_extension_resolution.py — impact
  • tests/test_incremental_mtime_collision.py — impact
  • tests/test_indirect_dispatch.py — impact
  • tests/test_indirect_dispatch_assign_return.py — impact
  • tests/test_indirect_dispatch_getattr.py — impact
  • tests/test_js_exported_scalar_bindings.py — impact
  • tests/test_languages.py — impact
  • tests/test_llm_backends.py — impact
  • tests/test_long_path_hashing.py — impact
  • tests/test_manifest_ingest.py — impact
  • tests/test_mcp_ingest.py — impact
  • tests/test_multilang.py — impact
  • tests/test_non_regular_files.py — impact
  • tests/test_office_incremental.py — impact
  • tests/test_office_limits.py — impact
  • tests/test_ollama.py — impact
  • tests/test_oversized_document_slicing.py — impact
  • tests/test_package_json_subpath_imports.py — impact
  • tests/test_pdf_slicing.py — impact
  • tests/test_pdf_token_estimate.py — impact
  • tests/test_phantom_external_import.py — impact
  • tests/test_pipeline.py — impact
  • tests/test_python_underscore_resolution.py — impact
  • tests/test_rationale.py — impact
  • tests/test_ruby_resolution.py — impact
  • tests/test_scala_self_type.py — impact
  • tests/test_stale_prune.py — impact
  • tests/test_swift_computed_properties.py — impact
  • tests/test_ts_new_expression_calls.py — impact
  • tests/test_typescript_module_extensions.py — impact
  • tests/test_unmapped_at_alias_resolution.py — impact
  • tests/test_vue_extraction.py — impact
  • tests/test_watch.py — impact
  • … and 1 more

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 detect.

The verifier did not have enough to check detect, so it is saying so rather than guessing. No false assurance is the whole point.

Guarantee: No guarantee either way, this is an honest abstention, not a pass.

Note: Reason: parameter `root` is annotated `Path` — outside the synthesizable primitive/collection set

· 51 more finding(s) on lines outside this diff (see the check run).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant