feat(semantic): [merge candidate build] FTS5 index and search tools, provider-aware typed embeddings, reranking, diagnostics, and eval harness

[Zireael]

Review Comment Resolution — 2026-07-25

All 26 outstanding review comments from PR #87 have been addressed. See replies on each thread for details.

Code fixes committed

Commit	Scope
53137af	Fork-specific housekeeping (degraded-grep docs, test env gate, LoggerMockFn type fix)
13d1309	P1: Reranker index dedup at 5 return sites, shell injection prevention in zir-aft-check.sh, autocrlf save/restore with chained trap
ad55bcf	P2: Increment suppressed_count in WarningDedup, fork safety comment in child_registry test
345498b	Fix missing mut on indices declaration
4bf6701	Remove unused stripJsoncSymbols import

Already addressed in codebase (no change needed)

ID	File	Issue	Resolution
3343597517	.gitignore	Inconsistent omo/ pattern	Already .omo/
3343613914	semantic_rerank.rs	Markdown fence stripping	strip_markdown_fences called in production
3343666968	config.ts	Missing rerank fields	All fields declared in schema
3343909788	config.ts	Enum backwards compat	Current values correct
3357767727	configure.rs	SSRF on rerank_base_url	validate_base_url_no_ssrf called
3359137353	configure.rs	jsonl_path validation	normalize_absolute_path + ParentDir check
3359137362	configure.rs	u64→u32 lossy cast	Uses u32::try_from
3370722719	paths.ts	Migration drops state	repairRootScopedStorageFile handles it
3370722720	child_registry.rs	/proc/stat parsing	Uses ps, not /proc
3378028776	semantic_eval.rs	Per-case top_k ignored	case.top_k.unwrap_or(params.top_k) used

Design intent / not actionable

ID	File	Issue	Rationale
3343667094	semantic_search.rs	Index status overwrite	status vs semantic_status intentionally separate
3370722725	semantic_search.rs	Lexical ordering	Design-intent clamped scores
3370733787	semantic_search.rs	Reranker warning gating	diagnostics_enabled vs output_mode serve different purposes
3370722736	helpers.rs	panic! in tests	No skip-mechanism panic found
3378028773	configure.rs	Sensitive logging	Only logs placeholder name, never template content

Docker validation

• cargo fmt ✅ | cargo check ✅ | cargo clippy ✅ | cargo nextest ✅

[cubic-dev-ai]

4 issues found across 107 files

<sub>Note: This PR contains a large number of files. cubic only reviews up to 100 files per PR, so some files may not have been reviewed. cubic prioritizes the most important files to review.
On a pro plan you can use ultrareview for larger PRs.

Re-trigger cubic</sub>

[cubic-dev-ai]

1 issue found across 69 files (changes from recent commits).

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name=".gitignore">

<violation number="1" location=".gitignore:95">
P2: Inconsistent .gitignore pattern: `omo/` should likely be `.omo/` to match the hidden tooling directory convention used by all other entries in this block.</violation>
</file>

<sub>Reply with feedback, questions, or to request a fix.

Re-trigger cubic</sub>

[greptile-apps]

Too many files changed for review. (286 files found, 100 file limit)

Bypass the limit by tagging @greptile-apps to review.

[cubic-dev-ai]

5 issues found across 10 files (changes from recent commits).

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="crates/aft/src/local_embed.rs">

<violation number="1">
P2: Test `resolve_model_files_downloads_and_reuses_cache` now silently passes as `ok` without doing any real work when `AFT_TEST_ONLINE` is not set. Previously `#[ignore]` made it explicitly visible as 'ignored' in test output. The `eprintln` skip message is captured by the test framework and invisible without `--nocapture`, so developers running `cargo test` see a passing test that exercised nothing — which is misleading and inflates coverage signal.

Suggestion: keep `#[ignore]` to preserve explicit 'ignored' status, and add the env-var gate as a secondary runtime check for those who use `--ignored` but need to avoid network.</violation>
</file>

<file name="packages/pi-plugin/src/config.ts">

<violation number="1" location="packages/pi-plugin/src/config.ts:379">
P1: A user-level `bash: false` is no longer able to disable the AFT bash surface when a project config is present: the shared merge expands the boolean into an object, and `resolveBashConfig` treats that object as enabled. Preserving an explicit boolean when only one side is configured (especially `false`) would retain the documented disable behavior.</violation>
</file>

<file name="crates/aft/src/model2vec_download.rs">

<violation number="1">
P2: Cache entries containing `--` in a repo component can pass this validation but be listed under a different `repo_id`, making cache output misleading and leaving the encoding ambiguous. An unambiguous cache-key encoding (or an explicit rejection of ambiguous names) would make the round-trip guarantee in this function true.</violation>
</file>

<file name="packages/opencode-plugin/src/config.ts">

