diff --git a/packages/darnit/src/darnit/sieve/builtin_handlers.py b/packages/darnit/src/darnit/sieve/builtin_handlers.py index 402065ad..ba6cadd8 100644 --- a/packages/darnit/src/darnit/sieve/builtin_handlers.py +++ b/packages/darnit/src/darnit/sieve/builtin_handlers.py @@ -327,6 +327,10 @@ def regex_handler(config: dict[str, Any], context: HandlerContext) -> HandlerRes files_checked: int - Number of files examined results: list[dict] - Per-file match details patterns_checked: list[str] - Pattern names checked + resolved_files: list[str] - (match mode) absolute paths of files + that existed on disk and were scanned. A downstream + ``llm_eval`` pass automatically falls back to this list when + ``files_to_include`` produces no content (issue #402). files_found: int - (exclude mode) number of files matching globs found_files: list[str] - (exclude mode) matched file paths """ @@ -569,6 +573,13 @@ def _regex_match_files( "patterns_checked": list(patterns.keys()), "results": all_results[:20], "any_match": any_match, + # Issue #402 Option 2: paths of files that actually existed on + # disk and were scanned. A downstream `llm_eval` pass can fall + # back to this list when `$FOUND_FILE` is empty (e.g. + # `pattern -> llm_eval` shapes with no `file_exists` sibling), + # avoiding the empty-file_contents bug documented in #402. Absolute + # paths -- llm_eval accepts either shape (see llm_eval_handler). + "resolved_files": list(file_paths), } if pass_if_any: @@ -611,6 +622,23 @@ def llm_eval_handler(config: dict[str, Any], context: HandlerContext) -> Handler prompt: str - Prompt for LLM evaluation confidence_threshold: float - Minimum confidence to accept (default: 0.8) analysis_hints: list[str] - Hints for the LLM + files_to_include: list[str] - Files whose contents to bundle with + the consultation. Supports: + - literal paths (relative to repo root or absolute) + - ``"$FOUND_FILE"`` -> `gathered_evidence["found_file"]` (from + a preceding `file_exists` PASS) + - ``"$RESOLVED_FILES"`` -> `gathered_evidence["resolved_files"]` + (list from a preceding `regex`/`pattern` pass). Fans out to + multiple candidates from a single sentinel (issue #402). + Missing files are silently skipped. Capped at 5 file contents + total; each capped at 10000 bytes. + + When `files_to_include` produces no file contents at all + (empty $FOUND_FILE, missing literal paths, and no + $RESOLVED_FILES sentinel), the handler automatically falls + back to `gathered_evidence["resolved_files"]` so a + `pattern -> llm_eval` shape without a `file_exists` sibling + never ships an empty consultation (issue #402). Note: This handler returns INCONCLUSIVE with a consultation request in the details, since actual LLM invocation happens at the MCP server level. @@ -622,15 +650,17 @@ def llm_eval_handler(config: dict[str, Any], context: HandlerContext) -> Handler message="No prompt specified for LLM evaluation", ) - # Resolve files_to_include: read file contents for LLM context + # Resolve files_to_include: read file contents for LLM context. + # Supported sentinels: + # $FOUND_FILE -> gathered_evidence["found_file"] (from file_exists) + # $RESOLVED_FILES -> gathered_evidence["resolved_files"] (from regex/pattern; issue #402) + # Literal paths are opened directly; missing files are silently skipped. files_to_include = config.get("files_to_include", []) file_contents: dict[str, str] = {} - for f in files_to_include[:5]: - resolved = f - if f == "$FOUND_FILE": - resolved = context.gathered_evidence.get("found_file", "") - if not resolved: - continue + + def _read(resolved: str) -> None: + if not resolved or len(file_contents) >= 5: + return full = os.path.join(context.local_path, resolved) if not os.path.isabs(resolved) else resolved try: with open(full, encoding="utf-8", errors="ignore") as fh: @@ -639,6 +669,24 @@ def llm_eval_handler(config: dict[str, Any], context: HandlerContext) -> Handler except OSError: pass + for f in files_to_include[:5]: + if f == "$FOUND_FILE": + _read(context.gathered_evidence.get("found_file", "")) + elif f == "$RESOLVED_FILES": + for candidate in context.gathered_evidence.get("resolved_files", []) or []: + _read(candidate) + else: + _read(f) + + # Issue #402 Option 2: automatic fallback for TOMLs that still ship + # `files_to_include = ["$FOUND_FILE"]` and no `file_exists` sibling. + # If nothing above resolved to real content, try `resolved_files` from + # the preceding regex/pattern pass. Preserves single-source-of-truth + # for the file list (the sibling pattern already declares it). + if not file_contents: + for candidate in context.gathered_evidence.get("resolved_files", []) or []: + _read(candidate) + return HandlerResult( status=HandlerResultStatus.INCONCLUSIVE, message="LLM consultation requested", diff --git a/tests/darnit/sieve/test_builtin_handlers.py b/tests/darnit/sieve/test_builtin_handlers.py index 03f287da..bc553c07 100644 --- a/tests/darnit/sieve/test_builtin_handlers.py +++ b/tests/darnit/sieve/test_builtin_handlers.py @@ -574,6 +574,31 @@ def test_recursive_glob(self, tmp_path, ctx): ) assert result.status == HandlerResultStatus.PASS + # ------------------------------------------------------------------------- + # Issue #402 Option 2: resolved_files evidence for llm_eval fallback + # ------------------------------------------------------------------------- + + def test_evidence_includes_resolved_files_on_match_path(self, tmp_path, ctx): + """`resolved_files` MUST list every path that existed on disk and was scanned.""" + readme = tmp_path / "README.md" + readme.write_text("hello world") + security = tmp_path / "SECURITY.md" + security.write_text("no matches here for the sentinel") + + result = regex_handler( + { + "files": ["README.md", "SECURITY.md", "MISSING.md"], + "pattern": r"hello", + }, + ctx, + ) + # MISSING.md must NOT appear -- the field is "existed on disk and was scanned". + assert "resolved_files" in result.evidence + resolved = set(result.evidence["resolved_files"]) + assert str(readme) in resolved + assert str(security) in resolved + assert not any("MISSING.md" in p for p in resolved) + # ============================================================================= # regex_handler — depth-limited file discovery @@ -852,6 +877,84 @@ def test_files_to_include_empty_by_default(self, ctx): ) assert result.details["consultation_request"]["file_contents"] == {} + # ------------------------------------------------------------------------- + # Issue #402 Option 2: resolved_files sentinel + automatic fallback + # ------------------------------------------------------------------------- + + def test_resolved_files_sentinel_reads_all_evidence_paths(self, ctx, tmp_path): + """`$RESOLVED_FILES` expands to every path in `gathered_evidence["resolved_files"]`.""" + for name in ("README.md", "SECURITY.md", "CONTRIBUTING.md"): + (tmp_path / name).write_text(f"# {name}\ncontent for {name}\n") + ctx.gathered_evidence["resolved_files"] = [ + str(tmp_path / "README.md"), + str(tmp_path / "SECURITY.md"), + str(tmp_path / "CONTRIBUTING.md"), + ] + + result = llm_eval_handler( + {"prompt": "Evaluate docs", "files_to_include": ["$RESOLVED_FILES"]}, + ctx, + ) + fc = result.details["consultation_request"]["file_contents"] + assert set(fc.keys()) == {"README.md", "SECURITY.md", "CONTRIBUTING.md"} + assert "content for README.md" in fc["README.md"] + + def test_resolved_files_fallback_when_files_to_include_yields_empty(self, ctx, tmp_path): + """The core #402 fix: `["$FOUND_FILE"]` with no found_file falls back to `resolved_files`.""" + readme = tmp_path / "README.rst" + readme.write_text("Sphinx-style rst readme content", encoding="utf-8") + # No `found_file` in gathered_evidence -- the failure mode from #402. + ctx.gathered_evidence["resolved_files"] = [str(readme)] + + result = llm_eval_handler( + {"prompt": "Evaluate README", "files_to_include": ["$FOUND_FILE"]}, + ctx, + ) + fc = result.details["consultation_request"]["file_contents"] + # Before the fix: {} (empty consultation). + # After the fix: README.rst content shipped via automatic fallback. + assert "README.rst" in fc + assert "Sphinx-style" in fc["README.rst"] + + def test_no_fallback_when_files_to_include_already_produced_content(self, ctx, tmp_path): + """Fallback fires only when file_contents is empty. Explicit paths win.""" + readme = tmp_path / "README.md" + readme.write_text("explicit content") + rst_readme = tmp_path / "README.rst" + rst_readme.write_text("rst content that must NOT leak in") + ctx.gathered_evidence["resolved_files"] = [str(rst_readme)] + + result = llm_eval_handler( + {"prompt": "Evaluate", "files_to_include": ["README.md"]}, + ctx, + ) + fc = result.details["consultation_request"]["file_contents"] + assert set(fc.keys()) == {"README.md"} + assert "rst content" not in fc["README.md"] + + def test_fallback_respects_five_file_cap(self, ctx, tmp_path): + """`resolved_files` with >5 entries is capped at 5 in the fallback path.""" + paths = [] + for i in range(8): + p = tmp_path / f"f{i}.md" + p.write_text(f"content {i}") + paths.append(str(p)) + ctx.gathered_evidence["resolved_files"] = paths + + result = llm_eval_handler( + {"prompt": "Evaluate"}, # no files_to_include -> fallback fires + ctx, + ) + assert len(result.details["consultation_request"]["file_contents"]) == 5 + + def test_no_fallback_when_gathered_evidence_has_no_resolved_files(self, ctx): + """Absent `resolved_files` is a no-op, not an error.""" + result = llm_eval_handler( + {"prompt": "Evaluate", "files_to_include": ["$FOUND_FILE"]}, + ctx, + ) + assert result.details["consultation_request"]["file_contents"] == {} + # ============================================================================= # manual_steps_handler