[libgraphql-core-v1] Task 16.6e: Hygiene, from_str family, Schema::resolve_span

[jeffmo]

Summary

Task 16.6e (fifth of six schema-hardening PRs): the hygiene sweep + the two small public APIs the Task 16.5 audit found claimed-but-missing.

What & why

• SchemaBuilder::from_str — named by the API-Disposition ledger but never implemented (only load_str/build_from_str existed). Inherent method mirroring the other string entry points (FromStr deliberately avoided — it would force a trait import for a builder convenience); build_from_str now delegates through it. Error type mirrors load_str's load-phase shape, documented on the method.
• Source labels for multi-source schemas — v1 hardcoded SchemaSourceMap::from_source(source, None), so diagnostics from multi-document schemas couldn't name their source (a silent v0 regression: v0's load_str took Option<&Path>). New from_str_with_label / load_str_with_label / build_from_str_with_label variants (impl AsRef<Path>) thread the label into SchemaSourceMap.file_path; unlabeled entry points keep their clean signatures; all share a private load_str_impl. Task 19's operation builders copy this shape.
• Schema::resolve_span(Span) -> Option<LineCol> — AD18 lists it as public API but nothing implemented it. Indexes the schema's source maps by the span's source_map_id (None out of range); rustdoc states the schema-originated-span-only domain per AD18's cross-artifact rule, the 0-based LineCol semantics, the byte-offset column approximation (source maps store line_starts only), and builtin→0:0.
• Hygiene: stale "build() is currently todo!()" rustdoc in validators/mod.rs replaced with accurate module docs; the completed TODO(Task 16) in union_type_validator.rs removed; the dead #[allow(dead_code)] mutation_type_name() builder accessor deleted (the field remains; build() reads it directly — removal verified reference-free).
• schema_def test coverage — the typed query API had zero dedicated tests; new schema_def_tests.rs covers typed lookups (incl. kind-mismatch → None), typed iterators, types_implementing(), root-op accessors, and resolve_span with hand-computed positions on a multi-line schema.
• Zeroed 9 pre-existing cargo doc warnings (broken/wrong-crate intra-doc links, private-item links) — semantic fixes, not silencing.

Tests

17 new tests + 2 running doctests (v1 crate 295 → 310 + 10 doctests). Highlights: two-source label-threading test asserting per-source file_path, per-document source_map_id, and cross-document line resolution; hand-verified resolve_span positions; out-of-range → None; builtin → 0:0.

Verification

• cargo test --workspace — 1417 green; cargo clippy --workspace --tests -- -Dwarnings — clean; cargo doc --no-deps -p libgraphql-core-v1 — 0 warnings
• Opus sub-agent review: no blockers; independently re-derived the resolve_span test's line_starts/position math, confirmed the build_from_str delegation is behaviorally identical on error paths, verified all three spot-checked doc-link fixes are semantic, and confirmed the label-threading test exercises real cross-document resolution. Its one worthwhile nit (error-type rationale in rustdoc) applied as the follow-up commit.
• Plan doc updated with [x] boxes + Completion Notes per the Execution Protocol

🤖 Generated with Claude Code

https://claude.ai/code/session_01TipQtpvuLuHjHmRsVEgwJw

---

Generated by Claude Code

[Copilot]

Pull request overview

Implements Task 16.6e for libgraphql-core-v1 by hardening schema-building ergonomics and diagnostics: adds the missing SchemaBuilder::from_str API family (including label-threading variants), implements Schema::resolve_span, and performs hygiene/doc fixes with expanded schema_def test coverage.

Changes:

• Added SchemaBuilder::from_str plus _with_label variants across the load_str / from_str / build_from_str family, threading labels into SchemaSourceMap::file_path.
• Implemented Schema::resolve_span(Span) -> Option<LineCol> using the schema’s stored source maps.
• Cleaned up stale docs/dead code and added new schema_def tests covering typed lookups/iterators, root-op accessors, and span resolution.

