Correctness, performance, and documentation overhaul

[networmix]

What this is

A multi-round review of the analysis engine, model layer, DSL, and documentation. Every finding was verified with a runtime probe before being applied, and the higher-risk fixes were re-checked by independent reviewers instructed to refute them — which caught several fixes that were initially incomplete (see How this was verified).

The [Unreleased] section of CHANGELOG.md is the authoritative summary. This description highlights what matters for review.

Correctness fixes

Each is probe-confirmed and covered by a regression test.

Fix	Symptom before
Log-domain Efraimidis-Spirakis keys for weighted failure sampling	u**(1/w) underflows for small weights. With per-hour failure rates (~1e-5), one entity was chosen 97% of the time, ordered by entity id rather than weight — silently inverting the intended bias.
Disjoint FlowIndex id ranges + rejection of duplicate policy-based demand triples	A cached demand and a FlowPolicy-based demand sharing (source, destination, priority) produced colliding flow ids whose flows merged. A plain-YAML scenario reported 15 units placed across a 10-unit min cut.
Injective composed demand ids + pseudo-endpoint collision check	Composed ids concatenate demand ids and group labels, both of which may contain |, so distinct demands could share a pseudo endpoint and route through a zero-cost bypass.
Scope-typed apply_failures_typed	A risk group sharing a name with a node excluded the node and left the group's members up.
Transitive member seeding for expand_groups	Shared-risk expansion differed depending on whether the failure came from an entity rule or a risk_group rule, and never reached members of nested groups.
Deterministic per-pair link ids	uuid-suffixed ids sorted differently on each rebuild, so a fixed seed selected different physical links across identical scenario builds.
Boundary guards	Capacities at/above the internal pseudo-edge capacity, accumulated path costs overflowing the core's int64 arithmetic, bound-context/argument mismatches, scalar in/not_in, and unhashable group_by all produced wrong numbers silently; they now raise.

Performance

• Weighted-choice selection precomputes per-rule weight splits and selects via a heap — ~4x faster on 100k-candidate pools (60ms → 15.7ms per iteration).
• MaximumSupportedDemand probes share one SPF DAG cache: SPF runs drop from probes x sources to sources.
• Membership rules flatten entity attributes once rather than once per rule.

Structure

• Selector evaluation moved to ngraph.model.selectors; ngraph.dsl.selectors keeps YAML-facing parsing and re-exports the moved names, so the model layer no longer depends on the DSL package at import or runtime.
• FailureManager pre-builds per-run inputs via a prepare_inputs hook on analysis functions, replacing kwarg-name sniffing inside the engine. Custom analysis functions opt in by setting the attribute.
• Shared helpers replace repeated logic; dead and production-orphaned code removed; the package layering policy is now written down in the design reference.

Breaking changes

• MaxFlowResult.min_cut returns a true minimum cut (capacity equals max flow) rather than all saturated edges. Saturated-edge analysis remains available via sensitivity().
• from_networkx(bidirectional=...) defaults to None and is inferred from graph type; undirected inputs now produce antiparallel arc pairs. Pass bidirectional=False for the old behavior.
• Failure-policy expand_children removed — cascading to children is inherent and always applied. Scenarios using the key must drop it.
• Inline-object flow_policy removed from the scenario schema; use a preset name string.

Documentation

Every curated document was verified by execution, not by reading: all Python examples, 46 DSL YAML blocks, the bundled scenarios, and 22 CLI invocations run against this branch, and documented outputs match actual output.

The design reference's algorithm sections were checked line-by-line against the NetGraph-Core C++ sources. That corrected unsupported complexity bounds and a mischaracterization of reverse residual arcs (they serve min-cut reachability, not cross-tier flow cancellation). A final prose pass tightened docstrings and docs, followed by independent verification that restored facts the tightening had dropped.

How this was verified

Findings came from parallel reviewers, then went through adversarial confirmation before being applied — reviewers were told to refute each fix. That step earned its keep: it caught that the expand_groups fix missed depth-3 hierarchies, that deterministic link ids collide when node names contain \|, that composed demand ids could still collide across demands, and that a cost bound belonged at 2^62 rather than 2^63. All four were repaired before this branch was cut.

Gates on this branch: make check-ci green (1186 tests, coverage 91.65%), make validate green, make docs regenerated.

Reviewing this

It is 149 files, so a suggested order:

1. CHANGELOG.md — the full picture in one place.
2. ngraph/model/failure/policy.py, ngraph/analysis/placement.py, ngraph/analysis/demand.py — the three highest-impact correctness fixes.
3. ngraph/model/selectors/ (new package) and ngraph/analysis/failure_manager.py — the structural changes.
4. docs/reference/design.md — the C++-verified algorithm descriptions.

docs/reference/api-full.md is generated by make docs; review the docstrings rather than that file.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 9ead3875f6

ℹ️ 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".

[chatgpt-codex-connector]

Refresh flattened attributes after membership mutations

When multiple membership rules use the same scope, this cache preserves the attributes from before the first rule mutated the entities. For example, if group A adds itself to nodes and a later group B matches risk_groups contains A, B sees the cached pre-A risk_groups values and silently matches nothing; risk-group-scoped rules similarly cannot observe children appended by earlier rules. Invalidate or update the relevant flattened map after each mutation rather than reusing it for the entire resolution pass.

Useful? React with 👍 / 👎.

[chatgpt-codex-connector]

Resolve registered descendants when expanding risk groups

When YAML defines mid and leaf as top-level groups and references mid by name as a child of top, the parser creates a child stub object rather than linking to network.risk_groups["mid"]. Recursing directly into that stub therefore never sees mid's registered leaf child, so failing top excludes direct mid members but silently leaves grandchild members and links active. Resolve a child through the registered group with the same name when available, while retaining inline-only descendants.

Useful? React with 👍 / 👎.

[chatgpt-codex-connector]

Dispatch selectors through explicitly supplied unbound contexts

When a caller supplies the newly documented unbound context form, such as context=analyze(network), this branch accepts it because is_bound is false, but the later ctx.max_flow()/ctx.max_flow_detailed() call omits source and sink and raises Unbound context: source and sink are required. sensitivity_analysis has the same path. Pass the analysis selectors into calls on unbound contexts, or document and reject them as unsupported.

Useful? React with 👍 / 👎.