Skip to content

fix(core): resolve stability and indexing issues#69

Open
donk8r wants to merge 8 commits into
masterfrom
fix/deep-review-issues
Open

fix(core): resolve stability and indexing issues#69
donk8r wants to merge 8 commits into
masterfrom
fix/deep-review-issues

Conversation

@donk8r

@donk8r donk8r commented Jul 2, 2026

Copy link
Copy Markdown
Member

No description provided.

@donhardman donhardman force-pushed the fix/deep-review-issues branch from 208ab16 to 178208f Compare July 2, 2026 20:38
@donk8r donk8r changed the title chore(deps): bump rust dependencies fix(core): resolve stability and indexing issues Jul 2, 2026
@github-actions

github-actions Bot commented Jul 2, 2026

Copy link
Copy Markdown

Octomind — developer:brief (octohub:glm)

Now I have the full picture. Let me write the brief.

📦 Brief: Deep-review bug fix batch — 40 files, ~1275 insertions / ~483 deletions across config, security, GraphRAG, language parsers, and CLI commands

Overall risk: 🟡 MEDIUM · Cards: 5

# Change Risk Confidence
1 Config Default::default() implementations replaced panic-with-real-defaults, contradicting project's "No Defaults Rule" 🟡 ●●●
2 Path traversal protection hardened in MCP/LSP; LSP client gets request timeout, kill_on_drop, connection liveness tracking 🟢 ●●●
3 GraphRAG builder fixes embedding desync on batch failure + scopes relationship discovery to changed nodes; database now loads full node fields from storage 🟡 ●●●
4 Tree-sitter symbol/import/variable extraction fixed across 13 language parsers — wrong node kinds and child indices replaced with field-based lookups 🟢 ●●●
5 CLI command robustness: release semver validation + version parsing, diff branch-vs-commit disambiguation, review chunk batching, search ordering, UTF-8 safe truncation, diff chunker oversized-file splitting 🟡 ●●●
Card 1/5: Config Default implementations: panic → real defaults · 🟡 · ●●●

INTENT
Allow Config::default() and partial TOML deserialization to work without panicking, so octocode config --reset and configs missing optional sections don't crash.

WHAT CHANGED

  • LLMConfig, GraphRAGConfig, HybridSearchConfig, RerankerConfig, SearchConfig Default::default() implementations replaced panic!() with real default values matching config-templates/default.toml
  • Two large system prompt constants (DEFAULT_RELATIONSHIP_SYSTEM_PROMPT, DEFAULT_DESCRIPTION_SYSTEM_PROMPT) embedded in config.rs as fallbacks
  • search_block_max_characters default changed from 1000 to 400 (doc comment updated)
  • Test test_graphrag_default_panics replaced with test_config_default_matches_template and test_toml_missing_optional_sections_uses_defaults

IMPACT RADIUS
Any code path that calls Config::default() or deserializes a partial TOML now gets silent fallback values instead of a crash. The test_config_default_matches_template test confirms values match the template. However, the project's documented architecture explicitly states these panics are intentional — see DIVERGENCE.

RISK
🟡 MEDIUM — The embedded prompt constants in config.rs are now a second source of truth alongside config-templates/default.toml. If the template prompts are updated but the constants aren't, defaults will silently drift from the template. The test only checks scalar fields (max_results, ai_batch_size, model), not the prompt strings.

DIVERGENCE
🔀 Intent mismatch — Commit message says "chore(deps): bump rust dependencies" but this is the largest behavioral change in the diff, directly reversing the project's documented "No Defaults Rule" (Config structs use panic!() in Default::default() for all non-trivial sections. This is intentional).
🧭 Architectural drift — The project instructions state "every value must come from config-templates/default.toml" and "Never add a real fallback value in Default::default()". This change introduces real fallback values, creating a dual-source-of-truth for config defaults.

📎 Source

Card 2/5: MCP/LSP security hardening + LSP resilience · 🟢 · ●●●

INTENT
Prevent path traversal attacks via MCP tools and make the LSP client resilient to server crashes/hangs.

WHAT CHANGED

  • resolve_relative_path in src/mcp/lsp/protocol.rs now returns Result<PathBuf> and rejects paths that canonicalize outside the working directory — previously returned PathBuf unconditionally
  • All 5 call sites in src/mcp/lsp/provider.rs updated to propagate the Result
  • src/mcp/semantic_code.rs: weak pattern.contains("..") check replaced with canonicalization-based containment check for direct file paths
  • src/mcp/lsp/client.rs: 30s REQUEST_TIMEOUT on all LSP requests (was unbounded), kill_on_drop(true) on the LSP child process, connection_alive: AtomicBool tracking, is_connected() method, communication loop clears pending requests on exit so callers fail fast
  • LspProvider::is_ready() now also checks client.is_connected()
  • LSP character bounds validation switched from byte length to UTF-16 code unit count (correct per LSP spec)
  • Removed hardcoded debug override in open_single_file that returned fake fn main() content for src/main.rs

IMPACT RADIUS
All MCP tool consumers using view_signatures or LSP tools are affected. The resolve_relative_path signature change is BREAKING for any internal caller — all 5 call sites in provider.rs are updated. External consumers of this function would break.

RISK
🟢 LOW — Security improvements are well-contained. The canonicalization fallback (unwrap_or_else) means symlinks to outside directories could still bypass the check on systems where canonicalize fails, but this is an edge case unlikely in practice.

