Codebase Review

DeepDelve Codebase Review & Critique

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

Executive summary

DeepDelve is a three tier multi agent deep research assistant that improves significantly on its predecessor. Its main architectural strengths are per attempt quota top up, which structurally resolves the retry quota starvation bug; auto fetch fusion, which combines search and fetch so the LLM can't shortcut to just reading snippets; artifact quarantine, which renames flawed drafts aside so the model can't re condition on its own mistakes; and deterministic URL grounding, a centralized way of tracking which URLs were actually fetched via fetch_url_to_workspace, rather than trusting the model's own narration about what it did.

That said, a strict code audit turned up several latent design flaws, thread safety violations, and broken fallbacks that get in the way of production readiness.

Root cause of the salvage fallback defect

One of the issues tracked in the roadmap is that salvage doesn't fire on a second quarantine. Here's what's actually going on.

The mechanics of the defect. The salvage fallback, _salvage_narrated_report, is meant to capture reports that the model narrates as chat text but never actually writes through the write_workspace_file tool. The current implementation is flawed because of recency bias combined with per turn state resets. In CLI mode (run_cli in tui.py), the text accumulator gets reset at the start of every iteration of the execution loop:

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

In TUI mode (run_agent, same file, around line 853), state["current_msg"] gets explicitly cleared to None whenever a function call or result is processed:

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

Tracing the failure mode (Academic Eval Item 3). Here's how this actually played out in a real session log, session_ffe5dbc7-4730-4093-98e3-8d51d4dbb0ac.json. On attempt 1, the required artifact final_report.md was missing, so the system nudged the model and returned should_continue = True. On attempt 2, 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. On attempt 3, the model delegated research, got real results back, and then narrated the entire correct report as chat text, including a Python snippet showing how it would write the file, but never actually invoked write_workspace_file. Since the tool was never called, final_report.md was still missing, so problem got set to "missing_artifact". Because the attempt counter was still under MAX_COMPLETION_CHECK_ATTEMPTS of 3, the completion check returned True again, advanced the attempt counter to 4, and told the model this was its final attempt. On attempt 4, the loop ran one last time, but the model, seeing the final warning, either produced nothing, errored out, or hit its token limit, so turn_text (or state["current_msg"]) for this last iteration ended up being an empty string. When the budget ran out, the system checked whether it should salvage the report, but it called _salvage_narrated_report(..., last_assistant_text=turn_text) with turn_text already empty. The long, highly accurate report the model had narrated back on attempt 3 was simply thrown away, because the system only ever checks the text of the immediate last turn, and that turn happened to be empty.

Why it's even worse in TUI mode. Salvage is structurally impossible there in almost all cases. Because state["current_msg"] gets cleared to None on any function call or result, if the model narrates a draft and then invokes any tool at all, even something like think_tool or write_todos, turn_msg becomes None before run_completion_check even runs. So last_assistant_text ends up as an empty string and salvage gets skipped entirely.

Critical code flaws and architectural critiques

Specialist separation is purely prompt based. Tiers 2 and 3 are architecturally separated in app.py into distinct sub agents, WebSearcher versus AcademicSearcher at tier 2, and DocumentAnalyzer versus DataAnalyzer at tier 3. But if you actually inspect the tool list definitions in app.py, WebSearcher and AcademicSearcher are configured with the exact same tools ([web_search, fetch_url_to_workspace, think_tool]), and so are DocumentAnalyzer and DataAnalyzer ([read_workspace_file, grep_workspace_file, think_tool]). The specialization is entirely prompt driven. There are no hard tool or network level constraints actually enforcing the separation, so if the Planner misroutes a task, or the specialist model drifts from its prompt, the specialization just fails with no system level guardrail to catch it. A reasonable fix would be giving AcademicSearcher its own literature specific tools, like an arXiv or Semantic Scholar API tool, while barring WebSearcher from using academic filters. The Analyzers could similarly get their own distinct parser utilities: structured JSON or CSV extraction tools for DataAnalyzer, and semantic summarization tools for DocumentAnalyzer.

Multi user concurrency and thread safety violations in tui.py. In tui.py, the session events and IDs are module level globals:

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

This design is highly stateful and not thread safe at all. It's fine for a single user CLI run, but if DeepDelve runs in web mode (python src/app.py --web, which serves the TUI via textual-serve), multiple users hitting the web app at the same time will share these exact same global session lists. User A's logs and prompts can leak into User B's UI stream, and the shared lists are vulnerable to concurrent write corruption, which can crash the engine with an IndexError or KeyError inside log_stream_content. The fix here is to refactor all of these session log lists and variables into instance attributes of the Textual App class, something like self._session_events, or manage them through task isolated contextvars instead.

The findings store in RunState is effectively dead. 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, ...})

This function is never called anywhere in the codebase. The agent framework runs and outputs content to the UI and to workspace files just fine, but the structured findings cache stays empty, since nothing ever populates it. That means the "Workflow as Knowledge" goal, reusing extracted facts to answer or verify future similar queries, is only half implemented. The fix is to have delegate_tasks parse the output of its sub agent runs and explicitly log each finding (URL plus summary block) into RunState via add_finding().

A capitalization stopword flaw in _extract_salient_terms. The content level grounding check extracts salient terms with a 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)

The stopword check here is a blunt instrument. If a multi word proper noun happens to start with a stopword, things like "The Python Programming Language," "Ayanami0730_deep_research_bench," or "A Multi Agent Framework," the entire phrase gets discarded, just because its first word matches _PROPER_NOUN_STOPWORDS (words like "The" or "A"). That leads to false negatives in term extraction, which means the grounding check can miss legitimate semantic overlaps. The fix is to strip leading stopwords from the matched phrase (or split and filter them) instead of throwing away the entire multi word match.

DuckDuckGo client thread safety risks. In web.py, the DDGS client is initialized lazily with 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()

The initialization itself is thread safe, but using _ddgs_client afterward isn't. Since delegate_tasks runs sub agent searches concurrently through asyncio.gather, multiple threads will end up calling client.text() or client.news() at the same time on that same DDGS instance. The duckduckgo_search library isn't officially documented as thread safe for concurrent calls on a single client instance, and high concurrency here can lead to HTTP connection pooling issues, session pollution, or interpreter lock contention. The fix is simple: instantiate DDGS() directly inside _do_search as a local, short lived object instead of reaching for a global singleton. It's extremely lightweight and perfectly safe to create fresh per search.

Actionable mitigation recommendations

Here's how to fix these without breaking compatibility or introducing new regressions.

Fix 1, robust history based salvage. Instead of relying on the single turn last_assistant_text parameter, the completion check should scan backward 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 would then update 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 the associated tracking dicts into part of the App instance in TUI mode, or use a thread local or context local variable for CLI runs, so every execution thread gets its own isolated state.

Fix 3, proper noun stopword extraction fix. Modify _extract_salient_terms to strip leading stopwords instead of discarding the whole phrase:

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))

A live CLI test case study

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

Ground truth, researched ahead of time. Colombia was eliminated by Switzerland in the round of 16. The match ended 0 to 0 after extra time, and Switzerland won the penalty shootout 4 to 3. This happened in June or July 2026, which is outside the training cutoff of standard local models.

The agent's execution timeline. On attempt 1, the factual lookup, the agent tried to answer from memory and claimed the qualifying format hadn't even been decided yet. The completion check flagged this as not_delegated and forced real research. On attempt 2, planning, the agent wrote a plan but still didn't delegate, and got flagged as not_delegated again. On attempt 3, research, the agent finally called delegate_tasks. A specialist ran a search, auto fetched a Sporting News page, and stored the correct facts in the plan (Switzerland, 0 to 0 draw, 4 to 3 on penalties), but it forgot to actually write the report, so it got flagged as missing_artifact. On attempt 4, synthesis and citation, the agent wrote final_report.md via write_workspace_file and correctly synthesized the facts (Switzerland, 0 to 0, round of 16). But then the grounding check failed: the agent had cited https://www.fifa.com/competition/worldcup/rounds/round-of-16/match/8759744, a URL it never actually fetched (it had only fetched the Sporting News page), and the strict URL grounding check caught the fabrication and flagged it as not_grounded. Since that was the final attempt, the run ended with "Retry budget exhausted with an unresolved issue (not_grounded). final_report.md exists but could NOT be fully verified this run."

What this tells us. Fact accuracy was 100 percent correct: the agent successfully gathered the right facts (Switzerland, round of 16, 0 to 0, 4 to 3 on penalties) from the web search and overcame its own lack of training data on the event. Citation reliability failed: the agent fabricated an official looking FIFA match link to make its report seem more authoritative, instead of citing the actual Sporting News page it had really fetched. The grounding check itself worked perfectly: it correctly identified the fabricated URL and refused to verify the report, which is exactly what the URL presence hard gate is designed to do. And there's a real budget distribution issue: the agent burned 2 of its 3 attempts on memory refusal and planning before it ever got around to delegating, which left it with no attempts left to fix the citation fabrication once that got flagged.