Codebase Review

DeepDelve Codebase Review & Critique

This document contains a comprehensive, strict review of the DeepDelve project. It evaluates the architecture, state management, grounding systems, and specific bug patterns found in the codebase.

---

1. Executive Summary

DeepDelve is a 3-tier multi-agent deep research assistant that improves significantly upon its predecessor. Its primary architectural strengths are:

• Per-Attempt Quota Top-Up: Resolves the retry quota starvation bug structurally.
• Auto-Fetch Fusion: Combines search and fetch to prevent LLMs from shortcutting to snippets.
• Artifact Quarantine: Renames flawed drafts aside to prevent the model from re-conditioning on its own mistakes.
• Deterministic URL Grounding: Centralized tracking of URLs fetched via fetch_url_to_workspace rather than trusting the model's narration.

However, a strict code audit has revealed several latent design flaws, thread-safety violations, and broken fallbacks that undermine its production readiness.

---

2. Deep-Dive: Root-Cause of the Salvage Fallback Defect

One of the key issues tracked in the roadmap is: "salvage doesn't fire on a second quarantine."

A. The Mechanics of the Defect

The salvage fallback (_salvage_narrated_report) is designed to capture reports that the model narrates in chat text but fails to write via the write_workspace_file tool. However, the current implementation is flawed due to recency bias and per-turn state resets:

1. The Turn Reset in CLI (run_cli in tui.py): At the start of each iteration in the execution loop, the text accumulator is reset:

   while has_requests:
       has_requests = False
       user_input_requests = []
       turn_text = ""  # <-- RESET HERE

2. The Turn Reset in TUI (run_agent in tui.py): In TUI mode, state["current_msg"] is explicitly cleared to None whenever a function call or result is processed:

   elif content.type == "function_result":
       ...
       state["current_msg"] = None  # <-- WIPE HERE

B. Trace of the Failure Mode (Academic Eval Item 3)

Let's trace how this happened in the actual session log session_ffe5dbc7-4730-4093-98e3-8d51d4dbb0ac.json:

