Skip to content

fix(search): iterative vscode field walk — deep content nesting no longer aborts index - #83

Merged
Icaro0310 merged 3 commits into
mainfrom
feat/multi-source-collectors
Oct 22, 2026
Merged

Icaro0310 merged 3 commits into
mainfrom
feat/multi-source-collectors

Conversation

@Icaro0310

@Icaro0310 Icaro0310 commented Oct 15, 2026 •

Copy link
Copy Markdown
Owner

Summary

  • _vscode_field_text rewritten iterative — a chat doc with hundreds of nested content fields hit RecursionError and aborted the whole index run (devin-review finding on feat(search): multi-source collectors — copilot-cli, codex-cli, vscode-chat #82, which merged while the fix was being prepared).
  • Output is unchanged: \n-join is associative over the flat leaf sequence, traversal order preserved (lists in order, dicts in _VSCODE_CONTENT_KEYS order, empties dropped).
  • Regression test: 5000-level nested content doc indexes and searches fine.

Test plan

  • pytest tests/test_collectors.py — 29 passed incl. test_vscode_deep_content_nesting_indexes

Generated with Devin


Devin Review

@github-actions github-actions Bot added the documentation Improvements or additions to documentation label Oct 15, 2026

@devin-ai-integration devin-ai-integration Bot left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note

Newer findings are available below. Devin Review posted a newer report on this PR, in addition to the findings presented here.

Devin Review found 4 potential issues.

Devin Review

Comment on lines +1389 to +1399
if any(r.is_dir() for r in copilot_roots):
_index_copilot_dirs(con, copilot_roots, stats)
if codex_dir is not None and Path(codex_dir).is_dir():
_index_codex_root(con, Path(codex_dir), stats)
vscode_dirs = (
[Path(d).expanduser() for d in vscode_chat_dirs]
if vscode_chat_dirs is not None
else []
)
if any(d.is_dir() for d in vscode_dirs):
_index_vscode_chat_dirs(con, vscode_dirs, stats)

@devin-ai-integration devin-ai-integration Bot Oct 15, 2026 •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 Disabled sources lose indexed history

When build_index receives a disabled source, it deletes that source's existing docs instead of leaving them untouched. The CLI passes None for each --no-* flag, so later queries lose previously indexed history.

Learn more

A source argument of None means the caller did not select that collector. The CLI source selection also uses None for explicit --no-* flags, while empty default directory lists mean auto-detection found no roots. The new branches call _drop_vanished_files with an empty present set for both situations. That removes all documents and watermarks for the source, even if its underlying stores still exist. The same behavior applies to Copilot, Codex and VS Code.

Example: Index Copilot and Codex into one database. Run devin-search index --sessions-db sessions.db --no-copilot --no-codex against it. Both sources' search hits disappear although neither store changed.

Recommended fix: Distinguish a disabled source (None) from an enabled source with no existing roots ([] or a missing explicit path). Call _drop_vanished_files only for enabled sources whose roots truly vanished. Preserve existing docs and markers for disabled sources; cover both transitions with tests.

Devin Review


Was this helpful? React with 👍 or 👎 to provide feedback.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 5d6db3d. build_index now removes stale docs and markers for a configured source whose roots all vanished, even though its collector is skipped. Covered by test_vanished_source_root_drops_stale_docs.

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 1 new potential issue.

Devin Review

Comment on lines +1208 to +1209
except (json.JSONDecodeError, RecursionError):
bad_lines += 1 # truncated tail or malformed mid-line

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔍 Deep JSON recovery lacks regression tests

No new test exercises RecursionError from JSON parsing, despite handlers across the collectors. Add deep-JSON fixtures to cover the recovery paths.

Devin Review


Was this helpful? React with 👍 or 👎 to provide feedback.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in 426dcf0 — three fixtures now drive json.loads past the recursion limit (200k-deep JSON as a copilot JSONL line, a codex rollout line, and vscode snapshot + mutations files) and assert sibling records still index. test_copilot_deep_json_line_skipped, test_codex_deep_rollout_line_skipped, test_vscode_deep_json_files_skipped.

Icaro0310 added a commit that referenced this pull request Oct 16, 2026
The previous fix made deep-JSON records fail soft, but no fixture drove
json.loads past the recursion limit, so the recovery paths were
untested (devin-review analysis on #83). New tests inject a 200k-deep
JSON value as a copilot JSONL line, a codex rollout line and vscode
snapshot/mutations files, and assert sibling records still index.

Generated with [Devin](https://devin.ai) & QA Engineer Icaro Galvao

Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Icaro0310 added a commit that referenced this pull request Oct 22, 2026
Rebase-port of the stale #83 branch: main landed the collector rewrite
(#82), content digests and --tool while the PR was open, so its net
delta was re-extracted rather than merged literally.

- _vscode_field_text is iterative — deeply nested content fields no
  longer RecursionError the index run
- json.loads RecursionError treated as JSONDecodeError across vscode
  snapshot/mutations, copilot and codex rollout parsing
- copilot non-string `type` records skipped instead of TypeError
- codex threads docs re-index on cwd/created_at metadata changes
- managed-but-gone copilot/codex/vscode roots sweep stale docs —
  completes the vanished-source contract documented in 0.2.1
  (`--no-<surface>` still preserves: None means unmanaged, not vanished)

Generated with [Devin](https://devin.ai) & QA Engineer Icaro Galvao

Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
@Icaro0310
Icaro0310 force-pushed the feat/multi-source-collectors branch from 426dcf0 to 5151280 Compare October 22, 2026 07:42
github-actions[bot]
github-actions Bot previously approved these changes Oct 22, 2026
Rebase-port of the stale #83 branch: main landed the collector rewrite
(#82), content digests and --tool while the PR was open, so its net
delta was re-extracted rather than merged literally.

- _vscode_field_text is iterative — deeply nested content fields no
  longer RecursionError the index run
- json.loads RecursionError treated as JSONDecodeError across vscode
  snapshot/mutations, copilot and codex rollout parsing
- copilot non-string `type` records skipped instead of TypeError
- codex threads docs re-index on cwd/created_at metadata changes
- managed-but-gone copilot/codex/vscode roots sweep stale docs —
  completes the vanished-source contract documented in 0.2.1
  (`--no-<surface>` still preserves: None means unmanaged, not vanished)

Generated with [Devin](https://devin.ai) & QA Engineer Icaro Galvao

Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
github-actions[bot]
github-actions Bot previously approved these changes Oct 22, 2026
json.loads of a 5000-deep doc RecursionErrors on 3.11 before the walk
ever runs, so the file-level variant could only pass on 3.12+. The
unit-level call tests the iterative fix on every supported Python; the
deep-JSON file path is covered by test_vscode_deep_json_files_skipped.

Generated with [Devin](https://devin.ai) & QA Engineer Icaro Galvao

Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
@Icaro0310
Icaro0310 merged commit 06ed92a into main Oct 22, 2026
12 checks passed
@Icaro0310
Icaro0310 deleted the feat/multi-source-collectors branch October 22, 2026 08:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant