Repository navigation
Conversation
🦋 Changeset detectedLatest commit: 146be7c The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Merge Risk: ⚪ Minimal · up to
|
| Check name | Status | Explanation |
|---|---|---|
| Title check |  |
The title clearly and concisely describes the primary change: preserving JSON format metadata in JsonChunker. |
| Description check | |
The description clearly explains the current defect, the new behavior, implementation changes, tests, changeset, validation limits, and scope. It does not use every template heading or provide a linke… |
| Docstring Coverage | |
No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 2… |
| Linked Issues check | |
Check skipped because no linked issues were found for this pull request. |
| Out of Scope Changes check | |
Check skipped because no linked issues were found for this pull request. |
Full details: Description check
-
Explanation
The description clearly explains the current defect, the new behavior, implementation changes, tests, changeset, validation limits, and scope. It does not use every template heading or provide a linked issue, but it contains the required information in equivalent sections.
Full details: Docstring Coverage
-
Explanation
No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 2 files. (1 skipped: 1 unsupported.)
✨ Finishing Touches
-
🧪 Generate unit tests (beta)
-
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Comment @coderabbitai help to get the list of available commands.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 3 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/rag/src/__tests__/json-chunker.spec.ts">
<violation number="1" location="packages/rag/src/__tests__/json-chunker.spec.ts:12">
P3: The new assertions cover `format` and `sourceType` but not the token position fields this PR says it preserves. Add a `tokenStart` assertion on the parsed-JSON chunk so a regression that drops it gets caught.</violation>
</file>
Reply with feedback, questions, or to request a fix.
View guided diff | Re-trigger cubic
| expect(chunks.length).toBeGreaterThan(0); | ||
| expect(chunks[0]?.metadata?.fields).toBeGreaterThan(0); | ||
| expect(chunks[0]?.metadata?.format).toBe("json"); | ||
| expect(chunks[0]?.metadata?.sourceType).toBe("json"); |
There was a problem hiding this comment.
P3: The new assertions cover format and sourceType but not the token position fields this PR says it preserves. Add a tokenStart assertion on the parsed-JSON chunk so a regression that drops it gets caught.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At packages/rag/src/__tests__/json-chunker.spec.ts, line 12:
<comment>The new assertions cover `format` and `sourceType` but not the token position fields this PR says it preserves. Add a `tokenStart` assertion on the parsed-JSON chunk so a regression that drops it gets caught.</comment>
<file context>
@@ -8,11 +8,15 @@ describe("JsonChunker", () => {
expect(chunks.length).toBeGreaterThan(0);
expect(chunks[0]?.metadata?.fields).toBeGreaterThan(0);
+ expect(chunks[0]?.metadata?.format).toBe("json");
+ expect(chunks[0]?.metadata?.sourceType).toBe("json");
});
</file context>
| expect(chunks[0]?.metadata?.sourceType).toBe("json"); | |
| expect(chunks[0]?.metadata?.sourceType).toBe("json"); | |
| expect(chunks[0]?.metadata?.tokenStart).toBe(0); |
This branch has not been deployed
1 participant
Summary
JsonChunkerfirst tokenizes its flattened JSON representation, then wraps the token chunks with JSON-specific metadata. The inherited token metadata containsformat: "text", andbuildMetadatamerges the inherited extras last, overwriting the intendedformat: "json". Valid JSON chunks therefore silently report the wrong format even thoughsourceTypeis JSON and structural fields are present.This PR removes the inherited
formatandsourceTypekeys before merging token chunk metadata. It retains token positions and JSON structural metadata, while invalid JSON still falls back to token chunking withformat: "text".Changes
TokenChunker'sformatandsourceTypefromJsonChunker's extras.@voltagent/rag.Reproduction and validation
mainsource at72a46c76b4507e1f98b41d5050a92bd30587eb70.JsonChunker,TokenChunker, andbuildMetadataimplementations, substituting only minimal tokenization/position helpers because the complete monorepo dependencies were unavailable.format: "text"; the targeted assertion fails.json/json) and invalid JSON fallback (text/token).git diff --check.Not run: official monorepo Vitest, package TypeScript build, lint, full CI, and integration tests. These still need real execution in the upstream/fork CI environment. The isolated harness is not a substitute for official tests.
Scope and prior work
Independent of #1459, which changes
detectFormatrouting for top-level arrays; this PR fixes metadata produced after a valid JSON payload has already reachedJsonChunker.AI use
Prepared with AI-assisted source review, duplication search, isolated reproduction, and patch verification. All validation limits are documented above.
Summary by cubic
Fixes
JsonChunkermetadata so valid JSON chunks reportformat: "json"andsourceType: "json"instead of inheriting"text"/"token"from the inner token chunker.formatandsourceTypebefore merging JSON metadata, while keeping token positions and structural fields.@voltagent/rag.Written for commit 146be7c. Summary will update on new commits.
Summary by CodeRabbit