• Attempt 1 (First Check): The required artifact final_report.md was missing. The system nudged the model, returning should_continue = True.
• Attempt 2 (Second Check): The agent called write_workspace_file, but the report cited ungrounded URLs (https://arxiv.org/html/2603.26718v2 and https://www.facebook.com/...). The grounding check failed, quarantined final_report.md as final_report.md.rejected_attempt_2, and nudged the model again.
• Attempt 3 (Third Check): The model delegated research, got results, and then narrated the entire correct report as chat text (including a python code snippet showing how to write the file, but without actually invoking the write_workspace_file tool).
• The System Check: Since the tool was not called, final_report.md was missing. problem was set to "missing_artifact".
• The Loop Transition: Since attempt = 2 (which is < MAX_COMPLETION_CHECK_ATTEMPTS of 3), the completion check returned True, advancing the attempt counter to 3 and instructing the model: SYSTEM WARNING: THIS IS YOUR FINAL ATTEMPT... Pushing agent to create it.
• Attempt 4 (Final Check): The loop executed one last time. But the model, seeing the final nudge, either produced nothing, errored out, or reached its token limit. Thus, turn_text (or state["current_msg"]) for this final iteration was "" (empty string).
• Budget Exhausted: The loop exited. The system checked if it should salvage the report. However, it called _salvage_narrated_report(..., last_assistant_text=turn_text) where turn_text was "".
• Conclusion: The long, highly-accurate report narrated in Attempt 3 was thrown away because the system only checked the text of the immediate last turn (Attempt 4), which was empty.

C. TUI-Specific Salvage Failure

In TUI mode, salvage is structurally impossible in almost all cases. Because state["current_msg"] is cleared to None on any function call or result, if the model narrates a draft and then invokes any tool (e.g., think_tool, write_todos), turn_msg becomes None before run_completion_check runs. Thus, last_assistant_text is passed as "" and salvage is skipped.

---

3. Critical Code Flaws & Architectural Critiques

Critique A: Purely Prompt-Based (Soft) Specialist Separation

Tiers 2 and 3 are architecturally separated in app.py into distinct sub-agents:

• Tier 2: WebSearcher vs. AcademicSearcher
• Tier 3: DocumentAnalyzer vs. DataAnalyzer

However, if you inspect the tool list definitions in app.py:

• Both WebSearcher and AcademicSearcher are configured with the exact same tools: [web_search, fetch_url_to_workspace, think_tool].
• Both DocumentAnalyzer and DataAnalyzer are configured with: [read_workspace_file, grep_workspace_file, think_tool].

Problem: The specialization is 100% prompt-driven; there are no hard tool or network-level constraints enforcing the separation. If the Planner misroutes a task or if the specialist model suffers from prompt drift, the specialization fails with no system-level guardrails.

Recommendation: The AcademicSearcher should have distinct, literature-specific tools (e.g., an ArXiv or Semantic Scholar API tool), while WebSearcher should be barred from using academic filters. Likewise, the Analyzers should have distinct parser utilities (e.g., JSON/CSV structured extraction tools for DataAnalyzer and semantic summarization tools for DocumentAnalyzer).

Critique B: Multi-User Concurrency & Thread-Safety Violations in tui.py

In tui.py, the session events and IDs are defined as global variables:

_session_events = []
_current_call_by_source = {}
_current_text_by_source = {}
_current_session_id = str(uuid.uuid4())

Problem: This design is highly stateful and completely thread-unsafe. While acceptable for a single-user CLI run, if DeepDelve is run in web mode (python src/app.py --web which serves the TUI using textual-serve), multiple users accessing the web application in parallel will share the same global session lists.

• User A's logs and prompts will leak into User B's UI stream.
• The shared lists will suffer from concurrent write corruption, causing the engine to crash with IndexError or KeyError inside log_stream_content.

Recommendation: All session log lists and variables must be refactored into instance attributes of the Textual App class (e.g., self._session_events) or managed via task-isolated contextvars.

Critique C: Dead findings Store in RunState

The RunState class in run_state.py defines:

def add_finding(self, source_url: str, summary: str) -> None:
    self.data["findings"].append({"source_url": source_url, "summary": summary, ...})

Problem: This function is never called anywhere in the codebase.

• The agent framework executes and outputs content to the UI and workspace files, but the structured findings cache remains empty.
• Because findings is never populated, the "Workflow as Knowledge" goal (reusing extracted facts to answer or verify future similar queries) is only half-implemented.

Recommendation: The output of sub-agent runs inside delegate_tasks should be parsed, and their findings (URL + summary block) should be explicitly logged into RunState via add_finding().

Critique D: Capitalization Stopword Flaw in _extract_salient_terms

The content-level grounding check extracts salient terms using regex in tui.py:

for m in re.finditer(r'\b[A-Z][a-zA-Z0-9]*(?:\s+[A-Z][a-zA-Z0-9]*){1,4}\b', text):
    phrase = m.group(0)
    if phrase.split()[0] not in _PROPER_NOUN_STOPWORDS:
        terms.add(phrase)

Problem: The stopword check is a blunt instrument. If a multi-word proper noun begins with a stopword (e.g., "The Python Programming Language", "Ayanami0730_deep_research_bench", or "A Multi-Agent Framework"), the entire phrase is discarded because the first word matches _PROPER_NOUN_STOPWORDS ("The", "A", etc.).

• This leads to false negatives in term extraction, causing the grounding check to miss legitimate semantic overlaps.

Recommendation: The code should strip leading stopwords from the matched phrase (or split and filter them) rather than discarding the entire multi-word match.

Critique E: DuckDuckGo Client Thread-Safety Risks

In web.py, the DDGS client is initialized lazily using a singleton pattern:

_ddgs_lock = threading.Lock()
_ddgs_client = None

def get_ddgs_client():
    global _ddgs_client
    with _ddgs_lock:
        if _ddgs_client is None:
            _ddgs_client = DDGS()

Problem: While the initialization is thread-safe, the usage of _ddgs_client is not. Since delegate_tasks runs sub-agent searches concurrently using asyncio.gather, multiple threads will invoke client.text() or client.news() concurrently on the same DDGS instance.

• The duckduckgo_search library is not officially documented as thread-safe for concurrent calls on a single client instance.
• High concurrency can lead to HTTP connection pooling issues, session pollution, or PyO3 interpreter lock contention.

Recommendation: Instantiate DDGS() directly inside _do_search as a local, short-lived object instead of using a global singleton. It is extremely lightweight and safe to create per-search.

---

4. Actionable Mitigation Recommendations

To resolve these issues without breaking compatibility or introducing regressions, we recommend the following specific code improvements:

Fix 1: Robust History-Based Salvage

Instead of relying on the single-turn last_assistant_text parameter, the completion check should look backwards through _session_events to find the most recent substantial text block generated by the primary Planner agent:

def _find_last_planner_text(events: list) -> str:
    # Scan backward for the most recent text block from the main agent
    for event in reversed(events):
        if (
            event.get("source") == "Agent" 
            and event.get("type") == "text" 
            and event.get("depth", 0) == 0
        ):
            text = event.get("data", {}).get("text", "")
            # Filter out system notifications that got appended to the log
            text_clean = re.sub(r'System \(\d/\d\):.*', '', text).strip()
            text_clean = re.sub(r'System \(final\):.*', '', text_clean).strip()
            if len(text_clean) >= 200:
                return text_clean
    return ""

In run_completion_check, the salvage condition can then be updated to:

elif problem == "missing_artifact":
    salvage_text = _find_last_planner_text(_session_events)
    if _salvage_narrated_report(req_artifact, salvage_text):
        # Notify success...

Fix 2: Thread-Safe Log Refactoring

Refactor _session_events and associated tracking dicts to be part of the App instance in TUI mode, or use a thread-local/context-local variable for CLI runs to guarantee isolated state per execution thread.

Fix 3: Proper Noun Stopword Extraction Fix

Modify _extract_salient_terms to strip leading stopwords instead of discarding:

for m in re.finditer(r'\b[A-Z][a-zA-Z0-9]*(?:\s+[A-Z][a-zA-Z0-9]*){1,4}\b', text):
    phrase = m.group(0)
    words = phrase.split()
    while words and words[0] in _PROPER_NOUN_STOPWORDS:
        words.pop(0)
    if words:
        terms.add(" ".join(words))

---

5. Live CLI Test Case Study & Comparison

We ran a live CLI test with the complex, time-sensitive query: "Who eliminated Colombia from the 2026 FIFA World Cup?"

A. Ground Truth (Pre-Researched)

• Result: Colombia was eliminated by Switzerland in the Round of 16.
• Match Details: The match ended in a 0-0 draw after extra time. Switzerland won 4-3 in the subsequent penalty shootout.
• Context: This occurred in June/July 2026, which is outside the training cutoff of standard local models.

B. Agent Execution Timeline

1. Attempt 1 (Factual Lookup): The agent attempted to answer from memory, claiming the qualifying format was not decided. The completion check flagged this as not_delegated and forced research.
2. Attempt 2 (Planning): The agent wrote a plan but still did not delegate. Flagged as not_delegated again.
3. Attempt 3 (Research): The agent finally called delegate_tasks. A specialist performed a search, auto-fetched a Sporting News page, and stored the correct facts in the plan (Switzerland, 0-0 draw, 4-3 PK shootout). However, it forgot to write the report. Flagged as missing_artifact.
4. Attempt 4 (Synthesis & Citation): The agent wrote final_report.md via write_workspace_file. It correctly synthesized the facts (Switzerland, 0-0 score, Stage: Round of 16).
5. The Grounding Check Failure: The agent included a citation to https://www.fifa.com/competition/worldcup/rounds/round-of-16/match/8759744. Since the agent never actually fetched this URL (it only fetched the Sporting News page), the strict URL grounding check caught the fabrication and flagged it as not_grounded.
6. Budget Exhaustion: Since it was the final attempt, the run ended with the status: "Retry budget exhausted with an unresolved issue (not_grounded). final_report.md exists but could NOT be fully verified this run."

C. Analysis & Comparison

• Fact Accuracy: 100% Correct. The agent successfully gathered the correct facts (Switzerland, Stage: Round of 16, 0-0, 4-3 PKs) from the web search and overcame its lack of training data.
• Citation Reliability: Failed. The agent fabricated an official FIFA match link (fifa.com/competition/worldcup...) to make its report look more authoritative, rather than citing the actual Sporting News page it fetched.
• Grounding Check Success: 100% Correct. The new grounding system successfully identified the fabricated URL and refused to verify the report, demonstrating that the URL-presence hard gate works exactly as designed to block hallucinated sources.
• Budget Distribution Issue: The agent wasted 2 of its 3 attempts on memory-refusal and planning before delegating, leaving it with no attempts remaining to fix the citation fabrication once flagged.