Reviewed changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
libgraphql-core-v1-plan.md	Marks Task 16.6e complete and documents the API/design outcomes.
crates/libgraphql-core-v1/src/validators/union_type_validator.rs	Updates validator docs to reflect current build wiring.
crates/libgraphql-core-v1/src/validators/mod.rs	Converts stale item docs to accurate module-level docs with correct link.
crates/libgraphql-core-v1/src/types/field_definition.rs	Adjusts rustdoc to avoid broken intra-doc link (but introduces internal-task wording).
crates/libgraphql-core-v1/src/span.rs	Fixes rustdoc link for ByteSpan.
crates/libgraphql-core-v1/src/schema/tests/schema_def_tests.rs	Adds dedicated typed schema API + resolve_span tests.
crates/libgraphql-core-v1/src/schema/tests/schema_builder_tests.rs	Adds tests for from_str and label threading across multi-source schemas.
crates/libgraphql-core-v1/src/schema/tests/mod.rs	Wires in the new schema_def_tests module.
crates/libgraphql-core-v1/src/schema/schema_def.rs	Adds Schema::resolve_span public API implementation + rustdoc.
crates/libgraphql-core-v1/src/schema/schema_builder.rs	Adds from_str and label variants; refactors shared load_str_impl; threads labels into SchemaSourceMap.
crates/libgraphql-core-v1/src/schema_source_map.rs	Clarifies docs and links Schema explicitly.
crates/libgraphql-core-v1/src/error_note.rs	Fixes intra-doc links and span type reference.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[jeffmo]

Why did we invent the term "label" (here and in other APIs) rather than just referring to this by what it actually is/represents? (e.g. source_path or path)? Would it be better to use source_path/path/something semantically closer to what it is?

[jeffmo]

You're right, and it's fixed in 2a1888c: *_with_label → *_with_source_path, with the parameter, private impl, docs, tests, and plan notes renamed to match. "Label" was trying to signal one nuance — the path needn't exist on disk (it's a diagnostic identifier: "<inline schema>", a URL, a test name all work) — but the parameter is typed impl AsRef<Path> and lands in a field literally named file_path, so the name was fighting both the type and the storage. That nuance now lives in the rustdoc instead of the name.

---

Generated by Claude Code

[jeffmo]

I think the answer is probably no, but I'm curious if there are any clever tricks we might employ to help ensure -- at the type level -- that a given Span is correctly associated with a given Schema? Not the end of the world if not (or if such a trick would impose overhead on the API and/or error message story or would otherwise have unworthy trade-offs), but I'd like you to think about it and tell me what you can come up with (if anything).

[jeffmo]

Thought about this a fair bit. There ARE tricks; I think none of the instance-level ones are worth their cost here, but there's a domain-level middle ground worth considering for Task 18. The menu:

1. Lifetime branding / generativity (the only true type-level instance guarantee). GhostCell-style: Schema construction introduces a fresh invariant lifetime 'brand (via a closure: SchemaBuilder::build_with(|schema: Schema<'brand>| ...)), spans it hands out become Span<'brand>, and resolve_span(Span<'brand>) only accepts same-brand spans. Two different Schemas can never exchange spans — the compiler proves it. Why I'd reject it here:

• It's viral. Every span-carrying type grows the brand: GraphQLType<'brand>, FieldDefinition<'brand>, every error type… the entire crate surface.
• Brands are closure-scoped. You can't store a branded Schema in a plain struct or return it up the stack without the brand infecting the container. That kills the "build a schema, keep it around, serve requests" usage pattern that is this crate's whole point.
• Fatal conflict with AD6/macro embedding: Schema must be 'static + serde for bincode embedding. Brands don't survive serialization — a deserialized Span<'brand> is meaningless, so the macro path can't exist under branding.

2. Per-schema runtime identity tag. Stamp each Schema with a unique id at build(), widen Span to carry it, resolve_span returns None on mismatch. Honest assessment: it only upgrades "wrong position" to "None", costs 4–8 bytes on a Copy type that's stored everywhere, and muddies serde/PartialEq determinism (bincode round-trip equality would need the id excluded or regenerated). Not worth it.

3. Domain-level newtypes (the one I'd actually consider). The realistic misuse class per AD18 isn't schema-A-vs-schema-B (one schema per process is overwhelmingly the norm) — it's schema-span vs operation-span, once Task 18 introduces operation documents whose spans use the same dense id space. That confusion CAN be caught at compile time cheaply: keep Span as the raw representation, but have operation-side types store/expose a DocSpan(Span) newtype (or a ZST-marker generic) so Schema::resolve_span simply won't accept an operation span and vice versa. Zero runtime cost, no serde impact (schema-side Span unchanged), and the churn lands entirely in operation types that don't exist yet — free if we decide before 18, expensive after.

Recommendation: keep resolve_span as-is for instance-level (the Option + documented span-domain contract), and if you like #3 I'll amend Task 18's spec to make operation spans a distinct type at API boundaries. Happy to do that as part of this PR's plan-doc or as a one-liner in the next one — say the word.

---

Generated by Claude Code