ADR-044: Reachability-Aware Suppression and the Effective Public ABI¶

Date: 2026-07-17 Status: Accepted — P0 slice implemented: pipeline-order correctness fix, Suppression.reachability/allow_public_break, entity/cause namespace split, suppression_would_hide_public_break diagnostic. P1 (first-class detection) is now also implemented — see "P1 slice" below: L5 call-graph evidence wired into MarkReachability/DetectInternalLeaks, the new internal_symbol_required_by_public_api overlay ChangeKind, a third reachability_kind value (symbol_availability), structured JSON/SARIF reachability fields, PolicyFile.internal_namespaces, and the two remaining checker.py suppression call sites routed through the diagnostic-emitting helper. P2 is now also implemented — see "P2 — empirical validation" below: the consumer_required_symbol_removed ChangeKind promoting --used-by's missing-symbol check to a first-class suppressible finding, the opt-in --verify-runtime old-consumer/new-library execution probe (consumer_runtime_load_failed, RISK-tier), and worked examples (case192/case193) exercising the headline scenario and its deliberate counter-example end to end. Verified: main@2e43d53 on 2026-08-04 Decision maker: Nikolay Petrov (@napetrov)

Context¶

A field review of an oneDAL integration (PR 3693) found that a blanket namespace suppression —

suppressions:
  - namespace: "oneapi::dal::**::detail::**"
    reason: "Private implementation details"

— silently hid a genuine ABI break: a public inline function (oneapi::dal::train()) called through to an exported detail:: specialization that the new library removed. Old applications compiled against the public header fail to load against the new library — a real func_removed break — but abicheck's report showed nothing, because the suppression matched the internal symbol before the tool had a chance to notice a public entry point depended on it.

The review's conclusion, and the premise of this ADR, is that this is not primarily a oneDAL configuration mistake — a project cannot reasonably be expected to hand-enumerate every internal symbol a public inline/template function happens to reach — but a tool correctness gap: abicheck already has the pieces to tell "truly unreachable internal churn" apart from "internal implementation detail that is part of the effective public ABI because public code depends on it," and does not consult them before suppression runs.

The pipeline-order bug, confirmed against the current code¶

abicheck/post_processing.py's DEFAULT_PIPELINE (the sequence compare() actually runs) has this shape today:

...
FilterNonPublicSurface()
DemoteOffPythonSurface()
ApplySuppression()            # ← suppression removes raw evidence here
SuppressRenamedPairs()
FilterRedundant()
EnrichAffectedSymbols()
AttributeStdlibEmbedding()
DetectInternalLeaks()         # ← too late: the removed symbol's changes
                               #   were already filtered out by ApplySuppression
DemoteUnreachableInternalChurn()
...

DetectInternalLeaks (internal_leak.detect_internal_leaks) works by scanning the surviving changes list for layout/identity-affecting kinds (_LEAK_TRIGGERING_KINDS) whose root type is internal, then walking the public surface to see if that type is reachable. If ApplySuppression already removed the triggering func_removed/type_*_changed entries for detail::train_ops_dispatcher<...> because they matched oneapi::dal::**::detail::**, DetectInternalLeaks never sees them — there is no evidence left to correlate with the public-reachability walk, so INTERNAL_TYPE_LEAKS_VIA_PUBLIC_API never fires, and DemoteUnreachableInternalChurn's "confirmed leak" check has nothing to confirm. The class of change the review calls out — internal_symbol_required_by_public_api, a symbol-availability leak via DECL_CALLS_DECL, not a layout leak — is not even in _LEAK_TRIGGERING_KINDS at all (see "What this ADR does not fix" below); but the ordering bug independently defeats every leak kind the pipeline does implement today, for any change matched by a broad rule that runs before the detector sees it.

ApplySuppression.run() (and every step downstream of it that adds new findings — DetectCppPatterns, DetectNamespacePatterns, DetectTemplatePatterns, DetectInternalLeaks itself) hand-applies ctx.suppression.is_suppressed(c) to its own new findings — a # Synthetic leak findings must respect user suppression rules too comment in DetectInternalLeaks.run() — but that only stops a synthetic leak finding from surviving suppression; it does nothing to restore the raw evidence ApplySuppression already deleted upstream, so the leak was never computed in the first place.

The second gap: entity namespace vs. cause namespace¶

suppression.py's namespace selector (_matches_namespace) matches a change if any of change.symbol, change.caused_by_type, or change.qualified_name falls in the namespace:

def _matches_namespace(compiled, change):
    return (
        _ns_match(compiled, change.symbol)
        or _ns_match(compiled, change.caused_by_type)
        or _ns_match(compiled, change.qualified_name)
    )

caused_by_type is set in two situations that must not be treated alike:

Redundancy linking (diff_filtering._mark_as_redundant): a derived change on a type is linked to its own root cause, both of which are the same entity — safe to match on either.
Cross-entity attribution (internal_leak._build_leak_change, crosscheck.py's leak/dependency findings): a public symbol's finding carries caused_by_type pointing at the internal type responsible. func_params_changed/var_type_changed on a public function/variable whose signature changed because an internal root type changed (diff_filtering._mark_as_redundant, called from _filter_redundant for _DERIVED_CHANGE_KINDS) is exactly this shape: symbol is public, caused_by_type is the internal root.

A rule like namespace: "oneapi::dal::**::detail::**" matches case 2 via caused_by_type alone — suppressing a finding whose symbol is the public, breaking entity, purely because its documented cause happens to live in an internal namespace. The suppression author who wrote the rule almost certainly meant "hide churn inside detail," not "hide any public finding whose explanation happens to mention detail" — the ADR's oneDAL example (kmeans::descriptor vs. kmeans::detail::descriptor_base) is this exact failure mode.

The one rule that does not change¶

Same authority boundary this codebase has used since ADR-024 §D4/D5 and restated in ADR-041: suppression must never manufacture confidence it does not have. A suppression rule may remove noise; it may never be the reason a real, public-reachable break goes unreported. Every mechanism this ADR adds is a safety default on top of existing opt-in suppression syntax — no existing narrowly-targeted suppression rule (symbol, symbol_pattern naming one entity, type_pattern) changes behavior. Only the two broad selectors (namespace, source_location) — the ones that can match an internal symbol a suppression author never explicitly reasoned about — get a new default, and it is an opt-out default (reachability: any restores the old behavior for a rule the user has audited).

Decision — P0 slice (this change)¶

D1. Compute reachability before suppression runs, not after¶

New PipelineStep, MarkReachability, inserted into DEFAULT_PIPELINE before ApplySuppression:

FilterReservedFieldRenames … EnrichSourceLocations
FilterNonPublicSurface()
DemoteOffPythonSurface()
MarkReachability()            # ← new: tags every change, before suppression sees it
ApplySuppression()            # ← now reachability-aware (D2)
SuppressRenamedPairs()
FilterRedundant()
...
DetectInternalLeaks()         # unchanged position: the underlying evidence it
                               # needs is no longer gone by the time it runs,
                               # because MarkReachability ran first and a
                               # public-reachable change survives ApplySuppression
DemoteUnreachableInternalChurn()
...

This is deliberately not the full literal reordering the review sketches (moving FilterRedundant/DetectInternalLeaks themselves ahead of suppression). That reordering would break the invariant ADR-013 and ADR-004 established — "suppression runs before redundancy filtering, so a suppressed change never contributes to the verdict whether root or derived" — and would require every downstream step that already hand-applies ctx.suppression.is_suppressed() to its own new findings (DetectCppPatterns, DetectNamespacePatterns, DetectTemplatePatterns, DetectInternalLeaks) to be re-audited for double-suppression or under-suppression. The actual bug is narrower than "suppression runs at the wrong pipeline position": it is "suppression has no reachability signal to consult." Giving it that signal — computed once, up front, independent of whatever else the pipeline does to the change list — fixes the reported failure with a much smaller blast radius, and is the literal mechanism the review's own "Recommended implementation" section describes (public_reachable: bool metadata attached to each change before matching).

MarkReachability.run():

Calls internal_leak.compute_leak_paths(ctx.old) and compute_leak_paths(ctx.new) once — this is a pure function of the snapshot (function/variable/type declarations), not of the change list, so it is safe to compute before any filtering has happened and does not duplicate DetectInternalLeaks's own later call (that call still needs to run after redundancy filtering to decide which triggering changes turn into a synthetic leak finding; this one only needs the raw reachable-type → path map).
For each change, resolves its root type (internal_leak._root_type_name_for_change, the same helper DetectInternalLeaks/DemoteUnreachableInternalChurn already use) and looks it up in the merged old/new path map. When found, sets:
Change.public_reachable = True
Change.reachability_kind — "value_embedding" when internal_leak._path_is_value_propagating holds for at least one matched path, else "pointer_or_signature" — mirroring the embedded_by_value severity-hint distinction _build_leak_change already renders in prose, now available as structured metadata.
Change.reachability_proof_path — internal_leak._format_path of the shortest matched path, e.g. "fn:oneapi::dal::train → base:oneapi::dal::detail::train_dispatch → oneapi::dal::kmeans::detail::train_ops_dispatcher<...>". A change whose root type is not internal, or is internal but unreachable from the public surface in either snapshot, keeps public_reachable=False and the two fields None — the common case, so this is a purely additive per-change annotation with no effect on a project with no internal-leak surface at all.

Three new fields on Change (checker_types.py), all defaulting to False/None — same additive convention as frozen_namespace_violation and surface_exclusion_reason, no schema/serialization version bump needed since JSON/SARIF/JUnit reporters already round-trip Change via dataclasses.asdict-style field enumeration.

Post-merge review rounds (Codex + CI), same change:

Perf regression. The first-shipped MarkReachability ran compute_leak_paths unconditionally on every compare() call — CI's benchmark_scaling.py baseline-regression gate caught up to +5075% on type/struct-heavy scenarios, since this duplicated the identical walk DetectInternalLeaks already performs later, on every comparison, even when no suppression file is configured to ever consult the tag. Fixed by skipping the step entirely when ctx.suppression is None (mirroring ApplySuppression's own no-op check) and, within a run, computing the leak-path walk lazily — only the first time a change whose subject is internal-namespaced is actually seen.
Pointer-only layout churn false-flagged. MarkReachability originally marked any internal type reachable via any path (including a pure pointer/reference indirection) as public_reachable. But DetectInternalLeaks deliberately does not treat a pure-layout change reached only through a pointer as a leak (it is not consumer-visible), and DemoteUnreachableInternalChurn would still correctly demote such churn later — so tagging it reachable only refused a broad suppression rule and appended a spurious suppression_would_hide_public_break diagnostic for churn that was always going to be demoted anyway. Fixed by mirroring DetectInternalLeaks's own _IDENTITY_VTABLE_KINDS/_path_has_indirection judgment inside MarkReachability before tagging.
Directly-public subjects are a known, deliberately unclosed gap — attempted, then reverted. The internal-type-leak walk (compute_leak_paths) only ever records internal type names — it has no notion of "this change's own subject is already public." A broad source_location/namespace rule matching a genuinely public function purely by file path (e.g. a public function physically declared under a path a source_location: "*/internal/*" glob matches, with no internal-namespaced name at all) is therefore not protected by the reachability gate. A fix was attempted: broaden MarkReachability so any change whose subject is not internal-namespaced is marked public_reachable = True directly (no leak-path proof needed), with the leak-path walk only consulted for a subject that is internal-namespaced (also needing the internal-namespace check to widen to match what Suppression._ns_match checks at match time — Change.qualified_name and a demangled form of the raw symbol, since a mangled/extern "C" symbol reads as a single opaque segment otherwise). That fix also exposed a real allow_public_break scoping bug — the gate applied to every rule regardless of selector breadth, so an ordinary narrow symbol: waiver of a known removal suddenly needed allow_public_break too, regressing test_suppression.py's basic suppression tests; corrected by scoping the gate to broad selectors only (D2 as written reflects this correction).

