fix(chunker): add TOML/PS1 file recognition for search indexing - #29
fix(chunker): add TOML/PS1 file recognition for search indexing#29aspnmy wants to merge 1 commit into
Conversation
The chunker.is_indexable() relied on SupportLang::from_path() which does not recognize .toml or .ps1/.psm1/.psd1 extensions. Meanwhile, map/digest had hardcoded extension checks for these formats via can_parse_for_hook(). Changes: - Add ChunkerKind::Plain(&str) variant for non-tree-sitter formats - Extend is_indexable() with explicit .toml/.ps1/.psm1/.psd1 checks - Add paragraph_split_points() for plain-text chunking at blank lines - Add tests: is_indexable_accepts_toml, is_indexable_accepts_powershell
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
📝 WalkthroughWalkthroughAdds a ChangesPlain-text chunking support
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
![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 adds support for indexing plain-text formats, specifically TOML and PowerShell files, using a new paragraph-based chunking strategy that splits content at blank-line boundaries. The review feedback correctly identifies a bug in the paragraph-splitting logic, which fails to recognize boundaries in files with CRLF line endings (common on Windows and in PowerShell scripts), and provides a robust code suggestion to handle carriage returns.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| fn paragraph_split_points(source: &str) -> Vec<usize> { | ||
| let mut points = vec![0usize]; | ||
| let bytes = source.as_bytes(); | ||
| let len = bytes.len(); | ||
| let mut i = 0; | ||
| while i < len { | ||
| if bytes[i] == b'\n' { | ||
| let start = i; | ||
| while i < len && bytes[i] == b'\n' { | ||
| i += 1; | ||
| } | ||
| if i - start >= 2 && i < len { | ||
| points.push(i); | ||
| } | ||
| } else { | ||
| i += 1; | ||
| } | ||
| } | ||
| if *points.last().unwrap() < source.len() { | ||
| points.push(source.len()); | ||
| } | ||
| points | ||
| } |
There was a problem hiding this comment.
The current implementation of paragraph_split_points only checks for consecutive \n bytes to identify paragraph boundaries (blank lines). However, on Windows platforms or in repositories using CRLF line endings (\r\n), a blank line is represented by \r\n\r\n. In this case, the bytes are \n, \r, \n, so bytes[i] == b'\n' followed by bytes[i+1] == b'\r' prevents the while loop from matching consecutive \ns. As a result, i - start will be 1, and the boundary will not be detected. This means paragraph splitting will completely fail for CRLF files (such as PowerShell scripts, which are highly likely to use CRLF on Windows).
We can fix this by explicitly handling optional \r characters between consecutive \n bytes.
fn paragraph_split_points(source: &str) -> Vec<usize> {
let mut points = vec![0usize];
let bytes = source.as_bytes();
let len = bytes.len();
let mut i = 0;
while i < len {
if bytes[i] == b'\n' {
let mut next = i + 1;
if next < len && bytes[next] == b'\r' {
next += 1;
}
if next < len && bytes[next] == b'\n' {
while next < len && (bytes[next] == b'\n' || bytes[next] == b'\r') {
next += 1;
}
points.push(next);
i = next;
} else {
i += 1;
}
} else {
i += 1;
}
}
if *points.last().unwrap() < source.len() {
points.push(source.len());
}
points
}
|
@coderabbitai full review |
✅ Action performedFull review finished. |
![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)
src/search/chunker.rs (1)
426-443: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winTests should assert the exact
ChunkerKind::Plainmapping, not onlyis_some().These tests currently pass even if
.toml/PowerShell later map to a different chunker kind. As written, they don’t lock the contract introduced in this PR.Proposed test tightening
#[test] fn is_indexable_accepts_toml() { - assert!( - is_indexable(&PathBuf::from("Cargo.toml")).is_some(), - "expected .toml to be indexable" + assert_eq!( + is_indexable(&PathBuf::from("Cargo.toml")), + Some(ChunkerKind::Plain("toml")) ); } #[test] fn is_indexable_accepts_powershell() { for ext in ["ps1", "psm1", "psd1"] { let p = PathBuf::from(format!("script.{}", ext)); - assert!( - is_indexable(&p).is_some(), - "expected .{} to be indexable", - ext - ); + assert_eq!( + is_indexable(&p), + Some(ChunkerKind::Plain("powershell")), + "expected .{} to map to plain powershell chunker", + ext + ); } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/search/chunker.rs` around lines 426 - 443, The tests is_indexable_accepts_toml and is_indexable_accepts_powershell currently only check that is_indexable returns Some using is_some(), but they should assert the exact ChunkerKind value being returned. Replace the is_some() checks with assertions that verify the returned value equals Some(ChunkerKind::Plain) to properly enforce the contract introduced in this PR. This ensures the tests will fail if these file types are later mapped to a different chunker kind.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/search/chunker.rs`:
- Around line 165-190: The paragraph_split_points function only detects
Unix-style newlines (LF, `\n`) when identifying blank-line boundaries, causing
Windows-style line endings (CRLF, `\r\n`) to not trigger paragraph splits.
Modify the logic in the while loop that checks for consecutive newlines to also
account for carriage return characters (`\r`). When counting consecutive newline
characters to determine if a blank line exists (currently checking if i - start
>= 2), ensure the logic correctly handles cases where `\r\n` sequences appear,
treating them as part of the blank line boundary detection instead of stopping
at just `\n` characters.
---
Nitpick comments:
In `@src/search/chunker.rs`:
- Around line 426-443: The tests is_indexable_accepts_toml and
is_indexable_accepts_powershell currently only check that is_indexable returns
Some using is_some(), but they should assert the exact ChunkerKind value being
returned. Replace the is_some() checks with assertions that verify the returned
value equals Some(ChunkerKind::Plain) to properly enforce the contract
introduced in this PR. This ensures the tests will fail if these file types are
later mapped to a different chunker kind.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 15476f2d-0aad-4138-98c0-30a664b5757b
📒 Files selected for processing (1)
src/search/chunker.rs
| /// Split at blank-line boundaries (consecutive newlines). | ||
| /// Used for plain-text formats (TOML, PowerShell) that lack tree-sitter grammars. | ||
| fn paragraph_split_points(source: &str) -> Vec<usize> { | ||
| let mut points = vec![0usize]; | ||
| let bytes = source.as_bytes(); | ||
| let len = bytes.len(); | ||
| let mut i = 0; | ||
| while i < len { | ||
| if bytes[i] == b'\n' { | ||
| let start = i; | ||
| while i < len && bytes[i] == b'\n' { | ||
| i += 1; | ||
| } | ||
| if i - start >= 2 && i < len { | ||
| points.push(i); | ||
| } | ||
| } else { | ||
| i += 1; | ||
| } | ||
| } | ||
| if *points.last().unwrap() < source.len() { | ||
| points.push(source.len()); | ||
| } | ||
| points | ||
| } | ||
|
|
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
CRLF blank lines are not split as paragraphs.
paragraph_split_points only treats consecutive \n bytes as a boundary, so Windows-style blank lines (\r\n\r\n) won’t trigger splits. That makes plain chunking behavior inconsistent across platforms and defeats the intended paragraph boundary behavior for common PowerShell/TOML files.
Proposed fix
fn paragraph_split_points(source: &str) -> Vec<usize> {
let mut points = vec![0usize];
let bytes = source.as_bytes();
let len = bytes.len();
let mut i = 0;
+
+ let linebreak_len = |b: &[u8], pos: usize| -> Option<usize> {
+ match b.get(pos).copied() {
+ Some(b'\n') => Some(1),
+ Some(b'\r') => {
+ if b.get(pos + 1) == Some(&b'\n') {
+ Some(2)
+ } else {
+ Some(1)
+ }
+ }
+ _ => None,
+ }
+ };
+
while i < len {
- if bytes[i] == b'\n' {
- let start = i;
- while i < len && bytes[i] == b'\n' {
- i += 1;
- }
- if i - start >= 2 && i < len {
+ if linebreak_len(bytes, i).is_some() {
+ let mut breaks = 0usize;
+ while let Some(lb) = linebreak_len(bytes, i) {
+ breaks += 1;
+ i += lb;
+ }
+ if breaks >= 2 && i < len {
points.push(i);
}
} else {
i += 1;
}
}
if *points.last().unwrap() < source.len() {
points.push(source.len());
}
points
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/search/chunker.rs` around lines 165 - 190, The paragraph_split_points
function only detects Unix-style newlines (LF, `\n`) when identifying blank-line
boundaries, causing Windows-style line endings (CRLF, `\r\n`) to not trigger
paragraph splits. Modify the logic in the while loop that checks for consecutive
newlines to also account for carriage return characters (`\r`). When counting
consecutive newline characters to determine if a blank line exists (currently
checking if i - start >= 2), ensure the logic correctly handles cases where
`\r\n` sequences appear, treating them as part of the blank line boundary
detection instead of stopping at just `\n` characters.
3 participants
本 PR 做了什么
修复 ast-bro 搜索索引器（chunker）对 TOML 和 PowerShell 文件的识别缺陷。
chunker::is_indexable()此前仅依赖SupportLang::from_path()检测文件类型，而
ast_grep_language枚举中不包含 TOML/PowerShell variant，导致
.toml/.ps1/.psm1/.psd1文件被搜索索引器跳过。新增
ChunkerKind::Plain变体，通过空行分段策略支持这些纯文本格式。为什么需要
map/digest命令可以通过can_parse_for_hook()中的硬编码扩展名列表正确解析 TOML/PS1 文件，但
index/search命令走独立的is_indexable()路径，两条路径不一致导致：
ast-bro map Cargo.toml✅ 可解析ast-bro search "serde"❌ 不返回 Cargo.toml 结果ast-bro map monitor.ps1✅ 可解析ast-bro search "Get-ProcessInfo"❌ 不返回 monitor.ps1 结果审查者验证方式
如何验证
效果对比
改前:
is_indexable("Cargo.toml")→None，文件不进入索引改后:
is_indexable("Cargo.toml")→Some(ChunkerKind::Plain("toml"))，文件被空行分段索引已测试平台
验证命令
风险与范围
关联 Issue
无
English Translation
What this PR does
Fixes the search indexer (chunker) to recognize TOML and PowerShell files.
Previously
chunker::is_indexable()relied solely onSupportLang::from_path(),which lacks TOML/PowerShell variants in
ast_grep_language, causing.toml/.ps1files to be silently skipped by the indexer.Adds a
ChunkerKind::Plainvariant with paragraph-boundary splitting.Why it's needed
map/digestparse TOML/PS1 correctly via hardcoded extension checks incan_parse_for_hook(), butindex/searchuse a separateis_indexable()path — creating an inconsistency where map works but search doesn't.
Reviewer Test Plan
Evidence (Before & After)
Before:
is_indexable("Cargo.toml")→NoneAfter:
is_indexable("Cargo.toml")→Some(ChunkerKind::Plain("toml"))Risk & Scope
AI Assistance Disclosure
本 PR 使用 aspnmy-Qwen-Auto-Cli 辅助实现和审查。
Summary by CodeRabbit
New Features
.tomlfiles.ps1,.psm1,.psd1)