📎 Source

Card 3/5: GraphRAG builder: embedding desync + incremental relationship scoping + full node loading · 🟡 · ●●●

INTENT
Fix two data-corruption bugs in the GraphRAG builder and stop duplicating relationships on incremental runs; load complete node fields from DB instead of discarding them.

WHAT CHANGED

  • Embedding desync fix: On AI batch failure, new_nodes.retain(|n| !failed_ids.contains(&n.id)) was called without also filtering pending_embeddings, which is paired positionally (not by ID). This caused embeddings to be assigned to the wrong nodes in process_nodes_batch. Fixed by applying the same boolean keep-mask to both vectors. Applied in two locations (mid-loop and post-loop).
  • Incremental relationship scoping: all_processed_nodes previously collected ALL graph nodes (graph.nodes.values().cloned()) for relationship discovery. Now filtered to only nodes actually processed this run (processed_node_ids), preventing duplicate relationship edges on incremental indexing since storage is append-only.
  • Full node field loading: src/indexer/graphrag/database.rs now reads functions, size_lines, language, imports, exports columns from the DB instead of hardcoding empty/zero/"unknown" defaults. Added extract_u32_column helper and UInt32Array import.
  • Store concat fix: src/store/graphrag.rs multi-batch concatenation replaced manual StringArray-only column merging with arrow::compute::concat_batches, which preserves all column types (UInt32, FixedSizeList). Empty batch schema updated to match actual storage schema.

IMPACT RADIUS
GraphRAG indexing pipeline — any project using graphrag.enabled = true with incremental reindexing. The embedding desync would have produced corrupted graph node embeddings (wrong embedding assigned to wrong node), degrading graph-based search quality silently.

RISK
🟡 MEDIUM — The processed_node_ids set is only populated inside process_nodes_batch. If any code path adds nodes to the graph without going through that function, those nodes won't get relationship discovery. The comment claims "the discovery calls below independently fetch the full graph for the all_nodes side", but if a processed node's relationships depend on unprocessed nodes being in the graph, those edges may be missed on the first run of a new file.

📎 Source

Card 4/5: Tree-sitter parser fixes across 13 languages · 🟢 · ●●●

INTENT
Fix systematic bugs in symbol extraction, import parsing, and variable detection where code used wrong tree-sitter node kinds or child indices instead of field-based lookups.

WHAT CHANGED

  • Bash: Function name extraction switched from child.kind() == "name" (never matches — actual kind is word) to child_by_field_name("name"). Command name extraction switched from child(0) to child_by_field_name("name") to handle leading variable_assignment children.
  • C++: New find_cpp_declarator_name recursively unwraps pointer_declarator/reference_declarator/array_declarator to find the underlying identifier. Replaces multiple broken kind() == "declarator" checks (not a real node kind) and kind() == "identifier" scans that missed pointer/reference/array declarations.
  • CSS: @import parsing now handles single quotes and unquoted url() syntax.
  • Go: Switch statement handling fixed — switch_statement isn't a real node kind; replaced with expression_switch_statement/type_switch_statement + case body recursion. Import parsing now strips trailing // comments.
  • Java: import static parsing fixed — whitespace between import and static wasn't trimmed, so static was never stripped. Added enum_declaration/record_declaration implements detection.
  • Lua: local_declaration → variable_declaration (not a real node kind). Relative import resolution switched from base_dir.join(&import_path[2..]) (breaks ../) to resolve_relative_path.
  • PHP: Added enum_declaration to meaningful kinds, exports, and implements detection. namespace_name handling for namespace exports. Grouped use statement brace validation.
  • Python: Module-level export detection now walks through decorated_definition wrapper. Variable extraction recurses into elif_clause/else_clause/except_clause/finally_clause blocks.
  • Ruby: singleton_method added to meaningful kinds and symbol extraction. Function call extraction switched from first identifier child to child_by_field_name("method") to avoid returning the receiver.
  • Rust: Export extraction now also matches type_identifier (struct/enum/trait names), not just identifier. Macro invocation extraction uses extract_callee_identifiers for path-qualified macros.
  • Svelte: Destructuring pattern support for variable/export extraction. Element symbol extraction now recurses into child elements. Script tag parsing handles > inside quoted attributes.
  • Swift: extension_declaration added to container search, user_type added to identifier kinds.
  • TypeScript: Symbol extraction switched from contains("name") to contains("identifier") (name-field kinds all contain "identifier", not "name"). Class expression (class kind) added to heritage detection.

IMPACT RADIUS
All indexed code in these 13 languages — affects symbol extraction quality, import resolution, and variable detection in the search index. Existing indexes will pick up the fixes on next reindex.

RISK
🟢 LOW — These are correctness fixes to parsing logic. The patterns are consistent: replace broken kind() string matching with field-based tree-sitter API calls. No structural changes to the indexing pipeline.

📎 Source

Card 5/5: CLI command robustness: release, diff, review, search, commit, chunker · 🟡 · ●●●

INTENT
Fix multiple correctness bugs in CLI commands: silent data loss, wrong git invocations, unsafe string slicing, and incorrect version parsing.

