Skip to content

routing: remove largest-SCC filtering as a hard default#17

Merged
blackopsrepl merged 1 commit intomainfrom
issue/4-remove-hard-default-scc-filter
Mar 21, 2026
Merged

routing: remove largest-SCC filtering as a hard default#17
blackopsrepl merged 1 commit intomainfrom
issue/4-remove-hard-default-scc-filter

Conversation

@blackopsrepl
Copy link
Copy Markdown
Contributor

Summary

  • add a ConnectivityPolicy to NetworkConfig and default it to preserving all fetched graph components
  • apply the configured connectivity policy consistently for fresh fetches and cached loads
  • keep the existing largest-SCC reduction available as an explicit opt-in and cover the behavior with hermetic tests

Validation

  • cargo fmt --all -- --check
  • cargo test connectivity_policy_keeps_all_components --test integration
  • cargo test largest_scc_filter_is_opt_in --test integration
  • cargo clippy --workspace --all-targets -- -D warnings
  • cargo test

Fixes #4

@blackopsrepl blackopsrepl merged commit e2d7b89 into main Mar 21, 2026
3 checks passed
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: fafc71e831

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

let _ = tx.send(RoutingProgress::CheckingCache { percent: 8 }).await;
}
match Self::load_from_file(&cache_path).await {
match Self::load_from_file(&cache_path, config).await {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Key the in-memory cache by connectivity policy

This only makes the file-cache load path policy-aware. RoadNetwork::load_or_fetch still returns an in-memory hit before reaching this call, and that cache is keyed only by bbox.cache_key(). If one process loads the same bbox first with ConnectivityPolicy::KeepAll and later with LargestStronglyConnectedComponent (or vice versa), the second call silently reuses the first graph, so routes and matrices are computed with the wrong connectivity semantics.

Useful? React with 👍 / 👎.

"Road network has {} SCCs, filtering to largest component",
scc_count
);
network.filter_to_largest_scc();
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Avoid persisting SCC-filtered graphs as canonical cache files

Filtering the freshly fetched graph here mutates it before load_or_fetch writes it to the shared <bbox>.json cache file. In the scenario where a bbox is first populated with ConnectivityPolicy::LargestStronglyConnectedComponent, every later default KeepAll load—even in a fresh process—can only reload the already-pruned graph, so the new default no longer preserves disconnected components for that region.

Useful? React with 👍 / 👎.

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.

routing: remove largest-SCC filtering as a hard default

1 participant