The broadening itself was then reverted, one CI run later: it regressed tests/test_libabigail_parity_extended.py::TestSuppressionParity:: test_suppress_by_source_location — a private helper (internal_fn, no namespace-segment hint) declared under src/internal/helper.h, matched and correctly suppressed by source_location: "*/internal/*". Both that case and Codex's public-function example are, structurally, the same shape: an unqualified/non-namespaced Visibility.PUBLIC symbol under a path a source_location glob matches. AbiSnapshot's visibility model marks every exported C/C++ symbol Visibility.PUBLIC regardless of whether the maintainer considers it part of the contract — that gap is the entire reason source_location-based suppression exists, to compensate for C/C++ having no true "this is private" linkage visibility. No signal in the name or the snapshot distinguishes "genuinely public, accidentally path-matched" from "genuinely private, correctly path-matched," so no naming heuristic can close Codex's gap without also breaking the ordinary case. Reverted MarkReachability back to the leak-path-only computation; kept the allow_public_break broad-selector scoping (independently correct) and the pointer-only-layout fix above. Closing this gap for real needs actual dependency evidence — the L5 call-graph / consumer-import work already on the P1/P2 roadmap below — not a heuristic on the symbol's own spelling.

Skip the walk for narrow-only suppression files too, not just no suppression at all (Codex). The ctx.suppression is None skip above only covers the no suppression configured case — but a suppression file containing only narrow rules (symbol/symbol_pattern/type_pattern, the common case: a handful of exact waivers) with the default (or explicit "any") reachability is also provably indifferent to the tag: both _passes_reachability_gate (short-circuits on resolved == "any") and _passes_public_break_gate (short-circuits on not self._is_broad_selector) return without ever reading Change.public_reachable for such a rule. Running the public-surface walk for that file is exactly the same waste the ctx.suppression is None fix targets. Added SuppressionList.needs_reachability_evidence() — true iff at least one rule is broad or has an explicit non-"any" reachability — and gated MarkReachability on it alongside the existing None check.
A third late-detector synthetic-finding gap, this time for genuinely public (not internal-leak) findings (Codex). The two already-fixed cases (internal_leak._build_leak_change, diff_templates._leak_change) cover findings whose subject is an internal type reached via a public entry point. diff_namespaces.py's DetectNamespacePatterns — also running after ApplySuppression — has a different shape: EXPERIMENTAL_REMOVED_WITHOUT_REPLACEMENT/ EXPERIMENTAL_GRADUATED (function path) and STD_REEXPORT_REMOVED build fresh Changes for a subject that is itself public (an experimental::/ re-exported function graduating or vanishing), not merely reachable from one. Untagged, a broad namespace: "lib::experimental::*" rule's default unreachable-only reachability silently suppressed the API-break finding with no diagnostic — the same failure mode this ADR exists to close, one detector later than the two already-fixed cases. Fixed the same way: tagged public_reachable=True/reachability_kind="direct_public_symbol" at construction time in _emit_experimental_change/ _build_std_reexport_change — but only for the function-sourced path. _index_funcs_by_stable_key/detect_std_reexport_removed filter on Visibility.PUBLIC before ever building a Change, so those findings' mere existence already proves the subject is public — the same reliable signal the two already-fixed cases have. The type-sourced path (_index_types_by_stable_key) has no such signal: RecordType carries no visibility field at all (unlike Function/Variable), and that index walks every type in snap.types regardless of whether it is genuinely public or an internal type that merely happens to have an "experimental"-segment name — tagging it too would reintroduce exactly the unreliable-heuristic problem that got the broader MarkReachability broadening reverted earlier in this same review cycle, just via a different code path. _emit_experimental_change/_findings_for gained an explicit subject_is_public parameter so the two call sites (funcs vs. types) state their own reliability instead of the function silently assuming one for both. Unlike a raw pre-existing change (suppressed via ApplySuppression, which can attach suppression_would_hide_public_break), these late-detector findings suppress inline via their own ctx.suppression.is_suppressed(c) call and have no diagnostic path — the same established scope boundary the two already-fixed cases also have; not being silently suppressed is the fix, a diagnostic for this whole class of finding is a separate, pre-existing gap this change does not newly introduce or attempt to close.
A fourth late-detector sweep, this time the whole diff_templates.py module (Codex). Fresh evidence beyond the namespace-detector fix above: DetectTemplatePatterns (also running after ApplySuppression) has the identical gap for CPO_KIND_CHANGED — a public name flipping between function and CPO-variable form. Rather than fix that one kind and wait for a further round to find its siblings, audited every detector detect_template_patterns calls: CPO_KIND_CHANGED, OVERLOAD_SET_REROUTED, and UNSPECIFIED_RETURN_NOW_NAMED all filter their source snapshot walk to Visibility.PUBLIC before ever building a Change, so all three got the same construction-time public_reachable=True/reachability_kind="direct_public_symbol" tag as the namespace-detector fix. MANDATORY_TEMPLATE_PARAM_ADDED was deliberately left untagged — its arity index merges observations from both public functions and snap.types under one shared stem key with no way to tell which contributed a given finding, the same no-reliable-signal problem the type-sourced namespace-detector path has; tagging it would reintroduce the reverted heuristic bug one level deeper. Also swept detect_missing_instantiations (INSTANTIATION_MISSING_FROM_BINARY, runs via DetectCppPatterns, same after-ApplySuppression position, same Visibility.PUBLIC-filtered construction) even though Codex's report didn't name it, since it is the same reliable-signal shape found while already auditing the module. A broader sweep of diff_cpp_patterns.py's remaining detectors (SYCL_OVERLOAD_SET_REMOVED, CPU_DISPATCH_ISA_DROPPED, TAG_TYPE_RENAMED, DEFAULT_TEMPLATE_ARG_CHANGED, INLINE_BODY_REFERENCES_RENAMED_MEMBER, BUNDLE_SONAME_SKEW — several BREAKING) for the same pattern remains open; scoped out of this round to avoid rushing verification of six more detectors across two large files without individually confirming each one's visibility-filtering the way every fix above required.
A fifth late-detector gap, back in diff_namespaces.py itself (Codex). detect_namespace_patterns() also runs detect_inline_namespace_version_bump, which was missed by the third round's sweep of that same module (that round covered _emit_experimental_change/_build_std_reexport_change only). It builds INLINE_NAMESPACE_VERSION_BUMPED from _emit_version_bumps, which reads old_list[0]/new_list[0] out of an index keyed by version-stripped namespace segments — and that index's entries come from _collect_versioned_entries, which merges public-function-sourced (f.name filtered to Visibility.PUBLIC) and type-sourced (t.name, unfiltered — RecordType has no visibility field) observations into one list per key, same shape as MANDATORY_TEMPLATE_PARAM_ADDED's arity index. The difference here: each entry is a (qualified_name, version, kind) tuple that already carries which source it came from, so — unlike the arity index — the signal survives into _emit_version_bumps and just wasn't read. Fixed by checking old_list[0][2] == "function" and new_list[0][2] == "function" (both sides, since old_q/new_q both flow into the emitted Change) before tagging public_reachable=True/ reachability_kind="direct_public_symbol" — a type-sourced bump stays untagged for the same no-visibility-field reason as the arity index.
The diff_cpp_patterns.py sweep the fourth round deliberately deferred (Codex). Fresh evidence named TAG_TYPE_RENAMED specifically: detect_tag_type_renamed builds its Change from a type pairing, but gates the finding on symbol evidence (only_removed/only_added) explicitly scoped to _PUBLIC_VIS per its own docstring — the finding only exists when real public-surface mangled symbols embed the tag's leaf name, the same "finding's mere existence already proves public reachability" signal the earlier leak-finding and namespace/template fixes rely on. Rather than fix only the named kind, finished the sweep the fourth round scoped out: detect_sycl_overload_set_removal (SYCL_OVERLOAD_SET_REMOVED) and detect_cpu_dispatch_isa_dropped (CPU_DISPATCH_ISA_DROPPED) both build their grouped findings exclusively from _PUBLIC_VIS-filtered old_funcs/new_funcs (plus, for the ISA detector, the raw PE/Mach-O export table — public by definition), so both got the same construction-time tag. detect_default_template_arg_changed (DEFAULT_TEMPLATE_ARG_CHANGED) is the same shape (old_funcs/new_funcs scoped to _PUBLIC_VIS), also tagged. detect_inline_body_renamed_member (INLINE_BODY_REFERENCES_RENAMED_MEMBER) was audited and deliberately left untagged: its _find_public_pimpl_holders helper infers "public" from not is_internal_type(name) — a naming/namespace heuristic, not a Visibility.PUBLIC filter — the exact shape of the heuristic that was tried and reverted earlier in this cycle (see the D1 "directly-public subjects" entry above); tagging it here would reintroduce that reverted bug through a different detector. detect_bundle_soname_skew (BUNDLE_SONAME_SKEW) turned out to be a false alarm on the original P2 list: it is invoked from bundle.py's separate compare-release/bundle-cohort command, never from DetectCppPatterns or any path that runs through MarkReachability/ ApplySuppression at all, so the pipeline-order bug this ADR closes does not apply to it.
Self-review: entity_namespace missing from the D4 diagnostic's own selector display. With CI green and Codex quiet, an independent self-review pass (prompted by "is this a full implementation, what's left") re-read the diff cold rather than re-trusting the prior rounds' conclusions, and found _build_suppression_overreach_change (post_processing.py) still fell back through rule.namespace or rule.cause_namespace or rule.source_location or rule.symbol or rule.symbol_pattern or rule.type_pattern or "?" — entity_namespace, the canonical spelling introduced by D3's namespace/cause split, was never added to this chain, even though the equivalent string-building in SuppressionAudit (suppression.py) already includes it. A rule written with entity_namespace: (not the legacy namespace: alias) that triggers suppression_would_hide_public_break would render as "?" (or whichever unrelated field happened to be set) in the diagnostic instead of naming the actual rule — undermining D4's whole stated purpose of "explaining why and how to override it." No test caught this: the existing regression test used the namespace alias and asserted only that "allow_public_break" appeared in the message, never the selector text itself. Also noticed while fixing it: rule.symbol/ rule.symbol_pattern/rule.type_pattern in that same fallback chain are unreachable dead code — would_withhold() requires not self._passes_public_break_gate(change), and that gate returns True unconditionally whenever _is_broad_selector is False, which is exactly the case whenever any of those three (primary narrow selectors) is set — so a rule naming one can never reach this diagnostic at all. Fixed by adding rule.entity_namespace to the chain and dropping the three dead branches (only the four broad-shaped fields — namespace, entity_namespace, cause_namespace, source_location — can ever actually appear here), plus a new regression test using entity_namespace only and asserting the rendered selector text, not just a substring of the fixed suffix.
DEFAULT_INTERNAL_NAMESPACES is a hard-coded convention list; a project using a different one is invisible to MarkReachability (Codex, P2). MarkReachability called compute_leak_paths(ctx.old/new, DEFAULT_INTERNAL_NAMESPACES) with the walk's own hard-coded default (detail/impl/internal/__detail/_impl) with no way to override it. A project whose internal-implementation convention uses a different segment — Codex's example: ns::priv::* — is never recognized as "internal" by the walk at all, so a change on a type in that namespace never gets public_reachable tagged, regardless of whether it is genuinely reachable from a public type. A broad namespace: "ns::priv::*" suppression rule (default reachability="unreachable-only") then suppresses the change with no diagnostic — exactly the failure mode this ADR exists to close, just for any internal-namespace convention outside the default five tokens. Verified this is not a heuristic gap like the reverted D1 "directly-public subjects" fix above — sibling pipeline steps DetectInternalLeaks and DemoteUnreachableInternalChurn (both pre-dating this ADR) already accept a namespaces: tuple[str, ...] | None constructor override for exactly this reason; MarkReachability was simply the odd one out, hard-coding the default with no override hook at all. Fixed by giving MarkReachability the identical constructor parameter, so it is at least structurally consistent with its siblings. This does not fully close the gap: DEFAULT_PIPELINE still constructs all three steps with no arguments (confirmed — no caller anywhere threads a non-default value today), so every project is still limited to the same five-token default until a real configuration surface exists. Deliberately did not attempt to auto-derive "the" internal segment from a suppression rule's own namespace glob (e.g. extracting literal segments from "ns::priv::*") — a pattern's leading segments are often shared with unrelated public types (e.g. "oneapi::dal::**::priv::**" — "oneapi"/"dal" are not internal markers), so blindly harvesting them would misclassify public types as internal project-wide, the same unreliable-heuristic failure mode as the reverted D1 fix, just reached from the opposite direction. Closing this for real needs a genuine project-level configuration surface (e.g. a PolicyFile.internal_namespaces: key) threaded consistently through MarkReachability/DetectInternalLeaks/DemoteUnreachableInternalChurn/ DetectNamespacePatterns — added to the P1 roadmap below as a concrete, scoped follow-up rather than attempted reactively in this round.
A sixth late-detector gap, this time entirely outside post_processing.py (Codex). Fresh evidence: pattern_verdicts. apply_pattern_verdicts() — invoked from checker._apply_pattern_verdicts_step, well after post_processing.DEFAULT_PIPELINE (and thus MarkReachability/ ApplySuppression) has already run — appends new OPAQUE_INVARIANT_BROKEN/ HANDLE_TYPE_CHANGED Changes that checker._filter_pattern_synthetic then runs through its own suppression.is_suppressed(c) call, the same "late synthetic finding, no diagnostic path" shape as the diff_namespaces.py/ diff_templates.py/diff_cpp_patterns.py sweeps above, just reached from a completely different module (--pattern-verdicts, ADR-027, not part of the DEFAULT_PIPELINE steps this ADR had audited). Audited both kinds: OPAQUE_INVARIANT_BROKEN's subject type is only ever tagged OPAQUE_POINTER in old_idioms (a precondition for this finding) when idioms. _recognise_opaque/_public_pointer_only found a genuine Visibility.PUBLIC function referencing it — the same reliable signal the other Visibility.PUBLIC-filtered late-detector findings have — so tagged public_reachable=True/reachability_kind="direct_public_symbol" at construction. HANDLE_TYPE_CHANGED's subject is a typedef alias: AbiSnapshot.typedefs is a plain dict[str, str] with no visibility field at all (typedefs, unlike Function/Variable, carry none), so _recognise_handle walking every declared typedef gives no reliable public/private signal for the alias itself — deliberately left untagged, same reasoning as MANDATORY_TEMPLATE_PARAM_ADDED. Added regression assertions for both (including the deliberately-untagged case) to test_pattern_verdicts.py. A wider audit of whether any other ADR-027/pattern-verdict-adjacent modules construct late synthetic findings the same way remains open — this round only confirmed the two kinds Codex's fresh evidence named.
RecordType.origin was a real, overlooked signal — closes three "deliberately untagged" cases from earlier rounds (Codex). Every prior round asserted "RecordType carries no visibility field, so a type-sourced finding has no reliable public/internal signal" — MANDATORY_TEMPLATE_PARAM_ADDED, the type-sourced path of _emit_experimental_change/EXPERIMENTAL_GRADUATED/ EXPERIMENTAL_REMOVED_WITHOUT_REPLACEMENT, and the type-sourced path of _emit_version_bumps/INLINE_NAMESPACE_VERSION_BUMPED were all left untagged on that basis. That premise was incomplete: RecordType does carry origin: ScopeOrigin (ADR-024 D1's Linkage × Origin surface model), and ScopeOrigin.PUBLIC_HEADER — set only under ADR-024's opt-in --public-header/--public-header-dir scoping — is exactly the reliable signal these three sites were missing. Without that flag every type's origin is ScopeOrigin.UNKNOWN (per ScopeOrigin's own docstring), so this degrades to the prior untagged behavior automatically for the common (no public-header set) case — not a regression, purely additive. Fixed all three:
diff_namespaces._emit_experimental_change/_findings_for: replaced the static subject_is_public: bool parameter with old_origins/ new_origins maps (None for the always-public function path, {qualified_name: ScopeOrigin} for the type path), looked up per finding against the specific old_q/new_q subject.
diff_namespaces._emit_version_bumps/_collect_versioned_entries: the per-entry "function"|"type" string became a plain is_public: bool (True for a Visibility.PUBLIC function, origin == ScopeOrigin.PUBLIC_HEADER for a type) — no other caller read the old string value.
diff_templates.detect_mandatory_template_param_added/_arities: now returns a second {stem: bool} map alongside the arity-set map, True when any contributing observation for that stem (function or type) was reliably public. Deliberately "any observation" rather than "the specific min-arity-driving one" — a stem with genuine public evidence should not be treated as safe-to-hide by a broad suppression rule even if a sibling internal instantiation also happened to share the stem name; this stays conservative in the direction this ADR cares about (never hides a real public break), unlike the reverted D1 heuristic which risked the opposite (falsely claiming public reachability with zero real evidence). Added _rec_public()/ScopeOrigin.PUBLIC_HEADER regression tests alongside each existing ScopeOrigin.UNKNOWN-default case in test_diff_namespaces.py/test_diff_templates.py. Left INLINE_BODY_REFERENCES_RENAMED_MEMBER (diff_cpp_patterns.py) untouched — its untagged reasoning is a different shape (a namespace heuristic risking false public claims, not a missing origin signal), not something ScopeOrigin fixes.
Self-review follow-up on the RecordType.origin fix above (two minor findings). _emit_experimental_change/_findings_for's new old_origins/new_origins parameters were typed dict[str, object] | None — loose enough to accept any value type and lose the point of adding a typed lookup in the first place; narrowed to dict[str, ScopeOrigin] | None. Separately, detect_experimental_namespace_changes built those maps with a plain {t.name: t.origin for t in old.types} comprehension, which silently lets a later RecordType sharing an exact qualified name overwrite an earlier one's origin — inconsistent with pattern_verdicts._exact_record's established "first match wins" exact-identity convention elsewhere in the reachability code. Replaced with a new _origin_by_name() helper using dict.setdefault for first-occurrence-wins semantics, with a docstring citing the _exact_record precedent. Two duplicate-named RecordTypes in one snapshot is unusual input either way; this is a consistency fix, not a response to an observed bug. Added TestOriginByName regression tests (test_simple_lookup, test_duplicate_name_first_occurrence_wins) to test_diff_namespaces.py.
_emit_version_bumps required BOTH sides public-header-tagged, silently hiding an asymmetric old-consumer break (Codex, fresh evidence). subject_is_public = old_list[0][2] and new_list[0][2] meant a type version bump (ns::__1::queue → ns::__2::queue) stayed untagged whenever only one side carried ScopeOrigin.PUBLIC_HEADER evidence — e.g. the type moved out of the scoped public-header set, or --public-header scoping only covered one snapshot. But the old side alone already proves the break: an application linked against the old public symbol breaks regardless of whether the new symbol also has public-header evidence — the same "old-side-only" reasoning EXPERIMENTAL_REMOVED_WITHOUT_REPLACEMENT already uses (checks only old_origins, D1 above). Changed and to or — either side's public-header evidence is now sufficient, matching MANDATORY_TEMPLATE_PARAM_ADDED's "any observation" conservatism (stay tagged reachable when any reliable evidence exists, never require all of it). Added test_old_side_public_alone_is_reachable/ test_new_side_public_alone_is_reachable to test_diff_namespaces.py.
Late detectors dropped the withheld-rule diagnostic even after their findings were correctly kept (Codex, fresh evidence). DetectCppPatterns, DetectTemplatePatterns, and DetectNamespacePatterns each build fresh Change objects after ApplySuppression already ran, so they filter their own findings through suppression by hand — but did so via the plain SuppressionList.is_suppressed() boolean, which silently discards the "matched but withheld by the reachability gate" information SuppressionList.evaluate() reports. The finding stayed correctly kept (not suppressed — that part of the D1/D3 fixes above was never wrong), but the SUPPRESSION_WOULD_HIDE_PUBLIC_BREAK diagnostic ApplySuppression would have produced for the same rule never appeared, leaving users with no explanation of why their matching rule didn't apply. Audited every post-ApplySuppression step with the same shape and found a fourth, DetectInternalLeaks (not named in the report but the identical bug). Fixed all four via a new shared _merge_findings_respecting_suppression() helper that calls evaluate() and appends the same _build_suppression_overreach_change() diagnostic ApplySuppression itself builds, replacing each detector's own hand-rolled dedup-and-filter loop. Added TestLateDetectorSuppressionDiagnostic (test_diff_templates's CPO case, plus a new DetectInternalLeaks case) and updated the two existing late-detector tests (test_experimental_removed_without_replacement_survives_broad_suppression, test_cpo_kind_changed_survives_broad_suppression) to assert the diagnostic now appears — their prior comments explicitly called this out as a documented, not-yet-closed limitation; this round closes it. Deliberately left open: checker.py's own is_suppressed() call sites (_filter_suppressed_changes, _apply_surface_metrics, _filter_pattern_synthetic — the last being the ADR-027 --pattern-verdicts path D3 above already partially audited) have the same shape but a different call signature (SuppressionList + suppressed: list[Change] directly, not PipelineContext) and were not part of Codex's report; converting them needs its own signature-compatible helper and individual verification, not a blind find-and-replace — tracked as a follow-up, not fixed in this round.
MarkReachability itself never tagged a directly-public-header type's own change (Codex, fresh evidence). internal_leak.compute_leak_paths only ever records internal types found while walking outward from the public surface — a type that IS the public surface (e.g. a header-only type never referenced by an exported function/variable, so nothing walks "into" it from elsewhere) never becomes a key in its result, so a raw change on that type's own layout got no tag at all, even though RecordType.origin == ScopeOrigin.PUBLIC_HEADER (ADR-024's opt-in --public-header scoping) is exactly the reliable signal already consulted for the late-detector findings in diff_namespaces.py/ diff_templates.py. Fixed by building an origin-by-name map (reusing diff_namespaces._origin_by_name) alongside the existing leak-path walk and tagging public_reachable=True/reachability_kind="direct_public_symbol" directly for a change whose root type carries that origin, before falling back to the leak-path check. Without --public-header every origin is ScopeOrigin.UNKNOWN, so this degrades to the prior behavior automatically — purely additive, not a regression. Explicitly not the reverted "any non-internal-namespaced subject" heuristic this class's own docstring warns against: ScopeOrigin.PUBLIC_HEADER is an explicit opt-in tag, not a naming guess. Added test_public_header_type_own_change_is_reachable/ test_non_public_header_type_own_change_stays_untagged to test_reachability_aware_suppression.py.
The public-header direct-tag above only looked at RecordType (Codex, fresh evidence). Function/Variable/EnumType all carry the same ScopeOrigin field — a public-header function/variable/enum's own change had the identical gap the RecordType fix above closes. Extended the direct-tag lookup to all four declaration kinds via a small _public_header_names() helper, plus owner-stripping for ENUM_MEMBER_REMOVED/ENUM_MEMBER_ADDED/ENUM_MEMBER_VALUE_CHANGED/ ENUM_LAST_MEMBER_VALUE_CHANGED — diff_types.py builds these findings' symbol as "EnumName::member", and unlike STRUCT_FIELD_* kinds this isn't stripped by the shared _root_type_name_for_change (deliberately left that shared helper alone rather than changing its existing behavior for the unrelated leak-path check). Added test_public_header_variable_own_change_is_reachable/ test_public_header_enum_member_change_is_reachable to test_reachability_aware_suppression.py.
checker._filter_pattern_synthetic had the exact is_suppressed() vs. evaluate() diagnostic gap the post_processing.py late-detector fix above closed (Codex, fresh evidence) — this is the ADR-027 --pattern-verdicts path (D3 above), a separate module invoked from checker._apply_pattern_verdicts_step well after MarkReachability runs, so its OPAQUE_INVARIANT_BROKEN/HANDLE_TYPE_CHANGED synthetics never got the withheld-rule diagnostic either. Unlike the four post_processing.py detectors, this function's signature doesn't take PipelineContext (a plain SuppressionList + suppressed: list[Change] instead) — the exact reason this call site was left as an open P1 roadmap item in the round above. Fixed anyway since the change was small and self-contained: _filter_pattern_synthetic now calls evaluate() and appends the same _build_suppression_overreach_change() diagnostic (imported from post_processing.py; no import cycle — post_processing does not import checker). Added test_lost_opaqueness_withheld_broad_rule_gets_diagnostic to test_pattern_verdicts.py. Narrows P1 roadmap item 6 to just the remaining two checker.py call sites (_filter_suppressed_changes, _apply_surface_metrics) — neither builds a fresh synthetic finding a suppression rule could plausibly want to match-but-withhold the same way (they filter pre-existing/aggregate findings, not late detector output), so closing them is lower priority than this one was.

D2. `Suppression` gains a reachability guard¶

New Suppression fields:

- namespace: "oneapi::dal::**::detail::**"
  reachability: unreachable-only   # default for namespace / source_location
  reason: "Private implementation details"

reachability: "unreachable-only" | "any" | "public-only".
Default is selector-dependent, not a single global default: a rule is broad (defaults "unreachable-only") when it has a broad, pattern-shaped selector (namespace/entity_namespace/ cause_namespace/source_location) and no primary narrow selector (symbol/symbol_pattern/type_pattern — the mutually-exclusive trio the loader already treats as a rule's main selector). Otherwise it defaults "any" — unchanged behavior.
A primary narrow selector present alongside a broad one exempts the rule from "broad" (post-review correction, Codex): symbol: "ns::detail::T", source_location: "*/internal/*" already names the exact audited entity — the source_location addition can only narrow which changes on that one entity match (selectors combine with AND semantics), never introduce an unaudited match the bare symbol: selector wouldn't already have matched, so it keeps the narrow-selector "unchanged behavior" guarantee rather than suddenly requiring allow_public_break.
member_name is deliberately not a primary selector for this purpose: alone it matches a bare trailing name across any containing type/namespace (per its own docstring, "independent of the containing type"), so namespace: "**::detail::**", member_name: "value_type" still counts as broad — the namespace filter there is doing the real scoping work, not merely narrowing an already-pinned-down match. This is the one case the ADR's first-shipped, coarser "any broad selector present makes the whole rule broad" rule was actually protecting against; narrowing the rule to exempt only the primary trio preserves that protection while fixing the symbol + source_location case.
"unreachable-only": the rule does not match a change with public_reachable=True.
"any": no reachability filtering (today's behavior).
"public-only": inverse — matches only public_reachable=True changes; the review's own "unusual, mainly debugging" case (e.g. temporarily silencing an in-progress leak investigation without touching genuinely private noise).
allow_public_break: bool = False. When a broad rule would suppress a change that is both public_reachable=True and a member of BREAKING_KINDS | API_BREAK_KINDS, the match is refused — the change stays in the report — unless allow_public_break: true is set on that rule. This gate is scoped to broad selectors only, matching reachability's own broad/narrow split (post-review correction — the first-shipped version applied it to every rule regardless of selector shape, which meant an ordinary, deliberate symbol: "_ZN3foo..." waiver of a known, intentional removal would also need allow_public_break: true the moment that symbol happened to read as public-reachable — defeating the basic "suppress one exact symbol I already reasoned about" use case suppression exists for in the first place; caught by test_suppression.py regressing when MarkReachability was broadened per D1's note below). A narrow rule (symbol/symbol_pattern/type_pattern/member_name) is exempt from this gate entirely — naming one exact symbol/type is already the deliberate, audited action, independent of whether that symbol turns out to be public or an internal type that leaks. A rule matching a non-breaking (COMPATIBLE/RISK) public-reachable change is also unaffected regardless of selector shape — this gate exists for exactly the failure mode the review reports (a BREAKING finding silently disappearing behind an unaudited glob), not to relitigate ordinary suppression of a RISK finding or of a symbol the author named exactly.
A match refused by either gate is recorded (D4) rather than silently dropped, so a suppression author sees why their rule did not apply.

D3. Split entity namespace from cause namespace¶

namespace (kept as the primary spelling for backward compatibility) is now an explicit alias for a new canonical field, entity_namespace: it matches only change.symbol / change.qualified_name — not change.caused_by_type.
New cause_namespace field: matches only change.caused_by_type, using the identical glob/ancestor-walk semantics _ns_match already implements.
entity_namespace and cause_namespace may be combined on one rule (conjunctive, like every other selector pair) to express "suppress a finding on this internal entity and caused by this internal namespace" — the genuinely-safe case the old single namespace field conflated with the unsafe one.
Loading both namespace and entity_namespace on the same rule is a load-time error (same "exactly one spelling" discipline symbol/ symbol_pattern/type_pattern already enforce) — they are the same field under two names, not two independent selectors.

This is a behavior change to the pre-existing namespace field's semantics (it no longer matches via caused_by_type), not merely an addition. It is deliberately not shipped behind a compatibility flag: per this repo's conventions (no backwards-compatibility shims for a correctness fix), and because the old behavior is the review's headline false-negative — a namespace rule that happens to over-match through caused_by_type was never a feature anyone could have been relying on for a correct result, by construction. tests/test_frozen_namespace.py had exactly one test asserting the old via-caused_by_type match (test_namespace_suppresses_caused_by_type_match); it is updated by this change to assert the new, safer behavior (test_namespace_does_not_match_caused_by_type) plus a new counterpart test for cause_namespace — the one place in this repo's own test suite that depended on the old semantics is also the one place demonstrating exactly why they were unsafe.

D4. `suppression_would_hide_public_break` diagnostic¶

New ChangeKind.SUPPRESSION_WOULD_HIDE_PUBLIC_BREAK (COMPATIBLE_WITH_RISK — it is not itself an ABI break; it is advisory metadata about a suppression decision). Emitted by ApplySuppression for every (rule, change) pair where a rule matched a change's selectors but the match was refused by D2's reachability or allow_public_break gate:

Namespace suppression "oneapi::dal::**::detail::**" matched
oneapi::dal::kmeans::detail::train_ops_dispatcher<...> but was not applied:
the symbol is public-reachable via oneapi::dal::train() →
oneapi::dal::detail::train_dispatch() →
oneapi::dal::kmeans::detail::train_ops_dispatcher<...>. Add
`allow_public_break: true` to this rule to suppress it anyway.

— the exact report shape the review's "Recommended implementation" section asks for. This rides as an ordinary Change appended to the change list (so it is visible in every existing report format — Markdown/JSON/SARIF/ JUnit — with no per-format plumbing), not a bolted-on side channel; a project that wants CI to fail loudly when this fires can already do so via --severity-risk error (existing severity-gating mechanism, ADR-009), requiring no new CLI surface for this slice.

What the P0 slice did not fix (closed by the P1 slice below)¶

The oneDAL dispatcher case (func_removed on an internal template specialization reached only via DECL_CALLS_DECL from a public inline function — no layout evidence, so internal_leak.py's _LEAK_TRIGGERING_KINDS/BFS-over-RecordType walk structurally cannot see it) was not closed by the P0 slice. MarkReachability reused only internal_leak.compute_leak_paths, which walks type-layout reachability (inheritance, by-value fields, signatures) — it had no access to the L5 semantic call graph (source_graph.py). The P1 slice below closes this gap.

P1 slice: call-graph reachability, the overlay kind, and remaining plumbing¶

Implemented as a follow-up change on the same branch, closing P1 items 1, 2, 5, and 6 below in full and item 4 in full; item 3 (propagation-aware edge semantics) is closed to the extent described under its own entry.

Item 1 (call-graph evidence). New internal_leak.compute_call_graph_leak_paths(snap, internal_namespaces) walks the optional L5 source graph's DECL_CALLS_DECL/DECL_REFERENCES_DECL edges from every public entry (buildsource.source_graph.is_public_dependency_node), returning internal_decl_name -> [formatted proof paths] — the call-graph sibling to compute_leak_paths's layout walk, reusing source_graph_findings._dependency_reachability/_dependency_path/ _format_dependency_path (all three already existed, unwired for this purpose). MarkReachability now consults this as a second, independent evidence source: a change untouched by the layout walk (no field/base/ signature evidence at all) can still be tagged public_reachable=True via a pure call/reference edge. Requires an embedded L5 graph (--sources/--build-info, or the L2 header-only graph built automatically for --depth headers and above since G29 Phase A); returns {} and changes no behavior otherwise, mirroring poi.resolve_changed_paths_public_impact's own degrade contract.
Item 2 (overlay ChangeKind). New internal_symbol_required_by_public_api (BREAKING_KINDS, registered in change_registry_suppression.py alongside the P0 diagnostic to stay under change_registry.py's line cap). Built by new internal_leak.detect_call_graph_leaks/_build_call_graph_leak_change, wired into the existing DetectInternalLeaks pipeline step alongside detect_internal_leaks. Triggers only on a change whose own kind is already BREAKING_KINDS (artifact-proven; not API_BREAK_KINDS — see the post-merge review round below) and whose subject is internal-namespaced and call-graph-reachable — per the authority rule (ADR-028 D3/ADR-041), the graph edge composes with and explains an already-proven break; it never manufactures one, exactly like INTERNAL_TYPE_LEAKS_VIA_PUBLIC_API's own BREAKING classification.
Item 3 (edge semantics) — partially closed. reachability_kind grew a third real value, "symbol_availability", for the call-graph case — no longer the "two-value approximation" the P0 slice shipped with. The finer DECL_CALLS_DECL vs. DECL_REFERENCES_DECL distinction the item also names is preserved as text inside reachability_proof_path (via _format_dependency_path's --[EDGE_KIND]--> annotation) rather than a further split of reachability_kind itself — a deliberate stopping point, not an oversight: a machine-readable call-vs-reference sub-enum is a reasonable further increment but wasn't required to close the item's core ask (distinguishing symbol-availability edges from the two layout-based kinds). Left as a candidate future refinement.
Item 4 (structured report fields). public_reachable/ reachability_kind/reachability_proof_path now appear as first-class fields (not just inside the suppression_would_hide_public_break diagnostic's prose) in JSON (reporter._change_to_dict and _to_json_leaf's _leaf_entry — the latter handles root TYPE_* changes, the category the layout walk tags most often, and was easy to miss since it's a separate hand-rolled dict) and SARIF (sarif._result_for's properties, camelCased per that format's convention). JUnit was left untouched — it doesn't surface caused_by_type/correlated_change_kind either, so adding reachability fields there would be new precedent, not parity.
Item 5 (configurable internal-namespace convention). New PolicyFile.internal_namespaces: list[str] (parsed identically to frozen_namespaces), threaded via a new PipelineContext.internal_namespaces field through PostProcessingPipeline.run() (appended after the existing optional parameters, not inserted mid-signature — a Codex review on the PR caught that an earlier draft inserted it before scope_to_public_surface, which would have silently broken any positional caller of that parameter) to MarkReachability/DetectInternalLeaks/ DemoteUnreachableInternalChurn. Deliberately not threaded into DetectNamespacePatterns's experimental_namespaces — despite this item's own wording grouping all four steps together, that parameter governs an unrelated convention (the experimental:: graduation namespace, a different default token set), and conflating the two would reintroduce a bug, not fix one.
Item 6 (remaining checker.py call sites). _filter_suppressed_changes and _apply_surface_metrics now call SuppressionList.evaluate() and append the same _build_suppression_overreach_change() diagnostic ApplySuppression/_filter_pattern_synthetic already produce, instead of the boolean is_suppressed().

Post-merge review round (Codex), same P1 change:

Mangled symbol vs. demangled label — item 1 was inert on real binaries (fresh evidence). compute_call_graph_leak_paths keyed its result dict by node.label — the L5 graph's demangled qualified name for a SOURCE_DECLARES-backed decl (ns::detail::train_ops_dispatcher), or, for a call-graph-only fallback node, either the mangled name or a #sha256:-suffixed qualified name depending on provenance. But diff_symbols.py builds a real FUNC_REMOVED Change with symbol= the mangled linker name (_ZN2ns6detail19train_ops_dispatcherEv), and _root_type_name_for_change returns that verbatim for a function-shaped kind — so detect_call_graph_leaks's lookup by c.symbol almost never matched compute_call_graph_leak_paths's label-keyed result for a real, castxml/clang-parsed C++ removal; the whole item 1/2 mechanism only appeared to work in unit tests that hand-construct a Change.symbol equal to the graph label. Worse, detect_call_graph_leaks also pre-filtered its triggering-change candidates with is_internal_type(root, ...) — a check that splits on "::" — which a bare mangled name (no :: at all) always fails, rejecting every real candidate before the (already-broken) lookup even ran. Fixed both: compute_call_graph_leak_paths now also resolves each internal target's own exported symbol via its SOURCE_DECL_MAPS_TO_SYMBOL edge (the same binary_symbol:// identity source_graph.localize_symbol() already uses for the reverse direction) and records the proof paths under that mangled key too, alongside the existing label key — a node with no such edge (no linkage, e.g. fully inlined) gets no mangled key, but no FUNC_REMOVED-shaped Change could ever look one up anyway. detect_call_graph_leaks dropped its redundant is_internal_type pre-filter entirely: a hit in the call-path dict is already sufficient proof of "internal and call-graph-reachable", since compute_call_graph_leak_paths gates its own key insertion on is_internal_type(node.label, ...) (the qualified name, which does have :: segments) before ever adding either key. Added test_result_also_keyed_by_mangled_exported_symbol/ test_func_removed_matches_via_mangled_symbol_not_label to test_internal_leak.py, reproducing the real-world mangled-vs-label shape the prior tests' hand-picked matching names had masked.
Item 4's new fields needed a schema version bump (Codex). The three new per-finding JSON fields (public_reachable/reachability_kind/ reachability_proof_path) are additive optional keys per abicheck/schemas/__init__.py's own documented policy ("additive changes — new optional keys… bump the MINOR component"), the same discipline every prior additive field (2.1 through 2.5) already followed with its own changelog comment — missed here even though the schema's additionalProperties: true meant no test caught it (unregistered keys validate anyway). Bumped REPORT_SCHEMA_VERSION (originally to "2.6"; renumbered to "2.7" when rebasing onto main's own unrelated 2.6 bump for reviewer_action, #595) with a matching changelog comment, added the three fields (with reachability_kind's enum) to compare_report.schema.json, and re-synced the published docs/reference/schemas/v1/ copy via scripts/publish_schemas.py.
Header-graph mode still had the mangled-vs-label gap; API_BREAK_KINDS triggers were a category error (Codex, fresh evidence, two findings). (1) The mangled-symbol-key fix above only helps when the L5 graph carries a SOURCE_DECL_MAPS_TO_SYMBOL edge — the build-integrated L4/L5 path (source_graph.py) creates one, but the header-only path (header_graph.py, built automatically at --depth headers and above, no real build at all) never does, so the mismatch this review round already fixed once still applied for header-graph-only snapshots. Fixed by also trying each trigger's own Change.qualified_name (set by EnrichSourceLocations from Function.name — the same demangled name a graph node's label carries in either mode, independent of graph provenance) as a fallback lookup key in both MarkReachability and detect_call_graph_leaks, alongside the existing mangled-symbol key. (2) detect_call_graph_leaks's trigger set was BREAKING_KINDS | API_BREAK_KINDS, but API_BREAK_KINDS is the SOURCE_CONTRACT evidence tier — "a source-level break that needs a recompile… not necessarily a shipped ABI break" per checker_policy.py's own docstring — and most of its members (e.g. inline_function_removed, whose own inline comment reads "no exported symbol") have no removed linker symbol at all. Composing one into this overlay's "can fail to resolve this symbol at load time" description was a false binary-load-time claim for a change that was never one — the same category of mistake _LEAK_TRIGGERING_KINDS's own hand-curated (not "every breaking-shaped kind") trigger set was designed to avoid. Restricted the trigger set to BREAKING_KINDS only. Extended test_internal_leak.py with test_header_graph_mode_matches_via_qualified_name (no SOURCE_DECL_MAPS_TO_SYMBOL edge, mangled Change.symbol + qualified_name set, matching only via the fallback key) and test_api_break_kind_is_not_a_trigger (an API_BREAK_KINDS member with call-graph evidence produces no overlay).
A third mangled-label shape, this time at classification, not key matching (Codex, fresh evidence). augment_graph_with_calls (call_graph.py) adds a fallback source_decl node — with no SOURCE_DECL_MAPS_TO_SYMBOL edge at all — for a callee that has no other node in the graph yet, labelling it via function_decl_identity, which returns the raw mangled name for any ordinary (non-extern "C") C++ function. A bare mangled name has no :: segments, so is_internal_type(node.label, ...) rejected it before compute_call_graph_leak_paths even reached the dual-key logic the first fix in this round added — the entry was silently dropped at classification, one step earlier than either of the two previously-fixed shapes. Fixed by demangling (abicheck.demangle.demangle, already used elsewhere in this codebase for the same mangled↔qualified correlation problem) only for this classification check when the label looks mangled (startswith("_Z")) — the stored key stays the original mangled label unchanged, since that already equals a real FUNC_REMOVED's Change.symbol directly (both are the same canonical Itanium-mangled linker symbol), so no further key-matching change was needed once classification correctly recognizes it as internal. Added test_mangled_only_label_demangled_for_classification/ test_mangled_label_not_internal_after_demangling_stays_dropped to test_internal_leak.py (the latter confirming demangling only changes what counts as internal, not a blanket allowance for every mangled label).
A fourth internal-namespace-threading gap: DetectTemplatePatterns (Codex, fresh evidence). Item 5's PolicyFile.internal_namespaces work threaded ctx.internal_namespaces through MarkReachability/ DetectInternalLeaks/DemoteUnreachableInternalChurn, deliberately excluding DetectNamespacePatterns (a different, unrelated experimental_namespaces convention). DetectTemplatePatterns is a distinct, genuine fourth case that was simply missed: detect_internal_template_leaks's own _INTERNAL_TEMPLATE_NAMESPACES (detail/impl/internal/__detail/_impl, plus __internal) is the same internal-implementation convention the other three steps use, but DetectTemplatePatterns.run() called detect_template_patterns(ctx.old, ctx.new) with no namespaces argument at all — a project with a custom convention (e.g. priv) would have its MarkReachability tag corrected but INTERNAL_TEMPLATE_LEAKS_VIA_PUBLIC_API still blind to it. Gave DetectTemplatePatterns the identical namespaces constructor parameter and self._namespaces or ctx.internal_namespaces or _INTERNAL_TEMPLATE_NAMESPACES fallback the other three steps use. Added test_pipeline_internal_namespaces_reaches_detect_template_patterns to test_reachability_aware_suppression.py.
A fourth node-label shape: hash-suffixed identities (Codex, fresh evidence). function_decl_identity (source_graph.py) has a third branch beyond mangled-name and bare-qualified-name: a declaration with no distinct mangled name (e.g. extern "C") gets "{qualified_name}#sha256:{digest}". compute_call_graph_leak_paths stored this raw hash-suffixed string as its only key for such a node, which a real Change.symbol/qualified_name never carries — the same key-mismatch bug class as the mangled-label case, one shape later. Fixed by also indexing the hash-stripped qualified name (splitting off "#sha256:...") alongside the existing label/mangled-symbol keys. Added test_hash_suffixed_label_also_keyed_by_stripped_name to test_internal_leak.py.
The call-graph entry set itself over-reached (Codex, fresh evidence). Every fix above was about matching a target symbol correctly once a walk from some public entry already reached it — this one is about which nodes get to seed the walk at all. compute_call_graph_leak_paths seeded entries via is_public_dependency_node (shared with crosscheck.py's advisory public_to_internal_dependency check): exported-symbol-mapped or public-header-visible, with no further distinction. But an ordinary, out-of-line exported function (e.g. api() defined in a .cpp file) is public in exactly that sense while its body — and therefore its own internal calls, e.g. to ns::detail::helper() — is compiled into the library's binary only, never into any consumer's; a consumer links against api()'s exported symbol alone and never sees, references, or embeds helper(). If helper() is removed, either the library's own build breaks (the vendor's problem, nothing to do with any external consumer) or api()'s recompiled body simply stops calling it — never a consumer-visible break. Treating every exported function as a valid call-graph entry (as the shared predicate does) therefore either manufactured a spurious "still reachable" narrative on a genuinely safe-to-suppress internal change, or — via post_processing.MarkReachability's public_reachable tag — blocked a broad internal-namespace suppression rule from ever applying to the common case, since most functions in most libraries are ordinary, out-of-line, non-template. The real criterion is whether the entry's own body is emitted into every including translation unit (true for inline functions/methods and templates, false for an ordinary out-of-line definition) — but that distinction, while computed at the L4 SourceAbiSurface layer (three separate reachable_declarations/ reachable_templates/reachable_inline_bodies buckets), was lost when folded into the L5 graph: an inline function generates two entities sharing one identity (a plain "function" declaration entity clang.py always emits, plus a sibling "inline" body entity), both collide onto the same graph node id, and add_node's first-writer-wins dedup keeps only the "function" entity's attrs["decl_kind"] (iterated first) — so even a decl_kind-based check would silently never see "inline" for exactly the functions that matter. Fixed in two parts, scoped to internal_leak.py's call-graph walk only (deliberately not touching crosscheck.py's advisory, RISK-only, non-suppression-gating check, which has no comparable precision requirement): (1) build_source_graph now computes the inline/template identity set up front and stamps attrs["consumer_compiled_body"] on every decl node from that set membership — so the attr is correct regardless of which sibling entity wins the id race; (2) a new is_consumer_compiled_public_entry predicate (buildsource/source_graph.py) layers that attr on top of is_public_dependency_node, defaulting permissively to True when the attr is absent (e.g. a header_graph.py node, which by construction only ever gets outgoing call/reference edges from in-header bodies in the first place, so the over-reach cannot arise there) — compute_call_graph_leak_paths now seeds its walk from this predicate instead. Added test_ordinary_function_decl_node_marked_not_consumer_compiled/ test_inline_function_decl_node_marked_consumer_compiled_despite_id_collision to test_source_graph.py (the id-collision case specifically) and test_ordinary_out_of_line_exported_entry_is_not_a_leak_path/ test_inline_entry_with_explicit_flag_is_still_a_leak_path to test_internal_leak.py.
Restricting the entry set was not enough on its own — the walk also over-reached (Codex, fresh evidence). The fix above stops an ordinary out-of-line exported function from seeding the call-graph walk, but compute_call_graph_leak_paths still computed reachability via the shared source_graph_findings._dependency_reachability, which expands every edge transitively from an already-validated entry with no further restriction. So a public inline wrap() calling an ordinary out-of-line exported api() (consumer_compiled_body: false), which itself calls an internal ns::detail::helper(), still had helper() show up in wrap()'s reachable set: api() is a legitimate entry-adjacent node (a consumer really does link against api()'s exported symbol, so it must stay recorded as reachable), but whatever api() calls happens entirely inside the library's own binary — the walk must stop expanding past such a node, not just refuse to start from one. Fixed by replacing the shared (and, for crosscheck.py's broader advisory use, correctly unrestricted) reachability helper with a new, internal_leak.py-local _consumer_compiled_reachability: a BFS that records every direct successor of a node but only continues expanding past a successor whose own consumer_compiled_body is true (defaulting permissively when the attr is absent, same rule as the entry predicate). It also returns each entry's predecessor-edge map so _reconstruct_path can rebuild the displayed proof path by replaying the same restricted walk, rather than calling the shared, unrestricted _dependency_path a second time and risking a displayed route the restriction above would not itself take. crosscheck.py's public_to_internal_dependency (RISK-only, never suppression-gating) is untouched — it still uses the original unrestricted _dependency_reachability/_dependency_path, since its broader "does any decl-dependency edge exist at all" question has no comparable precision requirement. Added test_walk_stops_expanding_past_non_consumer_compiled_intermediate to test_internal_leak.py. Post-merge review (Codex), one more node shape the "permissive default" missed. The fix above's permissive default (treat a node as consumer-compiled when consumer_compiled_body is simply absent) was scoped too broadly: it also covered a real, build-integrated call_graph.py fallback node — augment_graph_with_calls creates one, tagged provenance="call_graph", for a caller/callee identity with no other declaration node backing it (e.g. a project helper function the L4 declarations pass never separately captured) — which has no consumer_compiled_body attr at all, unlike the deliberate False the previous fix's own test used. A public inline wrap() calling such an intermediate (demo::helper_a, this exact fallback shape) which itself calls an internal helper() still had helper() read as reachable, since "no attr at all" fell through to the permissive branch. Fixed by narrowing the exception: the permissive default now only yields to a conservative False when the node's provenance is specifically the call_graph.py fallback tag (_CALL_GRAPH_FALLBACK_PROVENANCE) — every other attr-less node (header-graph nodes, type nodes, synthetic test fixtures with no provenance at all) keeps the original permissive default, since "no signal either way" is not the same claim as "known to be an uncertain build-integrated declaration." (An earlier version of this fix tried an allowlist — permissive only for a recognized header_graph.py provenance tag, conservative for everything else — but that regressed a wide swath of the existing test suite, whose synthetic fixtures never set a provenance at all; the blocklist framing above is the one that closes the real gap without disturbing tests that carry no opinion on the question.) Shared the fix as a new source_graph.is_consumer_compiled_node predicate consumed by both is_consumer_compiled_public_entry (the entry check) and internal_leak._is_consumer_compiled_node (the walk's own expand-past-this-node check), so the two can never drift out of sync again. Added test_walk_stops_at_call_graph_fallback_node_with_no_signal to test_internal_leak.py.

Roadmap (not committed — scope/sequence per the usual planning process)¶

P1 is implemented (above); P2 remains open, numbering mirrors the original review's priority tiers.

P2 — empirical validation¶

~~Consumer import manifests: --consumer-binary/--consumer-dir, ELF undefined-dynamic-symbol / PE-import / Mach-O-undefined-symbol collection from a baseline-built consumer, producing a consumer_required_symbol_removed finding when the candidate library no longer exports something a real consumer's baseline build referenced.~~ Closed — but not the way this item's own wording assumed. This infrastructure already existed: compare --used-by APP (ADR-005/ADR-043, appcompat.py) already collects a real consumer binary's ELF undefined symbols / PE imports / Mach-O undefined symbols and diffs them against the new library's export table — this item's roadmap text was written without apparent awareness of it, so no new --consumer-binary/ --consumer-dir flags or extraction code were needed. The genuine gap was narrower: a missing symbol was only a bespoke string in AppCompatResult.missing_symbols, special-cased by each reporter format (reporter.py/sarif.py/junit_report.py), never a real suppressible Change/ChangeKind the way PE_ORDINAL_RETARGETED already is. Added ChangeKind.CONSUMER_REQUIRED_SYMBOL_REMOVED (BREAKING_KINDS) and, in scope_diff_to_app, promoted every missing symbol not already represented by a library-diff Change (via the existing uncovered_missing_symbols dedup — the same helper _scoped_severity_summary already uses to avoid double-counting) into one, following the _check_pe_ordinal_imports/PE_ORDINAL_RETARGETED precedent exactly. AppCompatResult.missing_symbols (the raw string list) is untouched for backward compatibility with existing reporter code; the new Changes are purely additive into breaking_for_app. AppCompatResult.verdict's existing missing-symbols-force-BREAKING shortcut is also untouched (no exit-code/verdict behavior change) — this is enrichment (a real ChangeKind, docs, evidence tier, suppressibility in principle), not a severity change. Scoped to --used-by only (the ADR's literal "consumer's baseline build" framing); the sibling --required-symbol/plugin-host scoping paths have the identical ad-hoc-string shape and are a natural, structurally-identical follow-up, not attempted in this round.
~~Old-consumer/new-library execution harness (LD_BIND_NOW=1, optionally ASan/UBSan) as an opt-in validation capability alongside the static scanner, not a replacement for it.~~ Closed for the LD_BIND_NOW core; ASan/UBSan deliberately deferred, per the item's own "optionally". New abicheck/runtime_probe.py module + --verify-runtime flag (opt-in, valid only with --used-by): runs each consumer binary once against the old library and once against the new one, both times with LD_BIND_NOW=1 and LD_LIBRARY_PATH pointed at the respective library. Deliberately narrow detection: the only signal recognized is glibc's own symbol lookup error: ... undefined symbol: X on stderr — the dynamic linker's unambiguous statement that eager binding failed to resolve a real symbol. An app's own exit code or general crash behavior is explicitly not interpreted (too noisy/unreliable — an app can exit nonzero for reasons that have nothing to do with the library). New ChangeKind.CONSUMER_RUNTIME_LOAD_FAILED (RISK_KINDS, never BREAKING on its own — an execution environment can fail for unrelated reasons, so this only corroborates the static scanner, per the authority rule) fires only when the app ran cleanly against the old library but the linker names a missing symbol against the new one (RuntimeProbeResult.regressed_symbol). Linux-only (LD_BIND_NOW/ LD_LIBRARY_PATH are glibc/ELF mechanisms; macOS's DYLD_* env vars are stripped by SIP for most binaries, Windows has no equivalent) — skips silently on any other platform or when OLD/NEW aren't real binaries (mirrors abicheck/bundle.py's ELF-only degrade precedent), never raises. Folded into the same _apply_used_by_scoping worst-wins exit-code/verdict machinery PE_ORDINAL_RETARGETED/ CONSUMER_REQUIRED_SYMBOL_REMOVED already use, via a synthetic Change appended to breaking_for_app — no separate reporting path. Post-merge review (Codex), same change: the new scoped-only Changes (PE_ORDINAL_RETARGETED/CONSUMER_REQUIRED_SYMBOL_REMOVED/ CONSUMER_RUNTIME_LOAD_FAILED) rendered through the plain _change_to_dict/_result_for path (not appcompat_to_json's own override) reported evidence_status: artifact_proven purely from their BREAKING/RISK category — even though none of them come from an artifact-level library diff at all; the evidence is the consumer's own import table or execution. A pre-existing gap for PE_ORDINAL_RETARGETED that the two new sibling kinds simply inherited. Fixed by adding an evidence_status_override parameter to sarif._result_for (mirroring reporter._change_to_dict's existing one) and passing EvidenceStatus.CONSUMER_PROVEN at both scoped_only_changes render sites (JSON's _fold_scoped_compat_into_text, SARIF's to_sarif). Post-merge review (Codex), one more finding after threading suppression through: the suppression fix above only dropped the suppressed symbol from the synthesized Change; the same symbol was left in AppCompatResult.missing_symbols (the raw string list), which _compute_appcompat_verdict checks independently and unconditionally forces Verdict.BREAKING on — and which the scoped exit-code floor and missing-label text output also read directly. Suppressing the overlay Change alone was therefore cosmetic: the verdict/exit code/report text still failed on a symbol the user had explicitly (and correctly) waived. Fixed by also removing the symbol from missing_symbols itself when its overlay is suppressed — this overlay is the suppressible representation of a missing symbol (the entire point of promoting it out of a bespoke string), so a suppressed overlay must remove the raw string from every consumer of it, not just the Change list. symbol_coverage is deliberately computed from the pre-suppression count: it is a factual metric about the export table, not a gate, and should not be made to lie because a finding was waived. Post-merge review (Codex), three more findings on the same P2 change: (a) runtime_probe._run_once passed a bare Path straight to subprocess.run([str(app_path)], ...) — for a relative app name with no / (e.g. a --used-by app invocation from the app's own directory), that argv[0] shape makes the OS search PATH, not the current directory, exactly like an unqualified shell command; with . typically absent from PATH this silently raised OSError on both runs and the probe never fired at all. Fixed by resolving to an absolute path first (app_path.resolve()), same as lib_path already was. (b) Both CONSUMER_REQUIRED_SYMBOL_REMOVED (scope_diff_to_app) and CONSUMER_RUNTIME_LOAD_FAILED (_apply_used_by_scoping) are synthesized after the comparison pipeline's own suppression pass has already run over diff.changes — neither function received the active SuppressionList at all, so an exact symbol:/change_kind: suppression rule for either overlay finding could never actually suppress it; the scoped gate would keep failing on a change the user had explicitly (and correctly) waived. Fixed by threading suppression through scope_diff_to_app (and its check_appcompat/MCP-server call sites) and _apply_used_by_scoping, evaluating it against each synthesized Change before appending — mirrors how the base diff was already suppressed, just applied a second time to findings that did not exist yet at that point. (c) _apply_used_by_scoping appended the CONSUMER_RUNTIME_LOAD_FAILED finding to scoped.breaking_for_app after scope_diff_to_app had already computed scoped.verdict from the static scope alone — so a run with a clean static scope but a real runtime regression still reported the stale COMPATIBLE verdict instead of COMPATIBLE_WITH_RISK, even though the finding list itself was correct. Fixed by recomputing scoped.verdict via _compute_appcompat_verdict immediately after the append. Post-merge review (Codex), one more finding: the reachability gate itself, not just whether suppression ran at all. Both overlays' suppression evaluation (fixed above) still left public_reachable at the Change dataclass default (False) — but unlike an ordinary internal-namespace Change, where "maybe internal, maybe not" is exactly the ambiguity MarkReachability's walk exists to resolve, these two overlays have no such ambiguity at all: CONSUMER_REQUIRED_SYMBOL_REMOVED only ever exists because a real --used-by consumer's own undefined-symbol requirement genuinely resolved to nothing in the new library, and CONSUMER_RUNTIME_LOAD_FAILED only ever exists because the dynamic linker itself failed to resolve a symbol for a real, executed consumer binary — both are consumer-proven by construction. Left at the default, Suppression._passes_reachability_gate's "unreachable-only" default for a broad namespace/source_location rule reads public_reachable=False and matches, silently suppressing a break that can never actually be safe to hide the way an ordinary internal-namespace change sometimes is — precisely the failure mode this whole ADR exists to prevent, just missed for these two synthesized overlays specifically. Fixed by constructing both with public_reachable=True (a new reachability_kind value, "consumer_proven") before suppression evaluation, and switching both call sites from the cheaper is_suppressed to evaluate so a broad rule withheld by the allow_public_break gate (CONSUMER_REQUIRED_SYMBOL_REMOVED is BREAKING, so this gate applies to it) still emits the same SUPPRESSION_WOULD_HIDE_PUBLIC_BREAK diagnostic ApplySuppression produces for changes it sees directly, reusing post_processing._build_suppression_overreach_change rather than duplicating its construction (mirrors checker.py's own _filter_suppressed_changes, already routed through evaluate per P1 item 6). CONSUMER_RUNTIME_LOAD_FAILED is RISK-tier, so would_withhold never fires for it by design (a RISK-classified, reachability-mismatched change is the rule correctly declining to apply, not an overreach worth a diagnostic) — the same evaluate-based plumbing is still used for consistency and to stay correct if that kind's tier ever changes. A narrow symbol:/change_kind: rule is unaffected either way (exempt from both gates, unchanged behavior). Post-merge review (Codex), two more findings on the public_reachable fix itself: (a) the compare-report JSON schema's reachability_kind enum only listed the four public-surface-walk values (direct_public_symbol/value_embedding/pointer_or_signature/ symbol_availability) — a report containing either overlay now emits reachability_kind: "consumer_proven", which failed schema validation even though the report still advertised a passing report_schema_version. Fixed by adding "consumer_proven" to the enum (an additive change per the schema's own stability policy, abicheck/schemas/__init__.py) and bumping REPORT_SCHEMA_VERSION (originally to 2.7; renumbered to 2.8 in the same rebase mentioned above), re-synced to docs/reference/schemas/v1/ via scripts/publish_schemas.py. (b) Unrelated to the reachability fix but on the same file: runtime_probe._run_once's subprocess.run(..., text=True) has no errors= handling, and a real executable's stderr is arbitrary bytes with no guarantee of being valid UTF-8 (or the locale's encoding) — a non-UTF-8 byte would raise UnicodeDecodeError after the child process exits, escaping this best-effort probe entirely and aborting the whole compare invocation instead of degrading to a RuntimeProbeOutcome, exactly the failure mode the surrounding try/except OSError was meant to prevent. Fixed by adding errors="replace" so malformed bytes are substituted, not fatal — the symbol-lookup-error regex still matches the valid ASCII segments around them. Added a real-subprocess regression test (not a mocked subprocess.run, since the decoding itself is what's under test) to test_runtime_probe.py, and a jsonschema-validating regression test to test_cov95_cli.py for the enum fix. Post-merge review (Codex), one more finding on runtime_probe.py: _SYMBOL_LOOKUP_ERROR_RE's capture group was \S+, which does not stop at a comma — glibc appends ", version X" after the bare name for a versioned undefined-symbol failure (e.g. "undefined symbol: foo, version FOO_1.0"), so the captured group was "foo," (trailing comma included), not the real import/export name "foo". The synthesized consumer_runtime_load_failed finding therefore carried a symbol that could never match an exact suppression rule for the real symbol, and --verify-runtime reported the wrong name for every versioned import. Fixed by changing the capture group from \S+ to [^,\s]+ (stop at a comma or whitespace, whichever comes first) — the unversioned case is unaffected since there is no comma to stop at. Added test_versioned_symbol_lookup_error_strips_version_suffix to test_runtime_probe.py. Post-merge review (Codex), two more findings, one per module. (a) internal_leak.py: demangle() returns the full signature (e.g. ns::api::foo(ns::detail::T*)), not just the qualified name — the classification check at the call-graph fallback-node site (see the third-Codex-round fix above) passed this straight to is_internal_type, whose segment scan finds detail inside the parameter type and misclassifies an ordinary public function (ns::api::foo) as an internal leak target merely because it takes/returns an internal type — even though foo itself lives in the public ns::api namespace. A later func_removed on foo would then spuriously also emit internal_symbol_required_by_public_api, describing a genuinely public symbol's removal as if it were an internal one leaking through. Fixed by a new _strip_signature_params helper (tracks paren depth, cuts at the first depth-0 ( — the function's own parameter-list opening, not a nested one from e.g. a function-pointer parameter type) applied before classification; the stored lookup key is unaffected, only the classification input. Added TestStripSignatureParams (unit-level) and test_public_fn_with_internal_param_type_not_misclassified (integration-level) to test_internal_leak.py. (b) runtime_probe.py: a dynamic-linker failure for a reason other than an undefined symbol — most commonly a missing, unrelated dependency ("error while loading shared libraries: libfoo.so.2: cannot open shared object file") — does not match _SYMBOL_LOOKUP_ERROR_RE, so it fell through to ok=True regardless of the actual (nonzero) exit code. An old probe run that never actually loaded (for a reason unrelated to the library under comparison) would then still satisfy regressed_symbol's old.ok guard, letting a new run's real undefined-symbol failure report a false regression against a baseline that was never clean to begin with. Fixed by adding a second, narrower regex (_LOADER_ERROR_RE) for this exact, version-stable glibc substring, returning ok=False (no missing_symbol, since the failure isn't attributable to a specific symbol) when it matches — deliberately narrow, so the module's own documented "an app's own nonzero exit code is common and meaningless on its own" principle stays intact for every other failure shape; this is one more unambiguous linker statement, the same class as the symbol-lookup-error case already handled, not a general exit-code interpretation. Added test_loader_failure_for_unrelated_reason_is_not_ok to test_runtime_probe.py. A independent review pass (not Codex) of the full diff also found two documentation-only inaccuracies: examples/case192_.../README.md described the internal_symbol_required_by_public_api overlay as "RISK-classified" — it is actually BREAKING (change_registry_suppression.py), confirmed by ChangeKind.INTERNAL_SYMBOL_REQUIRED_BY_PUBLIC_API in BREAKING_KINDS at runtime; and this ADR's own "Consequences" section still read "No new CLI flags; no schema/serialization version bump" — true for the original P0 slice that bullet was written for, but directly contradicted by the P1/P2 slices documented earlier in this same file (REPORT_SCHEMA_VERSION bumped twice, --verify-runtime added). Both fixed. Post-merge review (Codex), one more finding on the direct-public-symbol check itself: "When the public-header declaration is a C++ function/variable, _public_header_names() records the demangled/qualified model name, while a removal/type-change root is still often the linker symbol (for example _ZN...), and only functions may have c.qualified_name filled by EnrichSourceLocations. This direct-public branch therefore misses a public-header ns::detail::api() removal, so a broad namespace suppression such as namespace: "**::detail::*" treats the hard FUNC_REMOVED as unreachable and suppresses it with no overreach diagnostic." Verified directly: _root_type_name_for_change(c) is c.symbol verbatim for a function-shaped change, and diff_symbols.py sets Change.symbol to the mangled linker name for FUNC_REMOVED/FUNC_ADDED/etc., while _public_header_names() collects Function.name, which is demangled — so root in public_header_names never matches for a real C++ symbol. A standalone public entry point that nothing else embeds or calls (so neither the layout walk nor the call-graph walk independently proves it reachable) therefore fell through MarkReachability's direct-public-symbol check entirely, leaving public_reachable at its default False and letting a broad namespace/source_location rule hide the removal with no suppression_would_hide_public_break diagnostic. Fixed by also checking c.qualified_name (populated by EnrichSourceLocations, which runs earlier in DEFAULT_PIPELINE, from the demangled Function.name for exactly the FUNC_REMOVED/FUNC_REMOVED_ELF_ONLY kinds this matters for) against public_header_names alongside root. Added test_public_header_cxx_function_removal_is_reachable to test_reachability_aware_suppression.py. Post-merge review (Codex), one more finding on runtime_probe.py: "When OLD and NEW are different files in the same directory, both _run_once calls compute the same LD_LIBRARY_PATH and execute the same app with an identical environment, so --verify-runtime never actually switches the consumer from the old library to the new one." Confirmed: _run_once set LD_LIBRARY_PATH to just lib_path.resolve().parent — when old_lib and new_lib are two different files sitting in the same directory (a realistic layout: versioned build artifacts, or a symlink the "old" build already left pointing at the SONAME the consumer's DT_NEEDED actually requests), the dynamic loader resolves the requested SONAME to whichever file in that shared directory happens to match first — independent of which of old_lib/new_lib this specific _run_once call was asked to test. A regression that only shows up against the new library could silently probe the old one twice and report a false clean bill of health. Fixed by staging each side's library, alone, into its own fresh tempfile.TemporaryDirectory(), under the name the dynamic linker will actually look up (DT_SONAME when present — read via the existing elf_metadata.parse_elf_metadata, since a file's on-disk name can differ from its embedded SONAME — falling back to the file's own basename on any parse failure, empty SONAME, or a defensively-rejected path-shaped SONAME), and listing that staging directory first on LD_LIBRARY_PATH (the library's real parent directory is still appended after it, so any other same-directory runtime dependency stays resolvable) — this forces each run to resolve to exactly the intended file regardless of what else sits alongside it. Added test_same_directory_old_new_libraries_resolve_independently and TestLoadName (four cases: SONAME present, parse failure, empty SONAME, path-shaped SONAME) to test_runtime_probe.py. Post-merge review (Codex), two more findings, one per module. (a) buildsource/source_graph.py: "When a build-source pack includes Kythe/CodeQL call edges, the adapters create source_decl nodes with provenance "kythe"/"codeql" and no consumer_compiled_body attribute; this fallback therefore treats those attr-less intermediate callees as consumer-compiled and keeps expanding through their bodies... reintroducing the overreach this predicate was meant to avoid for external graph backends." Confirmed: graph_backends.py's ingest_kythe_entries/ ingest_codeql_call_results/ingest_codeql_extends_results create exactly the fallback-node shape the prior Codex-review fix closed for call_graph.py's own fallback tag (is_consumer_compiled_node treating "no consumer_compiled_body attr" as safe-to-continue unless provenance == "call_graph") — but tagged provenance="kythe"/"codeql" instead, which the blocklist never covered, so an inline public wrapper → ordinary out-of-line helper → detail::leaf chain imported from an external indexer still walked straight through the helper. Fixed by generalizing _CALL_GRAPH_FALLBACK_PROVENANCE (a single string) into _NO_CONSUMER_COMPILED_SIGNAL_PROVENANCES (a frozenset of {"call_graph", "kythe", "codeql"}); is_consumer_compiled_node now checks membership in that set. Added test_walk_stops_at_kythe_codeql_node_with_no_signal (parametrized over both provenances) to test_internal_leak.py, alongside the existing test_walk_stops_at_call_graph_fallback_node_with_no_signal it mirrors. (b) runtime_probe.py: "When the consumer binary has legacy DT_RPATH without DT_RUNPATH, glibc searches that RPATH before LD_LIBRARY_PATH... so putting the staged directory first here does not force the old/new library selection." Confirmed against ld.so(8)'s documented search order: a legacy DT_RPATH (present with no DT_RUNPATH) is searched before LD_LIBRARY_PATH; DT_RUNPATH (what a modern toolchain actually emits) is searched after it — so the same- directory staging fix just above cannot force the loader's choice for a binary built with this legacy shape, and both the old and new probe runs could silently resolve through the embedded RPATH instead, defeating --verify-runtime's one purpose for exactly the binaries most likely to need it (older/vendored build setups). Rather than attempt a fragile override (e.g. LD_PRELOAD, which would load both copies of the library into one process and risk misattributing which copy's global/static state an undefined-symbol failure actually reflects — a correctness risk this probe's own "airtight claim, not inferred" design principle rules out), added _has_legacy_rpath_without_runpath() (reads the app's own DT_RPATH/DT_RUNPATH via parse_elf_metadata) and a new early-exit in run_runtime_probe: this exact shape now declines with attempted=False and an explanatory skipped_reason, instead of silently reporting a result the module cannot actually attribute to the library under test. A DT_RUNPATH present alongside a DT_RPATH makes ld.so ignore the DT_RPATH entirely, so that combination is unaffected. Added test_legacy_rpath_without_runpath_skips and test_runpath_alongside_rpath_does_not_skip to test_runtime_probe.py.
~~New worked examples exercising this ADR's headline scenario end-to-end (public inline dispatch to an exported internal specialization; the same case under a blanket namespace suppression, asserting the break survives and the diagnostic fires; a safe pimpl counter-example) — the review's examples A/B/D are the most valuable regression coverage and are natural examples/case*/ additions now that P1 item 1's call-graph reachability is wired (previously blocked on it).~~ Closed for A/B combined into one case, plus a deliberate negative-space counter-example in place of the literal pimpl case. examples/case192_call_graph_break_survives_suppression ships the full A/B scenario in one case (a public inline dispatcher's call into a removed internal specialization: BREAKING unmodified, refused under a broad namespace: "demo::detail::**" suppression rule with the suppression_would_hide_public_break diagnostic naming the proof path, then NO_CHANGE once the same rule adds allow_public_break: true). examples/case193_ordinary_exported_fn_call_not_reachable is the counter-example this round actually needed more: not the type-layout pimpl shape (already covered by case118-case120's public-surface scoping), but the call-graph walk's own negative-space check — an ordinary out-of-line exported function's internal call is not public-reachable, so the identical broad suppression rule applies cleanly with no diagnostic at all. Building it is what surfaced the transitive-traversal over-reach documented in the P1 slice above (a third example, a literal pimpl-via-graph case, remains a nice-to-have follow-up, not attempted in this round). Both ship hand-built AbiSnapshot pairs with an embedded L5 graph (scripts/gen_reachability_examples.py), validated compiler-free by tests/test_reachability_examples.py.

Consequences¶

A suppression file with only narrow (symbol/symbol_pattern/ type_pattern) rules sees no behavior change from D2/D4 — the reachability guard's default only engages for namespace/ source_location rules.
An existing broad namespace/source_location suppression rule that happens to also match public-reachable churn will, after this change, stop suppressing that subset of findings by default (they reappear in the report, tagged with the new diagnostic explaining why) — this is the intended fix, but is a visible behavior change for any suppression file relying on the old any-reachability default. allow_public_break: true is the escape hatch for a rule the maintainer has actually reviewed.
namespace's caused_by_type matching is removed outright (D3); a rule that depended on it (none found in this repo's own test suite/examples at authoring time) needs cause_namespace instead.
SUPPRESSION_WOULD_HIDE_PUBLIC_BREAK is a new ChangeKind, following the standard four-step procedure (/CLAUDE.md "Adding a new ChangeKind"). This P0 slice itself added no new CLI flags and no schema/serialization version bump; both since happened as the P1/P2 slices landed — REPORT_SCHEMA_VERSION bumped to 2.7 (P1 item 4's structured public_reachable/reachability_kind/ reachability_proof_path fields) and again to 2.8 (P2's "consumer_proven" reachability_kind enum member; both numbers shifted up by one from their original 2.6/2.7 when this branch was rebased onto main's own unrelated 2.6 bump for reviewer_action, #595), and --verify-runtime is a new P2 CLI flag. See abicheck/schemas/__init__.py's own changelog for the exact history.

References¶

abicheck/post_processing.py — DEFAULT_PIPELINE, PipelineContext, MarkReachability, ApplySuppression, DetectInternalLeaks, DemoteUnreachableInternalChurn
abicheck/internal_leak.py — compute_leak_paths, compute_call_graph_leak_paths, detect_internal_leaks, detect_call_graph_leaks, _LEAK_TRIGGERING_KINDS, _root_type_name_for_change
abicheck/suppression.py — Suppression, SuppressionList
abicheck/checker_types.py — Change
abicheck/checker.py — _filter_suppressed_changes, _apply_surface_metrics
abicheck/policy_file.py — PolicyFile.internal_namespaces
abicheck/reporter.py, abicheck/sarif.py — structured reachability fields
abicheck/buildsource/source_graph.py/source_graph_findings.py — the L5 graph and _dependency_reachability/_dependency_path/ _format_dependency_path the P1 slice's call-graph walk reuses
ADR-004 — Report filtering and deduplication (redundancy-before-verdict invariant this ADR deliberately does not disturb)
ADR-013 — Suppression system design (pipeline-ordering rationale this ADR amends)
ADR-024 — Public ABI surface resolution (audit-ledger / never-silently-drop convention this ADR follows for suppression_would_hide_public_break)
ADR-028 — Build-source evidence pack (the authority rule the P1 overlay kind's BREAKING classification relies on: L3-L5 evidence may explain/ correlate an artifact-proven break, never manufacture one)
ADR-041 — Compiler-facts semantic impact graph (PUBLIC_API_INTERNAL_DEPENDENCY_ADDED, the L5 graph schema the P1 slice's call-graph walk reuses)