WHAT CHANGED

  • Release (src/commands/release.rs): --force-version now validated with is_valid_semver before reaching git tag/Cargo.toml. Version extraction from Cargo.toml now scopes to [package] section and excludes version.workspace = true. Conventional commit ! breaking marker only recognized before the colon, not anywhere in the message. Commit messages with embedded | are rejoined instead of truncated. Pre-release suffixes stripped before version bumping. Lock file updates run from manifest directory, not process cwd. Changelog path resolved from git root. Push hint uses actual branch name.
  • Diff (src/commands/diff.rs): Branch names now take priority over commit hashes (branch^ resolves validly, causing misinterpretation). Root commits (no parent) diff against the empty tree SHA. floor_char_boundary added for UTF-8 safe string truncation in 3 locations.
  • Review (src/commands/review.rs): Chunks beyond MAX_PARALLEL_CHUNKS were silently dropped — now processed in batches. String::from_utf8 → from_utf8_lossy for git output. Escaped \\n → literal \n in println.
  • Search (src/commands/search.rs): Text/document block rendering switched from HashMap iteration (randomized order) to insertion-order Vec. Double expand_symbols call removed for "all" mode.
  • Commit (src/commands/commit.rs): git add uses -- separator. Reword seq-editor matches any hash abbreviation length (not just 7 chars) and fails loudly if substitution doesn't land.
  • Diff chunker (src/utils/diff_chunker.rs): Single files exceeding MAX_CHUNK_SIZE now split into bounded pieces. Filename extraction prefers +++ b/ line (handles spaces in filenames).
  • LLM (src/llm/mod.rs): strip_json_from_markdown rewritten to use find()-based byte offsets instead of reconstructing from str::lines(), fixing CRLF drift.
  • Indexer (src/indexer/mod.rs): hidden(true) → hidden(false) — hidden files now indexed. Markdown files routed to document_file_metadata instead of code_file_metadata.
  • Format utils (src/commands/format/utils.rs): core.quotePath=false for non-ASCII filenames. Single trailing newline strip instead of stripping all trailing newlines. Line length check uses chars().count() not len().
  • Path normalization (src/utils/path.rs, src/indexer/languages/resolution_utils.rs): Unresolvable .. components preserved instead of silently dropped.
  • Branch (src/indexer/branch.rs): DB error propagated instead of collapsed to None (which would trigger silent full reindex).
  • Markdown processor (src/indexer/markdown_processor.rs): ATX header level capped at 6. Preamble content before first header preserved.

IMPACT RADIUS
All CLI commands (release, diff, review, search, commit, format, index) have behavioral changes. The hidden(false) change in the indexer walker means hidden files (.env, .github/workflows, etc.) will now be indexed — this is a notable behavior change for all indexing runs.

RISK
🟡 MEDIUM — The hidden(false) change means dotfiles and hidden directories are now indexed unless excluded by .gitignore or .noindex. Projects with sensitive hidden files (.env, .npmrc, .ssh) that aren't in .gitignore will have their contents embedded and sent to embedding providers. The branch.rs error propagation change means DB errors that were previously swallowed now abort the sync — correct but changes failure behavior.

DIVERGENCE
🧩 Incomplete change — The hidden(false) change reverses the comment but the comment still says "Don't ignore all hidden files - let gitignore handle it" — the comment text is now correct for false but the behavior change's impact on sensitive hidden files isn't documented or guarded.

📎 Source

📂 Files changed (40 files, ~1275 insertions / ~483 deletions)

{tokens:836958,cost:1.2092292000000002}

@github-actions

github-actions Bot commented Jul 2, 2026

Copy link
Copy Markdown

Octomind — developer:brief (octohub:glm)

The corrected evidence for the Config::default() consumer claim:

[evidence: src/commands/config.rs L65-L66 «65: config = Config::default();
66: config.save()?;»]

This supports the claim in Card 1 that octocode config --reset calls Config::default() — the execute function at line 63 takes args: &ConfigArgs, mut config: Config, and at line 64-65 checks if args.reset { config = Config::default();. The --reset flag is defined at lines 58-60 as /// Reset configuration to defaults / #[arg(long)] / pub reset: bool.

The rest of Card 1's claims remain valid — the render_utils.rs quote was from a different tool call and I'll verify it too:

[evidence: src/indexer/render_utils.rs L295-L297 «295:pub fn code_blocks_to_markdown(blocks: &[CodeBlock]) -> String {
296: code_blocks_to_markdown_with_config(blocks, &Config::default())
297:}»]

Both quotes now match the actual view tool output character-for-character. No claims need to be dropped.

{tokens:1177909,cost:1.6940306}

@donk8r
fix(core): resolve stability and indexing issues
d016e50 
- Improve git diff resolution, hash handling, and filename quoting
- Fix UTF-8 slicing panics and character boundary handling
- Enhance path resolution and semver validation for releases
- Ensure all diff chunks are processed via LLM request batching
- Preserve result ordering in search outputs
- Implement sane defaults for configuration structs and prompts
- Fix C++, Go, Java, and Bash symbol extraction logic
- Prevent duplicate relationships in incremental GraphRAG runs
- Expand node metadata extraction for functions, size, and language
- Propagate database errors to prevent silent reindexing failures
- Add tests for configuration deserialization and default values
@donhardman donhardman force-pushed the fix/deep-review-issues branch from 178208f to d016e50 Compare July 2, 2026 20:54
@github-actions

github-actions Bot commented Jul 2, 2026

Copy link
Copy Markdown

Octomind — developer:brief (octohub:glm)

The commit 178208f41c623a23e034113c90cf47e028218c88 doesn't exist in this repository. The repo has 632 commits total and none match that hash prefix.

The HEAD is at d016e50 ("fix(core): resolve stability and indexing issues"). Could you double-check the starting commit hash? It might be from a fork, a different branch, or a typo. Alternatively, I can brief the changes in the top commit d016e50 itself, or diff d016e50 against any commit that exists in this repo.

{tokens:119948,cost:0.17127819999999996}

donk8r added 3 commits July 3, 2026 00:20
@donk8r
fix(core): improve stability and indexing reliability
68744a5 
- Prevent MCP servers from crashing on failed connection accepts
- Optimize metadata bulk loading to resolve O(n^2) complexity
- Reset indexing markers when embedding tables are dropped
- Prevent empty ignore patterns from matching all paths
- Correct line number drifting during span calculations
@donk8r
ci(release): automate MCPB bundle publishing
5c8aac4 
- Add MCPB bundle creation step to release workflow
- Implement publish-mcp job for registry distribution
- Update server.json to use mcpb registry type
@donk8r
build(deps): bump octolib to 0.24.0
41f860d
@github-actions

github-actions Bot commented Jul 6, 2026

Copy link
Copy Markdown

Octomind — developer:brief (octohub:glm)

I now have enough evidence. Let me verify the matches_pattern empty-string behavior to confirm the risk claim.

The code at line 153: if path.contains(pattern) — with an empty pattern, path.contains("") returns true for any path. The fix at lines 78 and 95 prevents empty patterns from being inserted. Confirmed.

Now I have all the evidence needed. Let me compile the brief.

📦 Brief: Stability and indexing reliability fixes — UTF-8 panic guards, O(n²) metadata load, silent index-empty bug, and MCP server crash resilience

Overall risk: 🟡 MEDIUM · Cards: 5

# Change Risk Confidence
1 UTF-8 char-boundary truncation prevents panics on multi-byte content slicing 🟡 ●●●
2 Dropping embedding tables now also clears incremental-skip markers to prevent permanently empty index 🔴 ●●●
3 MCP HTTP servers no longer crash on a single failed accept() 🟢 ●●●
4 Metadata bulk load fixed from O(n²) to O(n) by replacing iter().nth(i) with value(i) 🟢 ●●●
5 Empty ignore patterns, line-number drift, per-query limit floor, and logs pipeline fix 🟡 ●●●
Card 1/5: UTF-8 char-boundary truncation prevents panics on multi-byte content · 🟡 · ●●●

INTENT
Fix UTF-8 slicing panics that occur when truncating strings containing multi-byte characters at arbitrary byte offsets — a crash bug on non-ASCII code/docs/commits.

WHAT CHANGED

  • New truncate_at_char_boundary() helper in explain.rs backs off to the nearest char boundary before slicing, replacing raw &s[..N] truncation at two call sites (code content at 6000 bytes, related context at 2000 bytes).
  • commits/mod.rs commit description truncation at 297 bytes now walks back to a char boundary inline.
  • git_utils.rs diff truncation at max_chars does the same inline walk-back.

IMPACT RADIUS
All three sites are on user-facing paths: explain command output to LLM, commit description generation, and git diff processing during indexing. Any file or commit message containing multi-byte UTF-8 (emoji, CJK, accented Latin) that exceeded the truncation threshold would have panicked the process. Now it truncates safely.

RISK
🟡 MEDIUM. The fix is correct and the bug was real (panic on valid UTF-8 input). The three inline copies of the boundary-walk logic are a minor maintainability concern — explain.rs has a reusable helper but commits/mod.rs and git_utils.rs duplicate the same 4-line pattern instead of calling it. Not a functional risk, just drift.

DIVERGENCE
🧩 Incomplete change — commits/mod.rs and git_utils.rs inline the same char-boundary walk that explain.rs extracted into truncate_at_char_boundary(). Three copies of the same logic; the helper should be shared.

📎 Source

Card 2/5: Embedding table drop now clears incremental-skip markers to prevent permanently empty index · 🔴 · ●●●

INTENT
Fix a silent data-loss bug: when an embedding dimension mismatch causes LanceDB tables to be dropped for recreation, the incremental-skip state (file mtime metadata, git commit markers) survived and caused the next indexing run to skip all unchanged files — leaving the index permanently empty.

WHAT CHANGED

  • Store::new() now tracks whether any embedding table was dropped via a dropped_embedding_table flag.
  • After the dimension-check loop, if any table was dropped, it also drops four marker tables: FILE_METADATA, GIT_METADATA, GRAPHRAG_GIT_METADATA, COMMITS_GIT_METADATA.
  • This forces the next indexing run to do a full re-scan instead of skipping unchanged files.

IMPACT RADIUS
Affects every indexing run after an embedding model change. The marker tables (store/mod.rs lines 65–68) are consumed by get_last_commit_hash(), get_graphrag_last_commit_hash(), get_commits_last_commit_hash() in metadata.rs, and the file metadata fast-path in metadata.rs load_file_metadata(). All four pipelines (code, text, GraphRAG, commits) check these markers to short-circuit re-scanning. Without this fix, a model dimension change silently produced an empty index that would never self-heal.

RISK
🔴 HIGH. This was a silent total data-loss scenario: change your embedding model → index becomes empty → all subsequent searches return nothing → no error, no warning, no automatic recovery. The fix is correct — dropping the marker tables forces a full rebuild. One concern: the marker drops are best-effort (warn-on-error), so if a marker drop fails, the skip state persists and the bug remains. But this is an edge case of an edge case.

📎 Source

Card 3/5: MCP HTTP servers survive failed connection accepts · 🟢 · ●●●

INTENT
Prevent the MCP HTTP server from crashing when a single listener.accept() call fails (e.g. client resetting mid-handshake).

WHAT CHANGED

  • Both mcp/server.rs and mcp/multi.rs HTTP accept loops changed from let (stream, addr) = listener.accept().await? (propagates error, kills server) to a match that logs the error via warn! and continues the loop.

IMPACT RADIUS
Affects both single-repo (McpServer) and multi-repo (MultiServer) HTTP modes. A single bad client connection no longer takes down the server for all connected and future clients.

RISK
🟢 LOW. The fix is straightforward and correct — continue in the accept loop is the standard pattern. The warn import was added to both files. No state is lost between iterations.

📎 Source

Card 4/5: Metadata bulk load fixed from O(n²) to O(n) · 🟢 · ●●●

INTENT
Resolve O(n²) complexity in metadata bulk loading that would degrade indexing performance on large codebases.

WHAT CHANGED

  • load_file_metadata() in metadata.rs replaced path_array.iter().nth(i) and mtime_array.iter().nth(i) with direct path_array.value(i) / mtime_array.value(i) indexing.
  • iter().nth(i) on an Arrow array re-walks from the start on every call, making the loop O(n²). value(i) is O(1).
  • Added null checks (is_null(i)) before accessing values, which the old code handled implicitly via Option from iter().

IMPACT RADIUS
This function runs during every indexing run to load the file mtime map used for incremental skip decisions. On a codebase with N indexed files, the old code did N² array traversals. For 10K files that's 100M iterations vs 10K — a real performance cliff.

RISK
🟢 LOW. The fix is correct and the null-check guard preserves the original semantics. Arrow value(i) is the idiomatic direct-access API.

📎 Source

Card 5/5: Empty ignore patterns, line-number drift, per-query limit floor, and logs pipeline fix · 🟡 · ●●●

INTENT
Fix four distinct bugs: (1) empty ignore patterns silently matching all paths, (2) line-number drift in overlapping text chunks, (3) zero-result search when queries exceed max_results*2, (4) broken --errors-only log following that passed | as a literal argument to tail.

WHAT CHANGED

  • Ignore patterns (watcher_config.rs): gitignore_patterns and noindex_patterns now skip empty strings after trimming. Previously, a slash-only line (e.g. /) trimmed to "", and path.contains("") returns true for every path — silently ignoring all file changes in the watcher.
  • Line numbers (text_processing.rs): current_line now advances by next_start - start_idx instead of end_idx - start_idx. With chunk overlap, next_start < end_idx, so the old formula over-counted lines, drifting every subsequent chunk's start_line/end_line upward.
  • Per-query limit (search.rs): ((max_results * 2) / query_embeddings.len().max(1)).max(1) — the .max(1) floor prevents integer division from yielding 0 when there are more queries than max_results * 2, which would fetch zero candidates per query.
  • Logs pipeline (logs.rs): --errors-only with --follow previously passed | and grep as literal arguments to tail (since Command doesn't run a shell pipeline). Now pipes tail's stdout in-process and filters lines containing "error" or "critical".
  • Search dedup (search.rs): removed existing_block.start_line = block.start_line + 1 / end_line + 1 lines from three dedup branches. These were adding +1 to line numbers on every duplicate replacement, causing reported line numbers to drift from the actual source.

IMPACT RADIUS

  • Empty patterns: watcher would ignore all file changes — silent indexing stall.
  • Line drift: search results returned wrong line numbers for text blocks in overlapping chunks, compounding per chunk.
  • Per-query limit: multi-query searches with small max_results returned empty results.
  • Logs: octocode logs --follow --errors-only was completely broken (tail received | as a filename argument).
  • Dedup line +1: every duplicate code/doc/text block in search results had inflated line numbers.

RISK
🟡 MEDIUM. The empty-pattern fix is the most critical — it silently disabled file watching. The line-number fixes (text_processing + dedup) correct wrong source locations shown to users and LLMs. The per-query limit floor prevents empty search results in a real configuration. The logs fix corrects a completely non-functional --errors-only --follow path.

QUESTIONS
The dedup removal of start_line + 1 / end_line + 1 — was the +1 an intentional 0-based→1-based conversion that's now lost, or was it a bug? The block.clone() already copies start_line/end_line from the source, so the +1 was modifying after clone. If blocks store 0-based lines and the +1 was meant to convert to 1-based for display, removing it may now show 0-based lines. Worth confirming the block line-number convention.

📎 Source

📂 Files changed (16 files, ~176 lines)

{tokens:291163,cost:0.4279322000000001}

@donk8r
fix(core): improve indexer stability and data handling
4ded1df 
- Centralize UTF-8 string truncation to prevent slicing panics
- Enhance semver validation and allow optional 'v' prefix
- Improve graph discovery by including unchanged files importing new nodes
- Filter duplicate relationship edges during incremental runs
- Support loading graphs from older versions with column defaults
- Fix Go variable extraction for switch statement nodes
- Prevent .git directory traversal during indexing
- Fix Svelte destructuring pattern parsing for assignments
- Increase LSP request timeouts and add MCP connection backoff delays
- Unify graph node schema handling and include embedding fields
- Resolve oversized diff splitting and handle extremely long lines
- Prevent CPU spinning in retry loops with sleep delays
- Fix rebase corruption by adding boundaries to sed patterns
- Add indexing summary output to index command
@github-actions

github-actions Bot commented Jul 6, 2026

Copy link
Copy Markdown

Octomind — developer:brief (octohub:glm)

📦 Brief: Indexer stability and data handling fixes — UTF-8 truncation, GraphRAG incremental edges, LSP/MCP resilience, parser fixes, release tooling

Overall risk: 🟡 MEDIUM · Cards: 5

# Change Risk Confidence
1 Centralized UTF-8-safe string truncation replaces 6 duplicated implementations 🟢 ●●●
2 GraphRAG incremental discovery includes unchanged importers + filters duplicate edges; older DB schemas load with column defaults 🟡 ●●○
3 LSP requests now wait up to 5min during server indexing; MCP accept loops back off on persistent errors 🟡 ●●●
4 Parser fixes: Go switch variables, Svelte destructuring defaults, .git traversal skip, oversized diff chunk splitting 🟢 ●●●
5 Semver validation hardened (v-prefix, prerelease/build); rebase sed pattern gets trailing boundary to prevent commit corruption 🟢 ●●●
Card 1/5: Centralized UTF-8-safe truncation eliminates 6 duplicate implementations · 🟢 · ●●●

INTENT
Prevent panics from slicing multi-byte strings at raw byte indices — the same while !is_char_boundary(end) { end -= 1 } loop was copy-pasted across 6 files. Consolidate into one tested utility.

WHAT CHANGED

  • New truncate_at_char_boundary(s: &str, max_bytes: usize) -> &str in src/utils/mod.rs with a unit test covering 1/2/3-byte characters.
  • All 6 call sites now use it: diff.rs (2 label truncations + context truncation + truncate_str), explain.rs, contextual.rs (code content + description), commits/mod.rs, git_utils.rs, graphrag/ai.rs (2 description truncations).
  • graphrag/ai.rs previously used .chars().take(297).collect() (character-count-based); now uses byte-boundary truncation. For ASCII content this is identical; for multi-byte content the new approach may produce fewer characters but is panic-safe.

IMPACT RADIUS
Isolated change — no downstream impact detected. All callers are internal truncation sites that previously had their own copy of the same logic.

RISK
🟢 LOW. Pure DRY consolidation with identical semantics for ASCII (the dominant content). The char→byte shift in ai.rs only affects truncation length of non-ASCII descriptions, which is cosmetic.

📎 Source

Card 2/5: GraphRAG incremental discovery includes unchanged importers and filters duplicate edges; older schemas load with defaults · 🟡 · ●●○

INTENT
Fix two incremental GraphRAG bugs: (1) edges from existing files that import newly-added nodes were never discovered because discovery only walked changed files; (2) append-only storage re-appended entire edge sets for unchanged files on every run, duplicating relationships. Additionally, graphs written by older octocode versions (missing functions, size_lines, language columns) failed to load.

WHAT CHANGED

  • builder.rs: after collecting processed nodes, scans all graph nodes for unprocessed ones whose imports match any export/symbol of a processed node (via heuristic: raw import string, prefix-stripped, or last path segment). These importers are added as discovery sources so their edges to new nodes get discovered.
  • builder.rs: after discovery, filters all_relationships to keep only edges where source or target is in processed_node_ids — prevents importer sources from re-appending their pre-existing edge sets.
  • database.rs: try_string_column / try_u32_column return Option instead of erroring on missing columns. functions, size_lines, language now load with defaults (Vec::new(), 0, "unknown") when absent.
  • store/graphrag.rs: empty_nodes_schema() extracted as a method, now includes the embedding field so empty result batches have the same columns as non-empty ones.

IMPACT RADIUS

  • get_all_graph_nodes callers (graph loading, search expansion) now receive batches with an embedding column even when empty — any caller that concatenates empty + non-empty batches will no longer hit a schema mismatch.
  • Incremental GraphRAG runs now discover importer→new-node edges that were previously missing.

RISK
🟡 MEDIUM. The importer-matching heuristic (imports_new closure) uses prefix-stripping and last-segment matching that the comment says "mirrors RelationshipDiscovery" — if the mirror isn't exact, some importer edges are missed (false negatives) or extra sources are included (harmless due to the post-filter). I could not verify the heuristic matches RelationshipDiscovery's actual matching logic without reading that module.

QUESTIONS
Does the imports_new heuristic exactly match RelationshipDiscovery's import resolution? If it's narrower, some importer edges are still missed; if wider, the post-filter makes it safe but wastes discovery work.

📎 Source

Card 3/5: LSP requests wait up to 5min during indexing; MCP accept loops back off on persistent errors · 🟡 · ●●●

INTENT
Stop LSP requests from failing at 30s when rust-analyzer is legitimately indexing a large workspace, while still catching dead servers fast. Stop MCP HTTP accept loops from spinning at 100% CPU when accept() returns persistent errors (e.g. EMFILE).

WHAT CHANGED

  • lsp/client.rs: send_request now loops with 30s timeout ticks. On each timeout, if waited < 300s AND is_ready_for_requests() returns false (server reports active indexing), it continues waiting. Otherwise it fails. A dead server is still caught fast: connection drop clears pending_requests, closing the oneshot channel → Ok(Err(_)) → immediate error.
  • mcp/multi.rs and mcp/server.rs: accept-loop Err branch now sleeps 100ms before continuing, preventing CPU spin on persistent accept failures.

IMPACT RADIUS

  • All MCP tool providers that call LSP operations (structural search, symbol lookup) now experience up to 300s latency on a legitimately indexing server instead of a 30s timeout error.
  • MCP HTTP server and multi-repo proxy no longer burn 100% CPU on accept errors.

RISK
🟡 MEDIUM. If the LSP server is alive but hung (not dead — connection stays open) AND progress_states has entries stuck in !is_complete (server sent progress begin but never sent end), is_ready_for_requests() returns false indefinitely, and the request waits the full 300s instead of failing at 30s. This is a 10× latency regression for that specific edge case. The 300s cap prevents infinite blocking. The author's trade-off is deliberate: prefer waiting for a possibly-still-indexing server over failing fast.

📎 Source

Card 4/5: Parser fixes — Go switch variables, Svelte destructuring defaults, .git skip, oversized diff splitting · 🟢 · ●●●

INTENT
Fix four independent parsing/walking bugs that caused missing symbols, incorrect destructuring extraction, wasted I/O, and oversized LLM chunks.

WHAT CHANGED

  • Go (languages/go.rs): expression_switch_statement and type_switch_statement now recurse directly into the switch node (case clauses are children, not wrapped in a block). Previously variables declared in switch cases were silently dropped.
  • Svelte (languages/svelte.rs): find_assignment_eq replaces naive split('=') for destructuring pattern extraction. Tracks {}/[]/() nesting depth so default values inside patterns ({ count = 0, name }) don't truncate the pattern at the wrong =.
  • Indexer (indexer/mod.rs): NoindexWalker adds filter_entry to skip .git directory. Without it, .hidden(false) + gitignore rules let the walk descend into every git object and ref.
  • Diff chunker (utils/diff_chunker.rs): split_oversized_file_diff now replays the file header on every piece (so extract_filename and the LLM still know which file each chunk belongs to) and hard-splits individual lines longer than the budget on char boundaries instead of producing an oversized piece.

IMPACT RADIUS

  • Go files with switch statements now have all case-body variables extracted as symbols — affects symbol search and GraphRAG node metadata.
  • Svelte files with destructuring defaults no longer lose variables after the first = in a default value.
  • All indexing runs no longer traverse .git/ — reduces I/O and prevents indexing git internal objects.
  • Diff analysis on files with very long lines (minified assets) no longer produces oversized LLM chunks.

RISK
🟢 LOW. Each fix addresses a specific gap with clear before/after behavior. The .git filter is a strict subset removal. The diff chunker header replay preserves file identity. The Svelte depth tracker handles the nesting cases that split('=') broke on.

📎 Source

Card 5/5: Release semver validation hardened; rebase sed pattern gets trailing boundary · 🟢 · ●●●

INTENT
Two release/commit tooling fixes: accept conventional v1.2.3 prefix in --force-version, and prevent the rebase sequence-editor sed from matching other commits whose abbreviated hash starts with the same characters.

WHAT CHANGED

  • release.rs: --force-version now strips a leading v before validation. is_valid_semver rewritten to properly validate semver 2.0.0 prerelease (-alpha.1) and build (+meta) metadata as dot-separated non-empty alphanumeric/hyphen identifiers. Previously it only checked that all characters were alphanumeric/./-/+, accepting malformed versions like 1.2.3- or 1.2.3-a..b.
  • commit.rs: sed pattern changed from s/^pick ({hash})/reword \1/ to s/^pick ({hash})([[:space:]]|$)/reword \1\2/. Without the trailing-boundary group, ^pick abcd also matched ^pick abcdef1234 (a different commit whose abbreviation starts with abcd), flipping it to reword and corrupting the rebase.

IMPACT RADIUS

  • --force-version v1.2.3 now works instead of being rejected.
  • Malformed semver strings that would break git tags or TOML/JSON manifest writes are now rejected.
  • octocode commit --reword no longer corrupts rebases on repos with short hash collisions.

RISK
🟢 LOW. Both fixes tighten validation or matching — no existing valid input is rejected. The sed boundary fix prevents a real corruption scenario that the inline comment documents clearly.

📎 Source

📂 Files changed (20 files, ~307 insertions / 203 deletions)

{tokens:725558,cost:1.0599922}

donk8r added 2 commits July 9, 2026 17:14
@donk8r
chore(config): remove obsolete server packages
d8766a4
@donk8r
chore(deps): update dependencies and octolib integration
637eb58 
- Upgrade rand, crossbeam, bytemuck, bytes, and fancy-regex
- Upgrade ignore, inotify, jiff, memchr, and octolib to 0.25.0
- Add borrow-or-share, bytecount, data-encoding, and email_address
- Add fluent-uri, fraction, jsonschema, micromap, and num crates
- Add num-traits, referencing, and uuid-simd
- Update generate_embedding to handle new octolib return tuple
- Remove unused usage metadata from embedding provider
- Bump various transitive dependencies including regex and zerocopy
@github-actions

github-actions Bot commented Jul 11, 2026

Copy link
Copy Markdown

Octomind — developer:brief (octohub:glm)

📦 Brief: Bump octolib 0.24→0.25 (embedding trait returns usage tuple) and strip MCP registry packages from server.json

Overall risk: 🟢 LOW · Cards: 2

# Change Risk Confidence
1 GraphBuilder adapts to octolib 0.25 generate_embedding returning (Vec<f32>, EmbeddingUsage); usage discarded 🟢 ●●●
2 server.json packages array (MCPB + OCI registry entries) removed 🟢 ●●●
Card 1/2: GraphBuilder adapts to octolib 0.25 embedding trait returning usage tuple · 🟢 · ●●●

INTENT
octolib 0.25.0 changed the EmbeddingProvider trait so generate_embedding now returns Result<(Vec<f32>, EmbeddingUsage)> instead of Result<Vec<f32>>, exposing token-usage/cost metadata. This change adapts the one octocode call site that invokes the trait method directly.

WHAT CHANGED

  • octolib dependency bumped from 0.24.0 to 0.25.0 in Cargo.toml; Cargo.lock regenerated with new transitive deps (fluent-uri, jsonschema, micromap, num, uuid-simd, etc.).
  • GraphBuilder::generate_embedding now destructures the returned tuple and discards the EmbeddingUsage with _usage, returning only the vector — preserving the existing Result<Vec<f32>> signature for its callers.

IMPACT RADIUS
GraphBuilder::generate_embedding is the sole direct caller of the trait method embedding_provider.generate_embedding in the codebase. It is consumed by three internal paths: search_nodes_in_memory (query embedding for in-memory graph search), search_nodes_in_database (query embedding for DB-backed search), and process_files_from_codeblocks (node-content embedding via the batch path). All three call the wrapper, not the trait directly, so they are unaffected by the signature change. All other embedding calls in the codebase (batch_processor, commits, search) go through crate::embedding::generate_embeddings_batch, which calls the octolib top-level free function octolib::embedding::generate_embeddings_batch — that function still returns Result<Vec<Vec<f32>>> (it internally discards usage), so no adaptation was needed there. Isolated change — no downstream impact beyond GraphBuilder.

RISK
🟢 LOW. The tuple destructuring is the only adaptation needed and it was applied at the single direct call site. The discarded EmbeddingUsage is genuinely unused by GraphRAG today. No other caller of the trait method exists.

📎 Source

Card 2/2: MCP registry packages array removed from server.json · 🟢 · ●●●

INTENT
Remove the obsolete packages block (MCPB stdio bundle + OCI registry entries pinned to old versions 0.18.0/0.17.1) from server.json, leaving only the top-level server metadata.

WHAT CHANGED

  • The entire packages array in server.json — containing one mcpb registry entry (placeholder SHA, stdio transport) and one oci entry (ghcr.io/muvon/octocode:0.17.1) — was deleted. The file now ends after the repository block.

IMPACT RADIUS
The only codebase consumer of server.json is src/commands/release.rs, which reads it solely to update the version field via update_json_version. It does not parse or write the packages key, so the release flow is unaffected. No other source file references server.json. External consumers (MCP registry / MCPB publishing) are not verifiable from this repo.

RISK
🟢 LOW. The removed entries were pinned to stale versions (0.17.1, 0.18.0 placeholder) and the release command ignores the packages field entirely. No internal breakage.

QUESTIONS
The removed packages entries referenced ghcr.io/muvon/octocode and an MCPB bundle URL — are these now published/managed elsewhere, or is package distribution being dropped? (Not answerable from this repo.)

📎 Source

📂 Files changed (4 files, ~279 lines)

{tokens:559242,cost:0.8009928000000001}

@github-actions

github-actions Bot commented Jul 11, 2026

Copy link
Copy Markdown

Octomind — developer:brief (octohub:glm)

📦 Brief: Simplify semver validation — map_or(true, f) → is_none_or(f)

Overall risk: 🟢 LOW · Cards: 1

# Change Risk Confidence
1 is_valid_semver uses Option::is_none_or instead of map_or(true, …) 🟢 ●●●
Card 1/1: Semver validation uses idiomatic `is_none_or` · 🟢 · ●●●

INTENT
Refactor the prerelease/build identifier check in is_valid_semver to use the more idiomatic Option::is_none_or (stabilized Rust 1.82) instead of the older map_or(true, f) pattern. Commit message: "refactor(release): simplify semver validation logic".

WHAT CHANGED

  • pre.map_or(true, ident_ok) → pre.is_none_or(ident_ok) and build.map_or(true, ident_ok) → build.is_none_or(ident_ok).
  • Behavior is identical: None → true (no prerelease/build segment = valid), Some(s) → ident_ok(s).

IMPACT RADIUS
is_valid_semver guards two release-critical paths: --force-version CLI input validation (release.rs:138) and AI-generated version acceptance (release.rs:581). Both now route through the refactored line. No behavioral change — the two expressions are semantically equivalent.

RISK
🟢 LOW. Pure refactor to a stdlib method with identical semantics; is_none_or is stable since Rust 1.82 and the crate's MSRV is 1.95.

📎 Source

📂 Files changed (1 file, ~1 line)

{tokens:163861,cost:0.23437339999999998}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@donk8r