Skip to content

fix(boundary): support reference impl owners without graph collisions - #159

Merged
randlee merged 1 commit into
chore/orchestration-upstream-alignmentfrom
fix/reference-impl-owner
Sep 20, 2026
Merged

randlee merged 1 commit into
chore/orchestration-upstream-alignmentfrom
fix/reference-impl-owner

Conversation

@randlee

@randlee randlee commented Sep 19, 2026

Copy link
Copy Markdown
Owner

Valid Rust trait implementations such as impl<T: ?Sized + serde::Serialize> SerializeKindTag for &FieldValue<'_, T> {} abort boundary analysis with “only path owners are supported.” This blocks sc-observability's nonpublishing release preflight (PHC-QA-019).

Resolve reference, parenthesized, and grouped self types to their underlying path owner. Reference implementations and their methods get distinct graph identities, so owned, shared, mutable, and nested reference implementations coexist without merging nodes. Existing path-owner IDs remain unchanged. Cycle analysis continues attributing reference methods to the underlying type.

Validation:

  • cargo test -p sc-lint-boundary: 88 passed, including the exact generic FieldValue syntax, owned/shared/mutable/nested coexistence, transparent parentheses/groups, preserved path IDs, and cycle attribution.
  • Required aggregate gates just lint and just test: passed.
  • git diff --check: passed.
  • Built corrected CLI and sibling backend ran against unchanged sc-observability PR185 source 7246f6796a52ecb2d026d735426ae726968a1eb6. Parsing completes, scanning nine crates. The report still has status fail with 41 findings: 22 SCB-MANIFEST-001, one SCB-MANIFEST-002, six SCB-CYCLE-001, two SCB-CYCLE-002, and ten SCB-CYCLE-003. Successful CLI execution is not a clean lint result; these consumer diagnostics are outside this fix.

The change is limited to four boundary analyzer/test files. No consumer source/configuration changes, lint waivers, tags, releases, or publication. Other unnamed self-type shapes still produce explicit errors. Beads: lint-2va; review pending. Draft for independent QA, targeting develop.

@randlee

randlee commented Sep 19, 2026

Copy link
Copy Markdown
Owner Author

Final Quality Report

Generated: 2026-09-19T04:41:04Z
QA Pass: 1
Sprint/Task: phase-c-lint-parser-fix / phase-c-lint-parser-fix-qa
Branch: fix/reference-impl-owner
Commit: ba2d9bf622c1604e3f017c728040906b90e71bce
PR: #159
Final Verdict: PASS

Machine Status (JSON)

{
  "sprint": "phase-c-lint-parser-fix",
  "task": "phase-c-lint-parser-fix-qa",
  "branch": "fix/reference-impl-owner",
  "commit": "ba2d9bf622c1604e3f017c728040906b90e71bce",
  "pr": 159,
  "verdict": "PASS",
  "findings": {
    "blocking": 0,
    "important": 0,
    "minor": 1
  },
  "blocking_ids": [],
  "merge_readiness": "ready",
  "merge_reason": "All 8 review targets independently verified: structural correctness (arch-qa, code read), requirements/deliverable presence (req-qa, static analysis - its 4 execution-dependent items were separately confirmed directly by quality-mgr and rust-qa-agent, since req-qa's toolset has no command-execution capability), and full execution-based verification (rust-qa-agent: fmt/clippy/tests/just lint/just test all pass, 88/88 tests, CLI-against-consumer triage). Zero blocking or important findings. PR159 CI green (12/12) at the exact reviewed head.",
  "next_action": "none",
  "owner": "none",
  "recommendation": "Merge-ready from a QA standpoint. This is a supporting upstream fix for sc-observability's PHC-QA-019 backend-parser sub-cause only; consumer adoption (sc-lint version pin bump in sc-observability's release-preflight) is a separate, still-open step before PHC-QA-019 can be closed on the consumer side."
}

