Skip to content

Fix target-scoped single-agent tool validation#145

Open
rockyzl wants to merge 1 commit into
argonne-lcf:mainfrom
rockyzl:fix/target-scoped-tool-validation
Open

Fix target-scoped single-agent tool validation#145
rockyzl wants to merge 1 commit into
argonne-lcf:mainfrom
rockyzl:fix/target-scoped-tool-validation

Conversation

@rockyzl

@rockyzl rockyzl commented Jul 10, 2026

Copy link
Copy Markdown

Summary

  • Add a deterministic guard before the single-agent ToolNode executes run_ase.
  • Block follow-up tool calls that reuse a coordinate file from a previous molecule target.
  • Feed the agent a ToolMessage repair instruction so it can generate/reuse coordinates for the current target instead of running on stale artifacts.
  • Cover the stale-target case with routing tests.

Why

This addresses the smallest validation/routing slice discussed in #135: a follow-up such as glucose dipolemoment should not execute run_ase on a coordinate file produced earlier for another molecule, such as methane.

Demo scenario

Run the Streamlit UI:

streamlit run src/ui/app.py

Then use a single chat session:

  1. Ask for a property that creates a coordinate artifact for one molecule, for example:

    What is the IR spectrum of methane?
    
  2. Follow up with a different molecule target:

    glucose dipolemoment
    
  3. The routing risk is when the model tries to call run_ase with the old file, for example:

    {
      "name": "run_ase",
      "args": {
        "params": {
          "input_structure_file": "/tmp/methane.xyz",
          "driver": "dipole"
        }
      }
    }

Without this guard, that stale tool call can reach ToolNode. After this PR, it is blocked and the agent receives feedback to resolve/generate coordinates for the current target, glucose, before calling run_ase.

sequenceDiagram
    participant User
    participant Agent
    participant Validator
    participant ToolNode

    User->>Agent: What is the IR spectrum of methane?
    Agent->>ToolNode: smiles_to_coordinate_file -> /tmp/methane.xyz
    User->>Agent: glucose dipolemoment
    Agent->>Validator: run_ase(input_structure_file=/tmp/methane.xyz)
    Validator-->>Agent: Blocked: current target is glucose, file belongs to methane
    Agent->>ToolNode: generate/reuse glucose coordinates, then run_ase on glucose
Loading

The focused test is test_route_tools_blocks_stale_target_coordinate_reuse in tests/test_single_agent_routing.py.

Validation

  • git diff --check
  • PYTHONPATH=src python -m py_compile src/chemgraph/utils/tool_validation.py src/chemgraph/graphs/single_agent.py
  • PYTHONPATH=src python -m pytest tests/test_single_agent_routing.py -q -> 6 passed
  • PYTHONPATH=src python -m pytest tests/test_single_agent_routing.py tests/test_graphs.py tests/test_llm_agent.py tests/test_ui_main_interface.py -q -> 37 passed
  • PYTHONPATH=src python -m pytest -q -> 168 passed, 6 skipped

Refs #135

@rockyzl

rockyzl commented Jul 17, 2026

Copy link
Copy Markdown
Author

Hi @keceli and @tdpham2 — when you have a chance, could you please review this focused PR? It is ready and mergeable. The GitHub Actions runs from my fork are currently awaiting maintainer approval. Thank you!

@keceli

keceli commented Jul 19, 2026

Copy link
Copy Markdown
Contributor

Thank you @rockyzl for the PR. This is a tricky failure mode, and it is great that you went for a deterministic guardrail with real routing tests. The graph wiring reads cleanly.

My one bigger-picture worry is the design: the decision rests on re-parsing the free-text query, guessing the molecule from the filename, and string-matching the two. Even once polished, that base tends to drift, e.g. "dipole moment of the water molecule" gives target "water molecule" ≠ species "water", and the default molecule.xyz resolves to a generic "molecule". Each miss usually becomes another special case on the same guessing layer. The sturdier version is probably to track the active target molecule as explicit graph state (set when a molecule is resolved) and give coordinate files provenance, so run_ase checks identity structurally rather than from strings.

One thing I'd fix regardless: when a blocked run_ase shares a message with other parallel tool calls, only the blocked id gets a ToolMessage, so the siblings go unanswered and the next turn can "400 Bad Request". Worth handling all tool_call_ids.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants