[slin1237]

What

Two fixes to grammar/schema.rs that the review of #2881 asked for after that PR had been merged (its last two threads, answered at a commit the merge did not include), inside crates/symphony/ only:

• The pointer walk keeps its own list of schemas to look at, and follows a chain to its end. pointers_resolve follows every pointer it resolves into the definition it names; cycle detection alone let a request's chain of distinct definitions (D0 → D1 → … → Dn) send a recursive walk one frame deeper per link, and each link's own nesting rode on the stack too, so a long or deep enough chain would exhaust the stack and abort the process. The walk now pops the schemas still to look at from a list of its own, with the memo of pointers followed kept (each distinct pointer is followed once, so a wide schema costs one look per definition), and nothing a request sends reaches the call stack. The chain test types its property so the outcome depends on the walk, and pins ten links, five thousand links, and sixty-four links each wrapped a hundred and twenty times in properties before their pointer, all followed to the integer at the end, while the same five-thousand-link and sixty-four-deep chains with their last definition missing come out typeless (only a walk that reaches the end finds the pointer at nothing); a wide-schema test with seventy leaf definitions stays pinned.
• A block the property has but that is no object ("$defs": null, "$defs": [], an SDK writing an empty map as null) resolves nothing, as attached_to leaves it in place with no entry at the pointer; before, the pointer fell through to the root and the engine got a schema it could not compile.
• A property's own definitions stand before the root's. The document the engine gets (attached_to) keeps the property's own $defs and definitions and fills in the root's, so a pointer is now resolved the same way (Definitions::within): a property that defines a name the root defines badly keeps its type, and a sibling that reaches the root's dangling definition pins nothing. a_propertys_own_definition_stands_before_the_roots pins both.

No grammar on main changes for any table in the test corpus; both cases are new shapes.

Gates

By exit code (.claude/symphony/gates.sh): fmt, clippy -D warnings, test (344 lib, 10 fixture, 7 contract, 5 README), doc, width, codespell, all 0.

Merge

Squash under rule 25 (rule 18 does not apply: nothing of Format's changes). Subject: the PR title plus (#<n>). Body, with the authors of the old Kimi K3 parser whose definitions walk this code descends from (rule 27, as on #2881):

Signed-off-by: Simo Lin <25425177+slin1237@users.noreply.github.com>
Co-authored-by: Chang Su <8605658+CatherineSue@users.noreply.github.com>
Co-authored-by: Keyang Ru <rukeyang@gmail.com>
Co-authored-by: key4ng <key4ng@users.noreply.github.com>
Co-authored-by: Chen Hongtao <56470055+chenht2022@users.noreply.github.com>
Co-authored-by: LightSeek Foundation <243258330+lightseek-bot@users.noreply.github.com>

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: Organization UI
• Review profile: CHILL
• Plan: Team
• Run ID: c9afda9f-375f-4589-a0f3-1075b5d86427

📥 Commits

Reviewing files that changed from the base of the PR and between c9ed9cd and b475a59.

📒 Files selected for processing (1)

• crates/symphony/src/grammar/xtml.rs

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 1 remain after this review.

---

📝 Summary

Summary by CodeRabbit

• Bug Fixes
  • Schema references now prioritize definitions within a property before falling back to root-level definitions.
  • References resolve across long chains and deeply nested schemas, with repeated pointers processed only once.
  • References remain unresolved when a property’s local definition block is not an object.
  • Schema type information remains available in wide schemas, regardless of the number of definitions, and unresolved references no longer incorrectly assign a type.

Walkthrough

Schema shaping now checks property-local definition blocks before root blocks. Pointer validation follows each distinct local reference once with an iterative walk. Tests cover definition precedence, invalid local blocks, long and nested reference chains, and wide schemas.

Changes

Schema reference resolution

Layer / File(s)	Summary
Property-scoped definition lookup crates/symphony/src/grammar/schema.rs, crates/symphony/src/grammar/xtml.rs	Definitions tracks property-local and root definition blocks separately. Resolution checks the matching local block before the root block. Attachment preserves property definitions while filling missing names from root definitions. Tests cover precedence and non-object local blocks.
Iterative reference validation crates/symphony/src/grammar/schema.rs, crates/symphony/src/grammar/xtml.rs	Pointer validation uses an iterative pending-list walk and a HashSet to follow each distinct local reference once. Tests cover chains of up to 5,000 links, deeply nested schemas, and schemas with 70 referenced definitions.

Priority: ⬇️ Low

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix

Merge Risk: ⚪ Minimal · up to b475a

No identified issue blocks merging; run the normal checks before merging.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 78.57% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 14 functions across 2 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Title check	✅ Passed	The title clearly identifies both main fixes: bounded pointer traversal and property-local definitions taking precedence over root definitions.
Description check	✅ Passed	The description directly explains the schema-resolution changes, affected files, tests, and reported validation gates.

• Fix all pre-merge checks with AI

✨ Finishing Touches 💡 1

📝 Generate docstrings 💡

• Commit to this branch
• Create a new PR

🧪 Generate unit tests (beta)

• Commit to this branch
• Create a new PR

• Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

---

_{Comment @coderabbitai help to get the list of available commands.}

[claude]

👋 The PR description doesn't fully follow
PULL_REQUEST_TEMPLATE.md:

• Missing header: ## Description
• Missing header: ### Problem
• Missing header: ### Solution
• Missing header: ## Changes
• Missing header: ## Test Plan

Please update the PR description so reviewers have the context they need.

[hello-alexmcc]

Rule-17 review of #2886 at 55f49d2: comment. I'll approve once the three open threads are answered; the first needs a code change.

1. The cap counts breadth as well as depth (schema.rs:156): agreed, and it will bite real schemas. followed is never popped, so 64 bounds the distinct pointers a property reaches in all. A Pydantic- or OpenAPI-derived tool schema, whose properties often name dozens of $defs, crosses it with no chain at all, and the property then loses its type pin. What the stack needs bounded is depth. A path stack does it: push on the way into a definition, pop on the way out; refuse past N levels, and treat a pointer already on the path as a cycle. A memo of the definitions already found to resolve keeps a shared definition from being walked again, so wide schemas stay linear.
2. A non-object $defs in the property (schema.rs:125): agreed. The check and attached_to should treat "$defs": null or [] the same way. Either both skip it for the root's entries, or both treat the pointer as unresolved, so the check never passes a document the engine can't compile.
3. The chain test (xtml.rs:480): agreed. With p typed itself, the short chain pins object and the long one pins nothing, so the test follows the walk's result rather than the one-hop rule. A wide-schema case (65 distinct leaf definitions, typed) would pin item 1.

What holds at this head (on main 3c430bd):

• Gates: fmt, clippy and doc exit 0; tests 332 lib, 10 fixture, 7 contract and 5 README. The 5,000-link chain no longer overflows.
• Dump: every table's grammar is identical to main's (180 grammars).
• Parity: GLM and DSML 36 of 36. Kimi K3 vs the old builder differs only by "type": "tag", plus the {"$ref": "#"} property already noted on #2881. Definitions-first resolution changes nothing my 14 tool sets reach.
• Trailers: six in the squash body, and no AI trailer.

[hello-alexmcc]

Rule-17 re-review of #2886 at 396be3e: comment. One more round; the new thread is right.

The three earlier threads are settled. The wide schema stays pinned, the chain test follows the walk, and a non-object own block resolves nothing. Gates exit 0 (334 lib), every table's grammar is identical to main's, and the Kimi K3 payloads are identical to 55f49d2's.

The new thread (schema.rs:176): agreed. depth counts pointer hops, but the walk also recurses through each definition's own JSON nesting between hops. serde_json caps a document at 128 levels, but that bounds where a definition sits, not the chain through definitions. So 64 hops of ~120 levels each give ~7,700 nested calls from a request's tool schema, past the 5,000-link chain the test treats as safe.

Either of two shapes closes it:

• One counter for every descent (into a value, an array item, or a pointer's target), capped, with anything past the cap treated as unresolved, so the property takes any value.
• An iterative walk with an explicit work stack, so nothing request-shaped sits on the call stack at all.

A test with the thread's shape (64 definitions, each nesting ~120 levels before its $ref) would pin it.

[hello-alexmcc]

Rule-17 re-review of #2886 at b475a59: approve. This round only touches the test, and all five threads are answered and resolved.

• The chain test now proves the walk reaches the end: chain(..., whole: false) leaves the last definition out. The 5,000-link chain and the 64 × 120 chain then come out typeless, beside the three whole chains that pin object. A walk that stopped early would fail it.
• From c9ed9cd: the iterative walk, the wide-schema pin and the non-object own block, verified there. No table's grammar changes, and the Kimi K3 payloads are identical to the earlier rounds'.
• Gates at b475a59: fmt, clippy and doc exit 0; tests 344 lib, 10 fixture, 7 contract and 5 README.
• Trailers: six in the squash body, and no AI trailer. The merge is the maintainer's.