Skip to content

fix: key resource-import fallback by name, eliminate sentinel (closes #99)#115

Merged
avrabe merged 1 commit into
mainfrom
fix/issue-99-resource-fallback-sentinel
Apr 28, 2026
Merged

fix: key resource-import fallback by name, eliminate sentinel (closes #99)#115
avrabe merged 1 commit into
mainfrom
fix/issue-99-resource-fallback-sentinel

Conversation

@avrabe
Copy link
Copy Markdown
Contributor

@avrabe avrabe commented Apr 28, 2026

Summary

Closes #99.

build_resource_type_to_import previously had a fallback branch that
collapsed every [resource-rep] / [resource-new] core import onto a
single map slot keyed (0u32, prefix) whenever a component imported
resources without canonical entries. That sentinel was intentional for
the single-resource case but is a known foot-gun for any future code
path that writes multiple resources into it.

Approach: option (b) — key by resource name

Per the issue's two options for hardening, this PR implements option
(b)
: key the import-only fallback by the resource short-name extracted
from the import field (e.g., [resource-rep]frequency → key
("frequency", "[resource-rep]")). The sentinel is gone, and so is the
downstream used_sentinel gate in resolve_resource_positions.

Why option (b) over (a):

  • The fallback is genuinely useful (covers import-only components), so
    preserving it with safe keying is preferable to asserting a
    single-resource invariant that future work could legitimately need to
    break.
  • The downstream consumer already relied on a sentinel check to mark
    resolved entries as "callee does not define this resource"; with the
    sentinel removed, that signal becomes "matched via the name index"
    rather than "matched via type-id 0", which is a cleaner predicate.

Concrete changes

  • New ResourceImportMap struct with two indices:
    • by_type_id: HashMap<(u32, &'static str), (String, String)>
      populated from canonical ResourceRep / ResourceNew entries plus
      Step-5 missing-op inference and Step-6 alias propagation. Identical
      semantics to the pre-existing primary map.
    • by_name: HashMap<(String, &'static str), (String, String)>
      populated by scanning core module imports for [resource-rep]<rn>
      / [resource-new]<rn>. Replaces the sentinel slot.
  • New ResourceImportMap::resolve(component, type_id, prefix) returns
    Option<(&entry, matched_via_name_fallback: bool)>. Try by_type_id
    first; on miss, translate the type id to a resource short-name via
    ParsedComponent::resolve_resource_type and look up in by_name.
  • resolve_resource_positions — drops the used_sentinel gate. The
    flag now reads "did this match come from the name index?", which is
    the same logical condition without the sentinel.
  • resolve_inner_resource_imports / resolve_one_layout — extended to
    take the parsed component so they can use the name fallback for
    inner-list resource borrows.
  • All call sites in resolver.rs updated; no signatures outside the
    module changed.

Test plan

  • cargo test --lib -p meld-core — 181 unit tests pass (was 177 +
    4 new for issue Design: resource-fallback sentinel in build_resource_type_to_import #99).
  • cargo test --release --test wit_bindgen_runtime — 73 tests
    pass.
  • cargo test --release — full workspace green.
  • cargo fmt --all -- --check clean.
  • cargo clippy --workspace --all-targets -- -D warnings clean.
  • Pre-commit hooks (cargo-fmt + cargo-clippy + cargo-test) pass.

New tests

meld-core/src/resolver.rs:

  • test_issue_99_multi_resource_import_only_no_collapse — hand-built
    ParsedComponent with two distinct [resource-rep] and two
    [resource-new] imports; asserts 4 distinct entries in by_name,
    and that x and y resolve to different module/field tuples.
  • test_issue_99_single_resource_import_only_keyed_by_name
    single-resource case keys by the actual resource name, not by an
    empty-string sentinel.
  • test_issue_99_no_resources_produces_empty_map — empty fallback
    case is genuinely empty.
  • test_issue_99_get_by_name_disambiguates_resources — direct
    get_by_name on two resources from different interfaces returns
    the correct per-resource entries.

🤖 Generated with Claude Code

Replace the `(0u32, prefix)` sentinel slot in
`build_resource_type_to_import` with a per-resource name-keyed fallback
index. The previous design collapsed every `[resource-rep]` /
`[resource-new]` import onto a single map slot whenever a component
imported resources without canonical entries — silently overwriting
all but the first when more than one resource was imported.

Changes:

- New `ResourceImportMap` struct with two indices: `by_type_id`
  (canonical type-id → import) and `by_name` (resource short-name →
  import). The canonical lookup path is unchanged; the import-only
  fallback now lives in `by_name`, keyed per-resource.
- `resolve_resource_positions` uses the new combined `resolve()`
  helper, which tries the type-id index first then translates the
  type id to a resource short-name (via `ParsedComponent::
  resolve_resource_type`) for the name fallback. The legacy
  `used_sentinel` gate is gone — its job (marking imported-resource
  matches as "callee does not define") is now driven by whether the
  match came from the name fallback.
- `resolve_inner_resource_imports` / `resolve_one_layout` updated to
  take the parsed component for the same name-fallback path.
- All call sites in `resolver.rs` updated to pass the component
  alongside the resource map.
- Four new unit tests pin down the multi-resource case, the
  single-resource case (now keyed by name, not by sentinel), the
  empty case, and the per-name disambiguation property.

Test results: 181 lib unit tests pass (was 177 + 4 new); 73
`wit_bindgen_runtime` tests stay green; clippy `-D warnings` clean.

Closes #99.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@avrabe avrabe marked this pull request as ready for review April 28, 2026 17:47
@avrabe avrabe merged commit 991ed03 into main Apr 28, 2026
4 checks passed
@avrabe avrabe deleted the fix/issue-99-resource-fallback-sentinel branch April 28, 2026 17:55
avrabe added a commit that referenced this pull request Apr 29, 2026
V&V-hardening minor release. Closes 5 issues:

- #98 — semver-correct compare_version (PR #113)
- #99 — resource-import fallback name-keyed, sentinel eliminated (PR #115)
- #102 — Kani + proptest harnesses for parser/merger/resolver (PR #116)
- #104 — cargo-fuzz scaffolding with 4 targets (PR #114)
- #112 — Mythos v0.4 follow-up: items 4, 5, 6 confirmed and fixed (PR #117)

New approved STPA loss scenarios: LS-R-10, LS-CP-3.

Pre-release Mythos delta pass (tier-5 + tier-4 changed since v0.3.0):
merger.rs and resolver.rs scanned; **no confirmed findings**.

The cargo-fuzz scaffolding immediately surfaced a real parser panic on
truncated component-section input — tracked as #118 for v0.4.1 / v0.5.

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Design: resource-fallback sentinel in build_resource_type_to_import

1 participant