Validated Scope

  • Reference/paren/group self-type owner resolution (crates/sc-lint-boundary/src/graph/mod.rs:152-183): impl_owner() now recurses through Type::Reference/Type::Paren/Type::Group to the underlying Type::Path. Confirmed by direct code read (quality-mgr, arch-qa) and by directly executing the regression test (quality-mgr, rust-qa-agent).
  • No graph-node collisions between owned/shared/mutable/nested reference impls (graph/build.rs:490-521,603-611): reference impls get a lossless self_type-derived id suffix; non-reference ids are byte-for-byte unchanged. Confirmed via reference_impls_and_methods_do_not_collide_with_owned_impls test (arch-qa, code read; quality-mgr/rust-qa-agent, execution).
  • Cycle analysis attributes reference-impl methods to the real path-owner, never a phantom reference node (analysis.rs:520-539, build.rs:630-634). Confirmed via reference_method_cycles_keep_the_underlying_type_owner test.
  • Reproduction of the original failure: quality-mgr independently reproduced the exact pre-fix error in a scratch clone at parent commit 71fa79633b984f526b91f1680e03e5c437f77f53 (unsupported impl owner type \& FieldValue < '_ , T >`; only path owners are supported`), then confirmed the fix commit resolves it. rust-qa-agent independently reproduced the same thing.
  • Full test suite at the frozen commit: cargo test -p sc-lint-boundary -> 88 passed, 0 failed (quality-mgr and rust-qa-agent both ran this directly and got identical results, matching the PR's own claimed count).
  • Aggregate gate compliance: just lint && just test per upstream AGENTS.md (not sc-observability's just validate) - PASS (rust-qa-agent, executed directly).
  • CLI run against unchanged sc-observability PR185 consumer source at 7246f6796a52ecb2d026d735426ae726968a1eb6: ok:true, no CLI.BACKEND_EXEC_FAILURE, 41 findings reported (22 SCB-MANIFEST-001, 1 SCB-MANIFEST-002, 6 SCB-CYCLE-001, 2 SCB-CYCLE-002, 10 SCB-CYCLE-003). rust-qa-agent independently triaged: none reference FieldValue/SerializeKindTag/callsite (confirming the target case is cleanly resolved with no residual false positives there); spot-checked 2 of the 41 against the actual consumer source and confirmed both are real, applicable findings, not noise from this fix. CLI ok:true correctly not treated as a clean-lint result.
  • Change-scope discipline: git diff 71fa796..ba2d9bf --stat confirmed by quality-mgr directly - exactly the four named files (analysis.rs, graph/build.rs, graph/mod.rs, tests.rs), 233 insertions/18 deletions, nothing else. No consumer source/config changes, lint waivers, tags, releases, or publication.
  • PR159 CI: run 35421139810 at exact head ba2d9bf6 independently confirmed completed/success, all 12 jobs pass (quality-mgr, gh run view).

Findings Summary (Final)

  • Blocking: 0
  • Important: 0
  • Minor: 1

Residual Risks

  • ARCH-001 (Minor, informational - arch-qa): this commit's PHC-QA-019 linkage is a traceability nuance, not a defect - PHC-QA-019 has two distinct sub-causes (repo-root discovery CLI.CONFIG_ERROR, already fixed separately via the boundaries/ inventory on PR185; and this commit's CLI.BACKEND_EXEC_FAILURE/reference-impl-owner parser crash). This commit fixes only the second sub-cause. Do not treat PHC-QA-019 as fully closed by this commit alone - it remains open pending sc-lint publishing a release that incorporates this fix and sc-observability adopting that release in its preflight pin.
  • 2 of the 41 consumer lint diagnostics against PR185 (SCB-MANIFEST-001/002) were spot-checked as real, applicable findings unrelated to this fix - out of scope for this review, noted for separate triage.

Merge Readiness

  • Status: ready
  • Reason: All 8 review targets independently verified: structural correctness (arch-qa, code read), requirements/deliverable presence (req-qa, static analysis - its 4 execution-dependent items were separately confirmed directly by quality-mgr and rust-qa-agent, since req-qa's toolset has no command-execution capability), and full execution-based verification (rust-qa-agent: fmt/clippy/tests/just lint/just test all pass, 88/88 tests, CLI-against-consumer triage). Zero blocking or important findings. PR159 CI green (12/12) at the exact reviewed head.

Recommendation

Merge-ready from a QA standpoint. This is a supporting upstream fix for sc-observability's PHC-QA-019 backend-parser sub-cause only; consumer adoption (sc-lint version pin bump in sc-observability's release-preflight) is a separate, still-open step before PHC-QA-019 can be closed on the consumer side.

@randlee
randlee marked this pull request as ready for review September 19, 2026 17:42
@randlee
randlee added this pull request to stack #165 September 19, 2026 20:09
@randlee
randlee force-pushed the fix/reference-impl-owner branch from ba2d9bf to 0887a43 Compare September 20, 2026 17:47
@randlee
randlee requested a review from rand-lee September 20, 2026 17:53
@rand-lee
rand-lee force-pushed the fix/reference-impl-owner branch from 0887a43 to 235cac9 Compare September 20, 2026 18:27
@randlee
randlee force-pushed the fix/reference-impl-owner branch from 235cac9 to a24d36a Compare September 20, 2026 18:42
@randlee
randlee removed this pull request from stack #165 September 20, 2026 18:42
@randlee
randlee changed the base branch from develop to chore/orchestration-upstream-alignment September 20, 2026 18:43
@randlee
randlee added this pull request to stack #172 September 20, 2026 18:43
@randlee
randlee force-pushed the fix/reference-impl-owner branch from a24d36a to 74bfd41 Compare September 20, 2026 18:48
@randlee
randlee removed this pull request from stack #172 September 20, 2026 20:19
@randlee
randlee added this pull request to stack #183 September 20, 2026 20:19
@randlee
randlee merged commit e2477c3 into develop Sep 20, 2026
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant