Skip to content

[libgraphql-core-v1] Task 16.5: Plan audit & touch-up#103

Merged
jeffmo merged 5 commits into
mainfrom
lgcore_v1_task16.5
Jul 7, 2026
Merged

[libgraphql-core-v1] Task 16.5: Plan audit & touch-up#103
jeffmo merged 5 commits into
mainfrom
lgcore_v1_task16.5

Conversation

@jeffmo

@jeffmo jeffmo commented Jul 7, 2026

Copy link
Copy Markdown
Owner

Summary

Task 16.5 (meta-task): a two-agent audit of the v1 plan — completed Tasks 1–16 verified against the shipped code on main, and remaining Tasks 17–21 stress-tested for design defects — surfaced stale claims, two spec-design defects, unimplemented validations in "completed" work, silently-dropped v0 APIs, and entirely-untasked cutover work. This PR repairs libgraphql-core-v1-plan.md itself so every subsequent task executes against a corrected spec. Doc-only change — no code is touched.

Design corrections (new Architectural Decisions)

  • AD18 — Operation-side source maps. The prior Task 19 sketch passed SourceMapId(1) into OperationBuilder::from_ast, but Span.source_map_id indexes Schema.source_maps, which only holds schema documents and is immutable after build(). Operation spans would resolve against schema text or panic. New design: operation artifacts (FragmentRegistry, ExecutableDocument, Operation) own their documents' source maps; spans index the owning artifact; errors resolve spans eagerly at construction.
  • AD19 — Registry borrow lifetimes. ExecutableDocument<'schema> holding &'schema FragmentRegistry<'schema> cannot typecheck (the registry lives shorter than the schema, so a &'schema borrow of it is unobtainable). New shape: ExecutableDocument<'schema, 'reg> with a FragmentRegistryRef borrowed/owned enum — the Owned variant restores v0's combined fragments+operations single-document capability.

Plan repairs

  • Fixed stale claims: Task 2's completion note said doctests use ignore (they use a hidden use libgraphql_core_v1 as libgraphql_core; alias and DO run); Commit Strategy claimed 5 validators (type-ref validator was removed in Task 15); Task 16 body referenced schema/schema.rs (real file: schema_def.rs)
  • Synced File Structure with the actual tree (schema_def.rs, validators/edit_distance/, conversion_helpers.rs, into_graphql_type.rs, crate-root operation_kind.rs)
  • Corrected the Task 21 serde addendum: Span/ByteSpan/SchemaSourceMap/DirectiveLocationKind already derive serde — not a blocker; documented the real caveat (non-ASCII column resolution after deserialization) and specified the _macro_runtime public contract that emittable_schema.rs emits against

Goal reaffirmation + new tasks

  • Complete September 2025 spec validation is an explicit v1.0 goal — the checklist's "Deferred (future work)" bucket is gone. Those five validations are now scheduled, cutover-blocking Tasks 21.1–21.5 (value coercion §5.6.1; input-object literals §5.6.2–.4; variable validations §5.8.3–.5; fragment usage §5.5.1.4/§5.5.2.3; field-selection merging §5.3.2)
  • Validation Coverage Checklist is now maintained: ~25 implemented items verified against code and checked; the 4 gaps found in "completed" work annotated with owners (@oneOf §3.13.5 unimplemented; type extensions silently dropped with dead error kinds; IsValidImplementation 2.f TODO; build-level errors missing spec notes) → new Task 16.6 (schema hardening, 5 sub-PRs)
  • Added "v0 → v1 Public API Disposition": strings-only parity decision (all from_str entry points restored; file-I/O APIs deliberately dropped and documented), plus the facade ast glob re-export rationale
  • Task 19 split into 19a–19d (individually-reviewable PRs) with the v0 bug catalog (silent error-drop, duplicate-arg locations) as required regression tests
  • Added Tasks 22–27 covering the previously-untasked cutover: macros rewire, facade rewire, crate replacement (rename + 1.0.0, CI-script relocation FIRST), graphql-parser purge via feature-gated compat, docs, trackers

Verification

  • Doc-only; cargo untouched. Markdown code fences balanced; internal cross-references (AD numbers, task numbers, file paths) verified against origin/main:crates/libgraphql-core-v1
  • Pre-PR review pass ran against the diff; all findings addressed in the second commit (lifetime-arity fix in the 19c sketch, stale schema.rs refs, validator count, spurious §5.4.1.4 citation, libgraphql_core_v2 typos, unresolved-questions section added)

🤖 Generated with Claude Code

https://claude.ai/code/session_01TipQtpvuLuHjHmRsVEgwJw

Generated by Claude Code

claude added 2 commits July 7, 2026 07:11
@claude
[libgraphql-core-v1] Task 16.5: Plan audit & touch-up
08ece80 
Repairs this plan doc against the shipped Tasks 1-16 code and corrects
defects in the remaining task specs, per a two-agent audit:

- Fix stale claims (Task 2 doctest note; File Structure drift:
  schema_def.rs, edit_distance/, conversion_helpers.rs,
  into_graphql_type.rs, removed type_reference_validator.rs)
- Add AD18: operation-side source maps are owned by operation artifacts
  (the prior Task 19 sketch resolved operation spans against
  Schema.source_maps, which only holds schema documents)