<violation number="1" location="packages/opencode-plugin/src/config.ts:22">
P2: OpenCode subagents can no longer opt into background bash: the imported shared resolver defaults to `subagentAware: false`, so `subagent_background` is always undefined and `bash.ts` forces the synchronous path. OpenCode should use a subagent-aware wrapper/call (`resolveBashConfig(config, { subagentAware: true })`) while keeping the Pi resolver default false.</violation>
</file>

<file name="packages/aft-bridge/src/config-resolver.ts">

<violation number="1" location="packages/aft-bridge/src/config-resolver.ts:512">
P2: Configs that already have top-level `bash` lose supported `experimental.bash.long_running_reminder_*` settings during migration. The conflict path could merge those reminder fields into top-level `bash` before deleting the legacy block, matching `resolveBashConfig`'s pre-migration fallback behavior.</violation>
</file>

<sub>Tip: Review your code locally with the cubic CLI to iterate faster.

Re-trigger cubic</sub>

[cubic-dev-ai]

P1: A user-level bash: false is no longer able to disable the AFT bash surface when a project config is present: the shared merge expands the boolean into an object, and resolveBashConfig treats that object as enabled. Preserving an explicit boolean when only one side is configured (especially false) would retain the documented disable behavior.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At packages/pi-plugin/src/config.ts, line 379:

<comment>A user-level `bash: false` is no longer able to disable the AFT bash surface when a project config is present: the shared merge expands the boolean into an object, and `resolveBashConfig` treats that object as enabled. Preserving an explicit boolean when only one side is configured (especially `false`) would retain the documented disable behavior.</comment>

<file context>
@@ -1179,22 +370,18 @@ function mergeConfigs(base: AftConfig, override: AftConfig): AftConfig {
+    base.experimental,
+    override.experimental,
+  ) as AftConfig["experimental"];
+  const bash = mergeBashConfig(base.bash, override.bash) as AftConfig["bash"];
+  const inspect = mergeInspectConfig(base.inspect, override.inspect) as AftConfig["inspect"];
   const bridge = base.bridge;
</file context>

[cubic-dev-ai]

P2: Cache entries containing -- in a repo component can pass this validation but be listed under a different repo_id, making cache output misleading and leaving the encoding ambiguous. An unambiguous cache-key encoding (or an explicit rejection of ambiguous names) would make the round-trip guarantee in this function true.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At crates/aft/src/model2vec_download.rs, line 265:

<comment>Cache entries containing `--` in a repo component can pass this validation but be listed under a different `repo_id`, making cache output misleading and leaving the encoding ambiguous. An unambiguous cache-key encoding (or an explicit rejection of ambiguous names) would make the round-trip guarantee in this function true.</comment>

<file context>
@@ -253,6 +258,13 @@ pub fn list_cached_models() -> Vec<(String, PathBuf, u64)> {
+                // validation.  A manually-created directory like "evil"
+                // (no slash) would pass the cache check but then confuse
+                // downstream commands that expect a valid repo_id.
+                if crate::repo_id::split_hf_repo_id(&name).is_err() {
+                    continue;
+                }
</file context>

[cubic-dev-ai]

P2: OpenCode subagents can no longer opt into background bash: the imported shared resolver defaults to subagentAware: false, so subagent_background is always undefined and bash.ts forces the synchronous path. OpenCode should use a subagent-aware wrapper/call (resolveBashConfig(config, { subagentAware: true })) while keeping the Pi resolver default false.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At packages/opencode-plugin/src/config.ts, line 22:

<comment>OpenCode subagents can no longer opt into background bash: the imported shared resolver defaults to `subagentAware: false`, so `subagent_background` is always undefined and `bash.ts` forces the synchronous path. OpenCode should use a subagent-aware wrapper/call (`resolveBashConfig(config, { subagentAware: true })`) while keeping the Pi resolver default false.</comment>

<file context>
@@ -1,11 +1,48 @@
+  migrateAftConfigFile,
+  migrateRawConfig,
+  type ResolvedBashConfig,
+  resolveBashConfig,
+  resolveExperimentalConfigForConfigure,
+  resolveLspConfigForConfigure,
</file context>

[cubic-dev-ai]

P2: Configs that already have top-level bash lose supported experimental.bash.long_running_reminder_* settings during migration. The conflict path could merge those reminder fields into top-level bash before deleting the legacy block, matching resolveBashConfig's pre-migration fallback behavior.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At packages/aft-bridge/src/config-resolver.ts, line 512:

<comment>Configs that already have top-level `bash` lose supported `experimental.bash.long_running_reminder_*` settings during migration. The conflict path could merge those reminder fields into top-level `bash` before deleting the legacy block, matching `resolveBashConfig`'s pre-migration fallback behavior.</comment>

<file context>
@@ -0,0 +1,764 @@
+
+  if (Object.hasOwn(rawConfig, "bash")) {
+    logger?.warn(
+      `Config migration conflict at ${configPath}: experimental.bash dropped because top-level "bash" is already set`,
+    );
+  } else {
</file context>