refactor: split Android FrameworkTest.kt and document Pipeline DSL#19
Conversation
- Split 662-line FrameworkTest.kt into 6 focused test suites: - CoreTests.kt: PromptTemplate, OutputParser, Schema tests - ChainsTests.kt: all 7 built-in chain tests - DSLTests.kt: Pipeline DSL and model extension tests - ComposableTests.kt: Memory, Guardrail, ChainExecutor, SequentialChain tests - ErrorHandlingTests.kt: LocanaraException property and propagation tests - RAGTests.kt: new DocumentChunker and ChunkingConfig tests (15 cases) - TestHelpers.kt: shared MockModel and failingModel helpers - Add Pipeline DSL section to README.md with cross-platform code examples covering Swift declarative builder and Kotlin fluent API with step table Closes #13, #14
📝 WalkthroughWalkthroughRefactors Android tests by splitting the monolithic Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
… by split test files
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request focuses on improving the testability and documentation of the Locanara Android SDK. It refactors the existing test suite into smaller, more focused units and introduces comprehensive documentation for the Pipeline DSL, complete with cross-platform examples. Additionally, it expands the test coverage for the RAG (Retrieval-Augmented Generation) components. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
packages/android/locanara/src/test/kotlin/com/locanara/RAGTests.kt (2)
47-50: Add missing validation test for negativeminChunkSize.
ChunkingConfigvalidation tests cover most invalid inputs, butminChunkSize < 0is not explicitly tested (Line 47-Line 50 area currently ends validation cases early). Adding it will complete constructor guard coverage.Proposed diff
`@Test`(expected = IllegalArgumentException::class) fun `chunkOverlap greater than targetChunkSize throws`() { ChunkingConfig(targetChunkSize = 100, chunkOverlap = 150) } + + `@Test`(expected = IllegalArgumentException::class) + fun `negative minChunkSize throws`() { + ChunkingConfig(targetChunkSize = 100, chunkOverlap = 10, minChunkSize = -1) + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/android/locanara/src/test/kotlin/com/locanara/RAGTests.kt` around lines 47 - 50, Add a unit test that asserts ChunkingConfig throws IllegalArgumentException when minChunkSize is negative: create a test method (e.g., `minChunkSizeNegativeThrows`) that constructs ChunkingConfig with minChunkSize < 0 and expects IllegalArgumentException, similar to the existing `chunkOverlap greater than targetChunkSize throws` test, so the constructor guard for minChunkSize is covered; reference the ChunkingConfig class and add the test to RAGTests.kt alongside the other validation tests.
122-141: Test name and assertions are misaligned for sentence-boundary behavior.The test says “splits on sentences,” but current assertions only check non-empty chunks and a very loose size cap. It doesn’t verify actual sentence-boundary behavior.
Proposed diff
- fun `sentence-respecting chunker splits on sentences`() { + fun `sentence-respecting chunker preserves sentence boundaries`() { @@ - assertTrue(chunks.isNotEmpty()) - // Each chunk should not dramatically exceed the targetChunkSize + assertTrue("Expected more than one chunk for this input", chunks.size > 1) + // Each chunk should stay reasonably bounded and end on sentence punctuation chunks.forEach { chunk -> + val trimmed = chunk.content.trim() assertTrue( "Chunk length ${chunk.content.length} should be reasonable", - chunk.content.length <= 200 // generous bound + chunk.content.length <= 120 ) + assertTrue( + "Chunk should end at a sentence boundary", + trimmed.isNotEmpty() && trimmed.last() in listOf('.', '!', '?') + ) } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/android/locanara/src/test/kotlin/com/locanara/RAGTests.kt` around lines 122 - 141, The test `sentence-respecting chunker splits on sentences` doesn't verify sentence-boundary behavior: update the test that uses `DocumentChunker(ChunkingConfig(..., respectSentences = true))` and `sentenceChunker.chunk(text)` to explicitly assert sentences are not split across chunks by (1) splitting `text` into expected sentences (e.g., by ". " or a simple sentence splitter), (2) asserting each expected sentence appears entirely inside a single `chunk.content` (no sentence fragment spans two chunks), and (3) optionally asserting chunk boundaries align with sentence boundaries (e.g., chunks end with sentence terminators) and that chunk count is <= number of sentences; rename the test to reflect the behavior if needed and replace the loose size-only assertions with these sentence-containment checks.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/android/locanara/src/test/kotlin/com/locanara/CoreTests.kt`:
- Around line 48-53: The test "auto detection" currently only asserts formatted
output but doesn't verify that PromptTemplate.from detected placeholders; update
the test to also assert that the created PromptTemplate's inputVariables (or
equivalent property on PromptTemplate) contains exactly "name" and "place"
(e.g., assertEquals(setOf("name","place"), template.inputVariables) or the
appropriate collection type), so the test fails if from() stops populating
detected variables.
---
Nitpick comments:
In `@packages/android/locanara/src/test/kotlin/com/locanara/RAGTests.kt`:
- Around line 47-50: Add a unit test that asserts ChunkingConfig throws
IllegalArgumentException when minChunkSize is negative: create a test method
(e.g., `minChunkSizeNegativeThrows`) that constructs ChunkingConfig with
minChunkSize < 0 and expects IllegalArgumentException, similar to the existing
`chunkOverlap greater than targetChunkSize throws` test, so the constructor
guard for minChunkSize is covered; reference the ChunkingConfig class and add
the test to RAGTests.kt alongside the other validation tests.
- Around line 122-141: The test `sentence-respecting chunker splits on
sentences` doesn't verify sentence-boundary behavior: update the test that uses
`DocumentChunker(ChunkingConfig(..., respectSentences = true))` and
`sentenceChunker.chunk(text)` to explicitly assert sentences are not split
across chunks by (1) splitting `text` into expected sentences (e.g., by ". " or
a simple sentence splitter), (2) asserting each expected sentence appears
entirely inside a single `chunk.content` (no sentence fragment spans two
chunks), and (3) optionally asserting chunk boundaries align with sentence
boundaries (e.g., chunks end with sentence terminators) and that chunk count is
<= number of sentences; rename the test to reflect the behavior if needed and
replace the loose size-only assertions with these sentence-containment checks.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 48b436f8-583b-4679-a735-21f6e0f33dd4
📒 Files selected for processing (9)
README.mdpackages/android/locanara/src/test/kotlin/com/locanara/ChainsTests.ktpackages/android/locanara/src/test/kotlin/com/locanara/ComposableTests.ktpackages/android/locanara/src/test/kotlin/com/locanara/CoreTests.ktpackages/android/locanara/src/test/kotlin/com/locanara/DSLTests.ktpackages/android/locanara/src/test/kotlin/com/locanara/ErrorHandlingTests.ktpackages/android/locanara/src/test/kotlin/com/locanara/FrameworkTest.ktpackages/android/locanara/src/test/kotlin/com/locanara/RAGTests.ktpackages/android/locanara/src/test/kotlin/com/locanara/TestHelpers.kt
💤 Files with no reviewable changes (1)
- packages/android/locanara/src/test/kotlin/com/locanara/FrameworkTest.kt
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request is a great improvement to the project's structure and documentation. Splitting the monolithic FrameworkTest.kt into focused test suites significantly enhances maintainability and readability. The addition of new tests for RAG and error handling is also a valuable contribution. The new documentation for the Pipeline DSL in the README is clear and will be very helpful for users.
I've left a few minor suggestions, mostly related to improving documentation clarity for Kotlin code examples and adhering to Kotlin best practices by removing unnecessary non-null assertions (!!) in the new test files.
- Add inputVariables assertion to auto detection test - Replace assertNotNull + !! with requireNotNull for smart cast - Use safe call operator for nullable metadata access
There was a problem hiding this comment.
Pull request overview
This PR restructures the Android test suite into smaller, focused test files and expands JVM-only RAG coverage, while also adding README documentation for the cross-platform Pipeline DSL.
Changes:
- Split the former monolithic
FrameworkTest.ktinto multiple focused Kotlin test files and extracted shared test helpers. - Added new JVM tests for
DocumentChunker/ChunkingConfig(RAG layer) and adjusted chunking overlap advancement logic. - Added a new “Pipeline DSL” section to
README.mdwith Swift/Kotlin examples and a steps reference table.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/android/locanara/src/test/kotlin/com/locanara/TestHelpers.kt | Adds shared MockModel + failingModel helpers for reuse across test suites. |
| packages/android/locanara/src/test/kotlin/com/locanara/CoreTests.kt | Core-layer tests (templates/parsers/schema typing) moved into a focused file. |
| packages/android/locanara/src/test/kotlin/com/locanara/ChainsTests.kt | Built-in chain tests split into a dedicated suite. |
| packages/android/locanara/src/test/kotlin/com/locanara/DSLTests.kt | Pipeline DSL and model extension tests moved into their own suite. |
| packages/android/locanara/src/test/kotlin/com/locanara/ComposableTests.kt | Composable/memory/guardrail/executor/sequential chain tests split out. |
| packages/android/locanara/src/test/kotlin/com/locanara/ErrorHandlingTests.kt | LocanaraException property tests + chain error propagation tests moved out. |
| packages/android/locanara/src/test/kotlin/com/locanara/RAGTests.kt | Adds JVM tests for chunking config + document chunking behavior. |
| packages/android/locanara/src/main/kotlin/com/locanara/rag/DocumentChunker.kt | Fixes character-chunking overlap advancement to use target size for stepping. |
| packages/android/locanara/src/test/kotlin/com/locanara/FrameworkTest.kt | Removes the previous monolithic test file. |
| README.md | Adds Pipeline DSL docs + examples + reference table. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
🧹 Nitpick comments (2)
packages/android/locanara/src/test/kotlin/com/locanara/RAGTests.kt (2)
17-51: Good validation coverage, but missing test for negativeminChunkSize.The
ChunkingConfigconstructor validates thatminChunkSize >= 0(throwsIllegalArgumentException), but there's no test covering this case.Add test for negative minChunkSize validation
+ `@Test`(expected = IllegalArgumentException::class) + fun `negative minChunkSize throws`() { + ChunkingConfig(targetChunkSize = 512, minChunkSize = -1) + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/android/locanara/src/test/kotlin/com/locanara/RAGTests.kt` around lines 17 - 51, Add a unit test to ChunkingConfigTest that asserts the ChunkingConfig constructor throws IllegalArgumentException when minChunkSize is negative; create a new `@Test` annotated method (e.g., `negative minChunkSize throws`) that calls ChunkingConfig(targetChunkSize = 512, minChunkSize = -1) and uses `@Test`(expected = IllegalArgumentException::class) to validate the behavior, matching the style of the other validation tests.
111-119: Consider adding explicit test for consistent overlap behavior.Since the PR includes a bug fix for overlap progression (line 221 in
DocumentChunker.kt), adding a test that explicitly verifies chunks overlap by the configuredchunkOverlapamount would help catch future regressions.Suggested test for overlap verification
`@Test` fun `chunks have consistent overlap`() { val config = ChunkingConfig( targetChunkSize = 100, chunkOverlap = 20, respectSentences = false, minChunkSize = 10 ) val testChunker = DocumentChunker(config) val text = "A".repeat(300) val chunks = testChunker.chunk(text) // Move distance should be consistent: targetChunkSize - chunkOverlap = 80 for (i in 1 until chunks.size) { val moveDistance = chunks[i].startOffset - chunks[i - 1].startOffset assertEquals( "Move distance should be targetChunkSize - chunkOverlap", config.targetChunkSize - config.chunkOverlap, moveDistance ) } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/android/locanara/src/test/kotlin/com/locanara/RAGTests.kt` around lines 111 - 119, Add a new unit test that verifies overlap progression is consistent with the configured chunkOverlap by instantiating DocumentChunker with a ChunkingConfig (e.g., targetChunkSize and chunkOverlap set) and calling chunk(text), then assert for each adjacent pair of chunks that (chunks[i].startOffset - chunks[i-1].startOffset) == (config.targetChunkSize - config.chunkOverlap); reference DocumentChunker, ChunkingConfig and the chunk(...) method and name the test to indicate it checks "chunks have consistent overlap".
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/android/locanara/src/test/kotlin/com/locanara/RAGTests.kt`:
- Around line 17-51: Add a unit test to ChunkingConfigTest that asserts the
ChunkingConfig constructor throws IllegalArgumentException when minChunkSize is
negative; create a new `@Test` annotated method (e.g., `negative minChunkSize
throws`) that calls ChunkingConfig(targetChunkSize = 512, minChunkSize = -1) and
uses `@Test`(expected = IllegalArgumentException::class) to validate the behavior,
matching the style of the other validation tests.
- Around line 111-119: Add a new unit test that verifies overlap progression is
consistent with the configured chunkOverlap by instantiating DocumentChunker
with a ChunkingConfig (e.g., targetChunkSize and chunkOverlap set) and calling
chunk(text), then assert for each adjacent pair of chunks that
(chunks[i].startOffset - chunks[i-1].startOffset) == (config.targetChunkSize -
config.chunkOverlap); reference DocumentChunker, ChunkingConfig and the
chunk(...) method and name the test to indicate it checks "chunks have
consistent overlap".
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 51ef81e7-6e45-411c-820d-d3e93c364dfd
📒 Files selected for processing (5)
README.mdpackages/android/locanara/src/main/kotlin/com/locanara/rag/DocumentChunker.ktpackages/android/locanara/src/test/kotlin/com/locanara/CoreTests.ktpackages/android/locanara/src/test/kotlin/com/locanara/ErrorHandlingTests.ktpackages/android/locanara/src/test/kotlin/com/locanara/RAGTests.kt
✅ Files skipped from review due to trivial changes (2)
- README.md
- packages/android/locanara/src/test/kotlin/com/locanara/CoreTests.kt
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/android/locanara/src/test/kotlin/com/locanara/ErrorHandlingTests.kt
Summary
Closes #13, #14
Issue #13: Split Android FrameworkTest.kt into focused test suites
The monolithic
FrameworkTest.kt(662 lines) has been split into six focused test files, each covering a distinct layer of the framework:TestHelpers.kt— SharedMockModelandfailingModelhelpers extracted for reuseCoreTests.kt—PromptTemplateTest,OutputParserTest,SchemaTestChainsTests.kt— All 7 built-in chain tests (Summarize, Classify, Translate, Rewrite, Proofread, Chat, Extract)DSLTests.kt—PipelineTest,ModelExtensionTestComposableTests.kt—MemoryTest,GuardrailTest,ChainExecutorTest,SequentialChainTestErrorHandlingTests.kt—LocanaraExceptionproperty tests + chain propagation testsRAGTests.kt— 15 new tests forDocumentChunkerandChunkingConfig(pure-JVM, no Android context needed)The original
FrameworkTest.kthas been removed.Issue #14: Document Pipeline DSL with cross-platform code examples
Added a Pipeline DSL section to
README.mdbetween the Runtime Layer overview and the Packages section, covering:@PipelineBuilderin Swift)Test plan
./gradlew :locanara:testpasses with the new test file structureRAGTests(DocumentChunkerTest,ChunkingConfigTest) all pass on JVMSummary by CodeRabbit
Documentation
Bug Fixes
Tests