Refactor AI hints to use [[aihint:...]] format #362
Conversation
📝 WalkthroughWalkthroughThis PR renames the App-Provided Context feature from "ArbigentHint" to "AI Hints", updates APIs and data fields from Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used🧠 Learnings (1)📓 Common learnings🪛 markdownlint-cli2 (0.18.1)README.md92-92: Unordered list indentation (MD007, ul-indent) 93-93: Unordered list indentation (MD007, ul-indent) 106-106: Unordered list indentation (MD007, ul-indent) 107-107: Unordered list indentation (MD007, ul-indent) ⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (12)
🔇 Additional comments (1)
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[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 (1)
arbigent-core/src/main/java/io/github/takahirom/arbigent/ArbigentDevice.kt (1)
705-738: Parsing logic handles common cases, but nested]]in JSON arrays may truncate hints.The
indexOf(AI_HINT_SUFFIX, ...)approach correctly finds the first]]after the prefix, which fixes the original bug. However, if JSON content contains array syntax like[[aihint:["a","b"]]], the parser would extract["a","b"(truncating at the first]]).Consider whether this edge case needs handling, or if the documentation should note that
]]sequences within hint content should be avoided.🔎 Alternative: Find matching closing brackets
If you need to support
]]within hint content in the future, you could search for]]that is NOT preceded by another]:- val endIndex = text.indexOf(AI_HINT_SUFFIX, startIndex + AI_HINT_PREFIX.length) - if (endIndex > startIndex) { - val hint = text.substring(startIndex + AI_HINT_PREFIX.length, endIndex) + // Find ]] that closes the hint (last ]] in case of nested sequences) + val searchStart = startIndex + AI_HINT_PREFIX.length + var endIndex = text.lastIndexOf(AI_HINT_SUFFIX) + if (endIndex >= searchStart) { + val hint = text.substring(searchStart, endIndex)Or simply document that
]]should not appear in hint content.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
README.mdarbigent-ai-openai/src/main/java/io/github/takahirom/arbigent/OpenAIAi.ktarbigent-core-model/src/commonMain/kotlin/ArbigentResult.ktarbigent-core/src/main/java/io/github/takahirom/arbigent/ArbigentContextHolder.ktarbigent-core/src/main/java/io/github/takahirom/arbigent/ArbigentDevice.ktarbigent-core/src/main/java/io/github/takahirom/arbigent/UserPromptTemplate.ktarbigent-core/src/test/kotlin/io/github/takahirom/arbigent/TreeNodeExtensionsTest.ktarbigent-core/src/test/kotlin/io/github/takahirom/arbigent/UserPromptTemplateTest.kt
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: takahirom
Repo: takahirom/arbigent PR: 329
File: arbigent-core/src/main/java/io/github/takahirom/arbigent/ArbigentAgent.kt:0-0
Timestamp: 2025-07-08T15:34:42.649Z
Learning: takahirom prefers simplified implementations over complex caching libraries, favoring manual cache management through methods like clearResolvedGoalCache() rather than using bounded cache implementations with automatic eviction policies.
📚 Learning: 2025-07-02T09:57:37.159Z
Learnt from: takahirom
Repo: takahirom/arbigent PR: 318
File: arbigent-cli/src/main/kotlin/main.kt:27-28
Timestamp: 2025-07-02T09:57:37.159Z
Learning: In the arbigent project, YamlValueSource in arbigent-cli/src/main/kotlin/main.kt is accessible without requiring an explicit import statement, contrary to initial analysis suggesting a missing import.
Applied to files:
arbigent-core-model/src/commonMain/kotlin/ArbigentResult.kt
🪛 markdownlint-cli2 (0.18.1)
README.md
92-92: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
93-93: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
104-104: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (12)
- GitHub Check: cli-e2e-android (2, 2)
- GitHub Check: build
- GitHub Check: cli-e2e-ios
- GitHub Check: cli-e2e-android (1, 2)
- GitHub Check: test
- GitHub Check: build (macos-latest)
- GitHub Check: build (ubuntu-latest)
- GitHub Check: cli-e2e-web
- GitHub Check: build (windows-latest)
- GitHub Check: store-screenshot-test
- GitHub Check: compare-screenshot-test
- GitHub Check: Analyze (java-kotlin)
🔇 Additional comments (10)
arbigent-core-model/src/commonMain/kotlin/ArbigentResult.kt (1)
99-104: LGTM!The property rename from
appHintstoaiHintsaligns with the broader refactoring effort and maintains the same type and default value.arbigent-core/src/main/java/io/github/takahirom/arbigent/ArbigentContextHolder.kt (1)
96-111: LGTM!The parameter rename and updated call site properly propagate the
aiHintsnaming convention through the prompt construction chain.arbigent-core/src/main/java/io/github/takahirom/arbigent/UserPromptTemplate.kt (2)
14-18: LGTM!The constant and template placeholder renaming is consistent with the new AI hints convention.
88-101: LGTM!The
format()function properly handles the renamedaiHintsparameter and generates the<AI_HINTS>XML block correctly.arbigent-core/src/main/java/io/github/takahirom/arbigent/ArbigentDevice.kt (1)
275-276: LGTM!Correctly updated to use the renamed
findAllAiHints()function.arbigent-ai-openai/src/main/java/io/github/takahirom/arbigent/OpenAIAi.kt (2)
214-224: LGTM!The
aiHintsparameter is correctly extracted fromuiTreeStrings.aiHintsand passed tobuildPrompt.
339-358: LGTM!The
buildPromptfunction correctly propagatesaiHintsto the context holder's prompt method.arbigent-core/src/test/kotlin/io/github/takahirom/arbigent/UserPromptTemplateTest.kt (1)
115-167: LGTM!Test cases properly updated to reflect the renamed
aiHintsparameter and<AI_HINTS>XML tags. The position test ensures hints maintain correct ordering in the template output.arbigent-core/src/test/kotlin/io/github/takahirom/arbigent/TreeNodeExtensionsTest.kt (2)
7-112: LGTM!Existing tests properly updated to use
findAllAiHints()and the new[[aihint:...]]syntax.
114-211: Excellent edge case coverage!The new tests comprehensively verify:
- Hint coexistence with accessibility labels (line 117)
- JSON content handling (line 131)
- Single hint extraction per node behavior (line 145)
- Malformed hint robustness (unclosed, missing bracket, prefix-only)
- Empty hint content behavior
This test suite provides strong confidence in the parsing logic.
README.md
Outdated
| * **AI Hints - App-Provided Context:** | ||
| * Apps can provide domain-specific hints to help the AI understand the current screen better. | ||
| * Set `contentDescription` with `ArbigentHint:` prefix: | ||
| * Embed hints in `contentDescription` using the `[[aihint:...]]` format: | ||
| ```kotlin | ||
| view.contentDescription = "ArbigentHint:Video player, buffering" | ||
| // Hint only | ||
| view.contentDescription = "[[aihint:Video player, buffering]]" | ||
|
|
||
| // Coexisting with accessibility label | ||
| view.contentDescription = "Play button [[aihint:Video player, currently buffering]]" | ||
|
|
||
| // Structured data (JSON) | ||
| view.contentDescription = """[[aihint:{"screen":"player","state":"buffering"}]]""" | ||
| ``` | ||
| * The `[[...]]` format allows hints to coexist with accessibility labels and supports structured data like JSON. | ||
| * **Note:** Since this uses `contentDescription`, it affects screen readers. Enable hints only during testing via Intent flag or debug BuildConfig to avoid accessibility issues in production. |
There was a problem hiding this comment.
Fix markdown list indentation to align with style guidelines.
Lines 92–93 and 104 use 4-space indentation for nested list items, but the project's markdown style expects 2-space indentation (MD007).
🔎 Proposed fix for markdown indentation
* **AI Hints - App-Provided Context:**
- * Apps can provide domain-specific hints to help the AI understand the current screen better.
- * Embed hints in `contentDescription` using the `[[aihint:...]]` format:
+ * Apps can provide domain-specific hints to help the AI understand the current screen better.
+ * Embed hints in `contentDescription` using the `[[aihint:...]]` format:
```kotlin
- // Hint only
- view.contentDescription = "[[aihint:Video player, buffering]]"
+ * // Hint only
+ * view.contentDescription = "[[aihint:Video player, buffering]]"Wait, let me reconsider—the code block itself should stay unchanged. The fix should only apply to the list items:
* **AI Hints - App-Provided Context:**
- * Apps can provide domain-specific hints to help the AI understand the current screen better.
- * Embed hints in `contentDescription` using the `[[aihint:...]]` format:
+ * Apps can provide domain-specific hints to help the AI understand the current screen better.
+ * Embed hints in `contentDescription` using the `[[aihint:...]]` format:
```kotlin
// Hint only
view.contentDescription = "[[aihint:Video player, buffering]]"
@@ -101,7 +101,7 @@
// Structured data (JSON)
view.contentDescription = """[[aihint:{"screen":"player","state":"buffering"}]]"""
```
- * The `[[...]]` format allows hints to coexist with accessibility labels and supports structured data like JSON.
- * **Note:** Since this uses `contentDescription`, it affects screen readers. Enable hints only during testing via Intent flag or debug BuildConfig to avoid accessibility issues in production.
+ * The `[[...]]` format allows hints to coexist with accessibility labels and supports structured data like JSON.
+ * **Note:** Since this uses `contentDescription`, it affects screen readers. Enable hints only during testing via Intent flag or debug BuildConfig to avoid accessibility issues in production.Content Review: The documentation accurately reflects the new [[aihint:...]] format and provides clear examples covering hint-only, coexisting-with-label, and structured JSON use cases. The warning about screen reader impact is appropriate and helpful.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| * **AI Hints - App-Provided Context:** | |
| * Apps can provide domain-specific hints to help the AI understand the current screen better. | |
| * Set `contentDescription` with `ArbigentHint:` prefix: | |
| * Embed hints in `contentDescription` using the `[[aihint:...]]` format: | |
| ```kotlin | |
| view.contentDescription = "ArbigentHint:Video player, buffering" | |
| // Hint only | |
| view.contentDescription = "[[aihint:Video player, buffering]]" | |
| // Coexisting with accessibility label | |
| view.contentDescription = "Play button [[aihint:Video player, currently buffering]]" | |
| // Structured data (JSON) | |
| view.contentDescription = """[[aihint:{"screen":"player","state":"buffering"}]]""" | |
| ``` | |
| * The `[[...]]` format allows hints to coexist with accessibility labels and supports structured data like JSON. | |
| * **Note:** Since this uses `contentDescription`, it affects screen readers. Enable hints only during testing via Intent flag or debug BuildConfig to avoid accessibility issues in production. | |
| * **AI Hints - App-Provided Context:** | |
| * Apps can provide domain-specific hints to help the AI understand the current screen better. | |
| * Embed hints in `contentDescription` using the `[[aihint:...]]` format: |
🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
92-92: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
93-93: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
104-104: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
105-105: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
🤖 Prompt for AI Agents
In README.md around lines 91–105 the nested markdown list items use 4-space
indentation which violates MD007; change the list indentation to 2 spaces for
the nested bullets (e.g., the lines starting with "The `[[...]]` format..." and
"**Note:** ...", and any other nested list markers in this block) so they align
with the project's 2-space list style, and do not modify the contents of the
fenced Kotlin code block — only adjust the surrounding list marker indentation.
There was a problem hiding this comment.
It's working fine in previews
There was a problem hiding this comment.
@takahirom, understood! If the markdown renders correctly in your preview, you can keep it as-is.
If you'd like to suppress this markdownlint warning in the future, you can configure .markdownlint.json to adjust the MD007 rule settings or disable it for this project. But since it's rendering fine, no changes are needed.
🧠 Learnings used
Learnt from: takahirom
Repo: takahirom/arbigent PR: 329
File: arbigent-core/src/main/java/io/github/takahirom/arbigent/ArbigentAgent.kt:0-0
Timestamp: 2025-07-08T15:34:42.649Z
Learning: takahirom prefers simplified implementations over complex caching libraries, favoring manual cache management through methods like clearResolvedGoalCache() rather than using bounded cache implementations with automatic eviction policies.
What
ArbigentHint:prefix to[[aihint:...]]formatappHints→aiHints,APP_HINTS→AI_HINTS<HINT_FROM_APP>to<AI_HINTS>Why
[[...]]format allows hints to coexist with accessibility labels[[aistate:...]],[[aiaction:...]])]]exist in the text