- Add AD19: registry borrows use a second 'reg lifetime +
  FragmentRegistryRef owned/borrowed enum for combined documents (the
  prior single-'schema borrow could not typecheck)
- Rewrite Task 18/19 sketches accordingly; split Task 19 into 19a-19d
- Add "v0 -> v1 Public API Disposition" (strings-only parity decision;
  deliberately-dropped API list; facade ast glob re-export rationale)
- Correct Task 21 serde addendum (Span/ByteSpan/DirectiveLocationKind
  already derive serde) and specify the _macro_runtime public contract
- Reaffirm complete September 2025 spec validation as a v1.0 goal:
  Validation Coverage Checklist boxes verified against code and
  annotated with owning tasks; spec-section cross-check items added
- Add Task 16.6 (schema hardening: @OneOf, type extensions,
  IsValidImplementation 2.f, build-error spec notes, hygiene),
  Tasks 21.1-21.5 (spec-completeness validators; block cutover), and
  Tasks 22-27 (cutover: macros rewire, facade rewire, crate
  replacement, graphql-parser purge, docs, trackers)

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TipQtpvuLuHjHmRsVEgwJw
@claude
[libgraphql-core-v1] Task 16.5 review fixes
c1a864f 
Addresses pre-PR review findings:
- SelectionSetBuilder field in OperationBuilder sketch missing 'reg
  lifetime (would not compile as sketched)
- Task 16 body still referenced schema/schema.rs (file is schema_def.rs)
- Commit Strategy claimed 5 validators incl. removed type-ref validator
- Spurious spec citation (nonexistent section 5.4.1.4) in checklist
- libgraphql_core_v2 typos in doctest sketches (5 sites)
- Doctest-alias file count clarified (6 of 7 names/*.rs)
- mutation_type_name() hygiene item premise clarified (SchemaBuilder
  accessor, not the Schema one)
- Added closing Unresolved Questions section per tracker-doc convention

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TipQtpvuLuHjHmRsVEgwJw
@jeffmo jeffmo changed the title [libgraphql-core-v1] Task 16.5: Plan audit &amp; touch-up [libgraphql-core-v1] Task 16.5: Plan audit & touch-up Jul 7, 2026
@claude
[libgraphql-core-v1] Fix collapsible_match lint (new clippy toolchain)
426ebf1 
CI's clippy (1.96) now flags the empty-enum check's nested if under
-Dwarnings; collapse it into a match guard per clippy's suggestion.
Pre-existing on main (Task 16 code) — surfaced here because this is the
first PR since the toolchain moved. No behavior change; all 208 v1
tests pass.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TipQtpvuLuHjHmRsVEgwJw
jeffmo
jeffmo commented Jul 7, 2026
View reviewed changes
Comment thread libgraphql-core-v1-plan.md Outdated
Comment thread libgraphql-core-v1-plan.md Outdated
claude added 2 commits July 7, 2026 07:35
@claude
[libgraphql-core-v1] Address review: rename 'reg to 'fragreg; use Cow…
3fcc90b 
… for registry

- Rename the registry-borrow lifetime 'reg -> 'fragreg throughout
  (matches v0's existing convention for this exact lifetime)
- Replace the custom FragmentRegistryRef borrowed-or-owned enum with
  std::borrow::Cow<'fragreg, FragmentRegistry<'schema>> per review --
  same shape, plus Deref for free; documents the Clone-derive
  requirement this adds across the fragment/selection types

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TipQtpvuLuHjHmRsVEgwJw
@claude
[libgraphql-core-v1] Address fan-out review: fix stale claims, spec c…
0210bc9 
…ites, AD18 gaps

Applies the MUST-FIX findings (plus cheap checklist SHOULD-FIXes) from the
3-stage fan-out review of this PR (5 expert reviewers -> adversarial
critic -> arbiter):

- Correct the false "from_str (exist)" claim in the API Disposition ledger;
  SchemaBuilder::from_str is now a 16.6e work item (with the source-label
  parameter fix, so operation builders copy the right signature)
- Remove the false "Schema already has resolve_span" claim; Schema::
  resolve_span is now a 16.6e work item
- Fix four Sep-2025 section renumberings the checklist carried from
  Oct-2021 (5.4.3 required args, 5.2.4.1 single root field, 5.2.2.1
  operation-name uniqueness, 5.2.3.1 lone anonymous) and add the missing
  5.2.1.1 operation-root-type-exists row
- AD18: pin error-note scope (eager LineCol is operation-side only;
  schema errors keep the shipped deferred-Span ErrorNote), add the
  index-0 builtin seeding rule (Span::dummy() must never resolve to user
  text), and record the cross-artifact resolve_span misuse mitigation
  (doc-authoritative resolution with disjoint id spaces)
- Reconcile Task 18/19 sketches with AD18/AD19: Operation gains
  source_maps, OperationBuilder.source_map becomes Option, Selection-
  SetBuilder gains source-map access for eager error resolution, stale
  operation_kind.rs sketch removed (module exists at crate root),
  query_type() Option mismatch fixed, Clone derives added per AD19,
  Variable keeps its definition directives, set_fragment_registry added
  for the programmatic path
- Replace leaked session-internal P2.x labels with durable 16.6a-c ids
- New Task 16.6f owns the biggest coverage hole: SDL directive-
  application validation (5.7.1-.3 + argument rules) plus root-type
  distinctness (3.3.1), directive self-reference (3.13), and empty input
  objects; extend-schema handling added to 16.6b; @deprecated-on-required
  to 16.6c; oneOf-literal + IsNonNullPosition named in 21.1/21.3;
  5.5.1.4 scoped per-document; 5.1.1/5.5.1.2 rows and three verified
  [x] rows added so checklist absence is meaningful
- 19d: specify supplied-registry + in-document-fragments merge semantics
- 19a: require an explicit decision on __schema/__type meta-fields
- 19c: single-root-field respec'd as full CollectSubscriptionFields

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TipQtpvuLuHjHmRsVEgwJw
@jeffmo jeffmo merged commit cbaaa31 into main Jul 7, 2026
7 checks passed
@jeffmo jeffmo deleted the lgcore_v1_task16.5 branch July 7, 2026 22:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@jeffmo @claude