From 0c7a339fc8c9f9d88c011b541ac89008c46de996 Mon Sep 17 00:00:00 2001 From: Reo Sze <46519109+reowszer@users.noreply.github.com> Date: Wed, 8 Jul 2026 19:24:45 +0000 Subject: [PATCH 1/2] Fix relative path resolution in tool file readers Writers (smiles_to_coordinate_file, run_ase output) route relative paths through _resolve_path into CHEMGRAPH_LOG_DIR, but readers opened them from cwd -> FileNotFoundError on bare names like "water.xyz". Add _resolve_existing_path() and use it in run_ase, extract_output_json, file_to_atomsdata, generate_html. Prefers an existing path, else the log-dir location, else raw (so missing-file errors still surface). Add regression tests (EMT, no network). --- src/chemgraph/tools/ase_core.py | 44 ++++++- src/chemgraph/tools/ase_tools.py | 4 + src/chemgraph/tools/parsl_tools.py | 4 +- src/chemgraph/tools/report_tools.py | 8 ++ tests/test_ase_input_path_resolution.py | 162 ++++++++++++++++++++++++ 5 files changed, 220 insertions(+), 2 deletions(-) create mode 100644 tests/test_ase_input_path_resolution.py diff --git a/src/chemgraph/tools/ase_core.py b/src/chemgraph/tools/ase_core.py index b1c483e3..4010ff39 100644 --- a/src/chemgraph/tools/ase_core.py +++ b/src/chemgraph/tools/ase_core.py @@ -70,6 +70,41 @@ def _resolve_path(path: str) -> str: return path +def _resolve_existing_path(path: str) -> str: + """Resolve a path to read that a sibling tool may have written to the log dir. + + Tools that *write* files (``smiles_to_coordinate_file``, ``run_ase``'s + result JSON, ``save_atomsdata_to_file`` ...) send relative paths through + :func:`_resolve_path`, so a bare ``"water.xyz"`` lands in + ``CHEMGRAPH_LOG_DIR`` rather than the caller's cwd. A tool that later + *reads* that bare name must look in the same place, otherwise it raises + ``FileNotFoundError`` even though the file exists. + + This helper returns ``path`` unchanged when it already points at an + existing file (absolute paths and genuine cwd-relative paths keep working); + only when the raw path is missing does it fall back to the + ``CHEMGRAPH_LOG_DIR``-resolved location. The raw path is returned when + neither exists, so callers still surface a meaningful "not found" error. + + Parameters + ---------- + path : str + Absolute or relative file path to read. + + Returns + ------- + str + The raw path if it exists, else the log-dir-resolved path if that + exists, else the raw path unchanged. + """ + if os.path.isfile(path): + return path + resolved = _resolve_path(path) + if resolved != path and os.path.isfile(resolved): + return resolved + return path + + # --------------------------------------------------------------------------- # AtomsData <-> ASE Atoms conversions # --------------------------------------------------------------------------- @@ -366,7 +401,11 @@ def run_ase_core(params: ASEInputSchema) -> dict: start_time = time.time() - input_structure_file = params.input_structure_file + # Resolve a relative input path against CHEMGRAPH_LOG_DIR, matching how + # smiles_to_coordinate_file writes it. Without this, a tool that writes + # water.xyz into the session log dir and a later run_ase that reads + # "water.xyz" from cwd disagree -> FileNotFoundError. + input_structure_file = _resolve_existing_path(params.input_structure_file) output_results_file = _resolve_path(params.output_results_file) optimizer = params.optimizer fmax = params.fmax @@ -764,6 +803,9 @@ def extract_output_json_core(json_file: str) -> dict: json.JSONDecodeError If the file is not valid JSON. """ + # run_ase writes its result JSON via _resolve_path (into CHEMGRAPH_LOG_DIR), + # so a bare relative name passed here must resolve to the same place. + json_file = _resolve_existing_path(json_file) with open(json_file, "r", encoding="utf-8") as f: data = json.load(f) return data diff --git a/src/chemgraph/tools/ase_tools.py b/src/chemgraph/tools/ase_tools.py index 369c2753..5bcc21e3 100644 --- a/src/chemgraph/tools/ase_tools.py +++ b/src/chemgraph/tools/ase_tools.py @@ -15,6 +15,7 @@ from chemgraph.schemas.ase_input import ASEInputSchema from chemgraph.tools.ase_core import ( _resolve_path, + _resolve_existing_path, atoms_to_atomsdata, extract_output_json_core, run_ase_core, @@ -63,6 +64,9 @@ def file_to_atomsdata(fname: str) -> AtomsData: """ from ase.io import read + # A coordinate file written by smiles_to_coordinate_file/save_atomsdata_to_file + # via _resolve_path lands in CHEMGRAPH_LOG_DIR; resolve a bare name to match. + fname = _resolve_existing_path(fname) try: atoms = read(fname) return atoms_to_atomsdata(atoms) diff --git a/src/chemgraph/tools/parsl_tools.py b/src/chemgraph/tools/parsl_tools.py index cc7a51e0..65e09126 100644 --- a/src/chemgraph/tools/parsl_tools.py +++ b/src/chemgraph/tools/parsl_tools.py @@ -13,7 +13,7 @@ mace_input_schema, mace_output_schema, ) -from chemgraph.tools.ase_core import run_ase_core +from chemgraph.tools.ase_core import run_ase_core, _resolve_existing_path # Re-export schemas so existing ``from chemgraph.tools.parsl_tools import …`` # statements continue to work. @@ -85,6 +85,8 @@ def extract_output_json(json_file: str) -> dict: """Load simulation results from a JSON file produced by run_ase.""" import json + # Match run_ase's _resolve_path write location for bare relative names. + json_file = _resolve_existing_path(json_file) try: with open(json_file, "r") as f: ret = json.load(f) diff --git a/src/chemgraph/tools/report_tools.py b/src/chemgraph/tools/report_tools.py index 9761c1e6..06654a7c 100644 --- a/src/chemgraph/tools/report_tools.py +++ b/src/chemgraph/tools/report_tools.py @@ -7,6 +7,7 @@ from ase.data import chemical_symbols as _chemical_symbols from chemgraph.schemas.ase_input import ASEOutputSchema +from chemgraph.tools.ase_core import _resolve_existing_path from chemgraph.tools.ase_tools import is_linear_molecule @@ -341,6 +342,13 @@ def generate_html( str Path to the generated HTML file """ + # run_ase and the coordinate writers emit relative paths into + # CHEMGRAPH_LOG_DIR via _resolve_path; resolve bare names to match so a + # report can be built from files produced earlier in the same session. + results_json_path = _resolve_existing_path(results_json_path) + if xyz_path is not None: + xyz_path = _resolve_existing_path(xyz_path) + # Validate results_json_path exists if not os.path.isfile(results_json_path): return ( diff --git a/tests/test_ase_input_path_resolution.py b/tests/test_ase_input_path_resolution.py new file mode 100644 index 00000000..8e18257c --- /dev/null +++ b/tests/test_ase_input_path_resolution.py @@ -0,0 +1,162 @@ +"""Regression tests for input_structure_file path resolution in run_ase_core. + +Background +---------- +Coordinate files are written into the session log directory via +``_resolve_path`` (see ``ase_core._resolve_path``). When a tool writes +``water.xyz`` into ``CHEMGRAPH_LOG_DIR`` but ``run_ase`` is later invoked +with the BARE relative name ``"water.xyz"`` (and cwd is not the log dir), +``run_ase_core`` used to raise ``FileNotFoundError`` because it looked for +the file relative to cwd only. + +The fix (ase_core.py ~369-376) resolves a relative ``input_structure_file`` +against ``CHEMGRAPH_LOG_DIR`` when the raw path is not already an existing +file, so ``run_ase`` finds the structure the earlier tool wrote. + +These tests use the EMT calculator: it needs no model downloads and no +network, and handles water (H, O) fine, so the tests stay hermetic and fast. +""" + +import os + +import pytest + +from chemgraph.schemas.ase_input import ASEInputSchema +from chemgraph.schemas.calculators.emt_calc import EMTCalc +from chemgraph.tools.ase_core import ( + run_ase_core, + extract_output_json_core, + _resolve_existing_path, +) +from chemgraph.tools.ase_tools import file_to_atomsdata +from chemgraph.tools.cheminformatics_core import smiles_to_coordinate_file_core + + +@pytest.fixture +def log_dir(tmp_path, monkeypatch): + """Point CHEMGRAPH_LOG_DIR at a throwaway session dir (auto-restored).""" + monkeypatch.setenv("CHEMGRAPH_LOG_DIR", str(tmp_path)) + return tmp_path + + +def test_run_ase_resolves_bare_name_from_log_dir(log_dir, tmp_path, monkeypatch): + """A bare relative input_structure_file is found in CHEMGRAPH_LOG_DIR. + + This is the regression: before the fix, calling run_ase with the bare + name "water.xyz" (written into the log dir) while cwd is elsewhere + raised FileNotFoundError. + """ + # Write the structure into the log dir through the real write path + # (smiles_to_coordinate_file_core -> _resolve_path), using a bare name. + result = smiles_to_coordinate_file_core("O", output_file="water.xyz") + assert result["ok"] is True + written_path = result["path"] + # It must have landed inside the log dir, not cwd. + assert os.path.dirname(written_path) == str(log_dir) + assert os.path.isfile(written_path) + + # Run from a directory that is deliberately NOT the log dir, so the bare + # name cannot be found relative to cwd. + other_dir = tmp_path / "elsewhere" + other_dir.mkdir() + monkeypatch.chdir(other_dir) + assert not os.path.isfile("water.xyz") # bare name not resolvable via cwd + + schema = ASEInputSchema( + input_structure_file="water.xyz", + output_results_file="out.json", + driver="energy", + calculator=EMTCalc(), + ) + out = run_ase_core(schema) + + assert out["status"] == "success", out + assert out["single_point_energy"] is not None + # The output should have been written into the log dir too. + assert os.path.isfile(os.path.join(str(log_dir), "out.json")) + + +def test_run_ase_missing_file_still_fails(log_dir, tmp_path, monkeypatch): + """A genuinely missing input file still returns a FileNotFoundError dict. + + Guards against the fix masking real missing-file errors: no such file + exists anywhere (log dir or cwd), so run_ase must report failure. + """ + monkeypatch.chdir(tmp_path) + assert not os.path.isfile(os.path.join(str(log_dir), "does_not_exist.xyz")) + + schema = ASEInputSchema( + input_structure_file="does_not_exist.xyz", + output_results_file="out.json", + driver="energy", + calculator=EMTCalc(), + ) + out = run_ase_core(schema) + + assert out["status"] == "failure", out + assert out["error_type"] == "FileNotFoundError", out + + +def test_resolve_existing_path_prefers_existing_then_log_dir( + log_dir, tmp_path, monkeypatch +): + """_resolve_existing_path: cwd file wins, else log-dir file, else raw.""" + # 1. A real cwd-relative file is returned unchanged. + monkeypatch.chdir(tmp_path) + (tmp_path / "here.txt").write_text("x") + assert _resolve_existing_path("here.txt") == "here.txt" + + # 2. A bare name only present in the log dir resolves to the log dir. + (log_dir / "only_in_log.txt").write_text("y") + other = tmp_path / "cwd2" + other.mkdir() + monkeypatch.chdir(other) + resolved = _resolve_existing_path("only_in_log.txt") + assert resolved == os.path.join(str(log_dir), "only_in_log.txt") + assert os.path.isfile(resolved) + + # 3. A path that exists nowhere is returned unchanged (caller reports it). + assert _resolve_existing_path("nope.txt") == "nope.txt" + + +def test_extract_output_json_resolves_bare_name(log_dir, tmp_path, monkeypatch): + """extract_output_json_core finds a results JSON written into the log dir. + + Regression for the same asymmetry as run_ase: the agent calls + extract_output_json("out.json") by bare name after run_ase wrote it into + CHEMGRAPH_LOG_DIR. + """ + # Produce a result JSON in the log dir via the real run_ase write path. + cf = smiles_to_coordinate_file_core("O", output_file="water.xyz") + assert os.path.dirname(cf["path"]) == str(log_dir) + out = run_ase_core( + ASEInputSchema( + input_structure_file="water.xyz", + output_results_file="results.json", + driver="energy", + calculator=EMTCalc(), + ) + ) + assert out["status"] == "success", out + assert os.path.isfile(os.path.join(str(log_dir), "results.json")) + + # Read it back by BARE name from a different cwd. + other = tmp_path / "elsewhere2" + other.mkdir() + monkeypatch.chdir(other) + data = extract_output_json_core("results.json") + assert isinstance(data, dict) and data, data + + +def test_file_to_atomsdata_resolves_bare_name(log_dir, tmp_path, monkeypatch): + """file_to_atomsdata reads a coordinate file written into the log dir.""" + cf = smiles_to_coordinate_file_core("O", output_file="mol.xyz") + assert os.path.dirname(cf["path"]) == str(log_dir) + + other = tmp_path / "elsewhere3" + other.mkdir() + monkeypatch.chdir(other) + atoms = file_to_atomsdata("mol.xyz") # bare name; before fix -> FileNotFoundError + assert atoms is not None + # water has 3 atoms + assert len(atoms.numbers) == 3 From 68c434abc2e136d8fe778e06618163d5e771d160 Mon Sep 17 00:00:00 2001 From: Reo Sze <46519109+reowszer@users.noreply.github.com> Date: Wed, 15 Jul 2026 17:31:53 +0000 Subject: [PATCH 2/2] Resolve relative paths in MCP tool file readers Extends the CHEMGRAPH_LOG_DIR path resolution from the LangChain tool readers to the MCP servers and the graspa/xanes cores, so a small model that echoes back a bare filename ("water.xyz") for a file a sibling tool wrote into the log dir still resolves it instead of failing with FileNotFoundError. - execution/utils.resolve_structure_files: resolve each listed filename against the log dir (shared by the ase/mace/graspa/xanes _hpc ensemble tools). Directory inputs are left unchanged. - mcp/ase_mcp_hpc.py, mcp/mace_mcp_hpc.py: _embed_inline_if_local resolves the bare name on the submitting host before its local-vs-remote check and stores the resolved absolute path back into the job. Worker code unchanged. - tools/graspa_core.py, tools/xanes_core.py: resolve input_structure_file via _resolve_existing_path instead of Path(...).resolve(), so the single- structure _hpc path (which delegates here) also resolves bare names. The ase/mace cores already did this via run_ase_core. - mcp/data_analysis_mcp.py: aggregate_simulation_results resolves each input. - mcp/hpc_misc_mcp.py: inspect_json checks the log dir before its nearby-files fallback. - mcp/graspa_mcp_parsl.py, mcp/xanes_mcp_parsl.py: same fallback in their copied list-resolution logic (deprecated servers; included for consistency). The general server (mcp/mcp_tools.py) already delegates to the fixed core and returns absolute paths; added tests to pin that behavior. The _hpc worker functions are deliberately left untouched: when the backend shares the filesystem the worker delegates to the (now-fixed) cores, and when it does not the bare name is resolved on the submitting host before submission. --- src/chemgraph/execution/utils.py | 11 ++- src/chemgraph/mcp/ase_mcp_hpc.py | 17 +++- src/chemgraph/mcp/data_analysis_mcp.py | 7 ++ src/chemgraph/mcp/graspa_mcp_parsl.py | 6 +- src/chemgraph/mcp/hpc_misc_mcp.py | 9 +++ src/chemgraph/mcp/mace_mcp_hpc.py | 17 +++- src/chemgraph/mcp/xanes_mcp_parsl.py | 5 +- src/chemgraph/tools/graspa_core.py | 6 +- src/chemgraph/tools/xanes_core.py | 6 +- tests/test_graspa_tools.py | 40 ++++++++++ tests/test_mcp.py | 104 +++++++++++++++++++++++++ 11 files changed, 215 insertions(+), 13 deletions(-) diff --git a/src/chemgraph/execution/utils.py b/src/chemgraph/execution/utils.py index c7a0ed0b..885c2719 100644 --- a/src/chemgraph/execution/utils.py +++ b/src/chemgraph/execution/utils.py @@ -78,11 +78,20 @@ def resolve_structure_files( ValueError If no files are found or if listed files do not exist. """ + # A bare relative filename (e.g. "water.cif") from a small model refers to + # a file a sibling tool wrote into CHEMGRAPH_LOG_DIR, not the cwd. Resolve + # each listed name against the log dir before checking existence so those + # inputs still resolve; absolute/cwd paths are returned unchanged. The + # directory branch is intentionally left untouched: writer tools emit + # files (not directories) into the log dir, so there is no sibling-written + # directory to fall back to. + from chemgraph.tools.ase_core import _resolve_existing_path + structure_files: list[Path] = [] output_dir: Path = Path.cwd() if isinstance(input_source, list): - structure_files = [Path(p) for p in input_source] + structure_files = [Path(_resolve_existing_path(str(p))) for p in input_source] missing = [p for p in structure_files if not p.exists()] if missing: raise ValueError(f"The following input files are missing: {missing}") diff --git a/src/chemgraph/mcp/ase_mcp_hpc.py b/src/chemgraph/mcp/ase_mcp_hpc.py index a1cc5182..2a84e6ba 100644 --- a/src/chemgraph/mcp/ase_mcp_hpc.py +++ b/src/chemgraph/mcp/ase_mcp_hpc.py @@ -153,14 +153,23 @@ def _embed_inline_if_local(job: dict) -> None: if job.get("remote_structure_file") or job.get("inline_structure"): return input_file = job.get("input_structure_file") - if not input_file or not os.path.isfile(input_file): - return # remote path -- worker will read it directly + if not input_file: + return from ase.io import read as ase_read - from chemgraph.tools.ase_core import atoms_to_atomsdata + from chemgraph.tools.ase_core import _resolve_existing_path, atoms_to_atomsdata + + # A small model may echo back a bare name ("water.xyz") for a file a + # sibling tool wrote into CHEMGRAPH_LOG_DIR. Resolve it here, on the + # submitting host where the log dir lives, before deciding whether the + # input is local. Absolute/cwd paths are returned unchanged. + resolved = _resolve_existing_path(input_file) + if not os.path.isfile(resolved): + return # remote path -- worker will read it directly - atoms = ase_read(input_file) + job["input_structure_file"] = resolved + atoms = ase_read(resolved) job["inline_structure"] = atoms_to_atomsdata(atoms).model_dump() diff --git a/src/chemgraph/mcp/data_analysis_mcp.py b/src/chemgraph/mcp/data_analysis_mcp.py index 0b360ad7..fc928c38 100644 --- a/src/chemgraph/mcp/data_analysis_mcp.py +++ b/src/chemgraph/mcp/data_analysis_mcp.py @@ -112,12 +112,19 @@ def aggregate_simulation_results( str Human-readable success or error message. """ + from chemgraph.tools.ase_core import _resolve_existing_path + all_data = [] for file_path in file_paths: if not file_path or not isinstance(file_path, str): continue + # A small model may pass a bare name for a result file a sibling tool + # wrote into CHEMGRAPH_LOG_DIR. Resolve it against the log dir; an + # absolute or cwd-relative path is returned unchanged. + file_path = _resolve_existing_path(file_path) + try: with open(file_path, 'r', encoding='utf-8') as f: for line in f: diff --git a/src/chemgraph/mcp/graspa_mcp_parsl.py b/src/chemgraph/mcp/graspa_mcp_parsl.py index 80779630..0beeb9a5 100644 --- a/src/chemgraph/mcp/graspa_mcp_parsl.py +++ b/src/chemgraph/mcp/graspa_mcp_parsl.py @@ -99,12 +99,16 @@ async def run_graspa_ensemble( params : graspa_input_schema_ensemble Input parameters for the ensemble of gRASPA calculations. """ + from chemgraph.tools.ase_core import _resolve_existing_path + input_source = params.input_structures structure_files: list[Path] = [] output_dir: Path = Path.cwd() # Default fallback if isinstance(input_source, list): - structure_files = [Path(p) for p in input_source] + # Resolve bare names against CHEMGRAPH_LOG_DIR so a file a sibling tool + # wrote there still resolves; absolute/cwd paths are unchanged. + structure_files = [Path(_resolve_existing_path(str(p))) for p in input_source] missing = [p for p in structure_files if not p.exists()] if missing: raise ValueError(f"The following input files are missing: {missing}") diff --git a/src/chemgraph/mcp/hpc_misc_mcp.py b/src/chemgraph/mcp/hpc_misc_mcp.py index 106e5c52..61d47227 100644 --- a/src/chemgraph/mcp/hpc_misc_mcp.py +++ b/src/chemgraph/mcp/hpc_misc_mcp.py @@ -36,6 +36,15 @@ def inspect_json( ) -> dict[str, Any]: """Inspect JSON artifacts without assuming one fixed output-file layout.""" target = Path(path).expanduser() + # A small model may pass a bare name for a JSON file a sibling tool wrote + # into CHEMGRAPH_LOG_DIR. If the raw path is not a file, resolve it against + # the log dir before falling back to the directory / nearby-files logic. + if not target.is_file(): + from chemgraph.tools.ase_core import _resolve_existing_path + + resolved = Path(_resolve_existing_path(str(target))).expanduser() + if resolved.is_file(): + target = resolved if target.is_file(): return { "status": "ok", diff --git a/src/chemgraph/mcp/mace_mcp_hpc.py b/src/chemgraph/mcp/mace_mcp_hpc.py index c3bbe5fd..c31c3157 100644 --- a/src/chemgraph/mcp/mace_mcp_hpc.py +++ b/src/chemgraph/mcp/mace_mcp_hpc.py @@ -149,14 +149,23 @@ def _embed_inline_if_local(job: dict) -> None: if job.get("remote_structure_file") or job.get("inline_structure"): return input_file = job.get("input_structure_file") - if not input_file or not os.path.isfile(input_file): - return # remote path -- worker will read it directly + if not input_file: + return from ase.io import read as ase_read - from chemgraph.tools.ase_core import atoms_to_atomsdata + from chemgraph.tools.ase_core import _resolve_existing_path, atoms_to_atomsdata + + # A small model may echo back a bare name ("water.xyz") for a file a + # sibling tool wrote into CHEMGRAPH_LOG_DIR. Resolve it here, on the + # submitting host where the log dir lives, before deciding whether the + # input is local. Absolute/cwd paths are returned unchanged. + resolved = _resolve_existing_path(input_file) + if not os.path.isfile(resolved): + return # remote path -- worker will read it directly - atoms = ase_read(input_file) + job["input_structure_file"] = resolved + atoms = ase_read(resolved) job["inline_structure"] = atoms_to_atomsdata(atoms).model_dump() diff --git a/src/chemgraph/mcp/xanes_mcp_parsl.py b/src/chemgraph/mcp/xanes_mcp_parsl.py index acd64be8..a4bcc274 100644 --- a/src/chemgraph/mcp/xanes_mcp_parsl.py +++ b/src/chemgraph/mcp/xanes_mcp_parsl.py @@ -118,13 +118,16 @@ async def run_xanes_ensemble(params: xanes_input_schema_ensemble): write_fdmnes_input, extract_conv, ) + from chemgraph.tools.ase_core import _resolve_existing_path input_source = params.input_structures structure_files: list[Path] = [] output_dir: Path = Path.cwd() if isinstance(input_source, list): - structure_files = [Path(p) for p in input_source] + # Resolve bare names against CHEMGRAPH_LOG_DIR so a file a sibling tool + # wrote there still resolves; absolute/cwd paths are unchanged. + structure_files = [Path(_resolve_existing_path(str(p))) for p in input_source] missing = [p for p in structure_files if not p.exists()] if missing: raise ValueError(f"The following input files are missing: {missing}") diff --git a/src/chemgraph/tools/graspa_core.py b/src/chemgraph/tools/graspa_core.py index 0294b75f..09171c91 100644 --- a/src/chemgraph/tools/graspa_core.py +++ b/src/chemgraph/tools/graspa_core.py @@ -279,7 +279,11 @@ def _calculate_cell_size( return [uc_x, uc_y, uc_z] - cif_path = Path(params.input_structure_file).resolve() + # Resolve a bare relative name against CHEMGRAPH_LOG_DIR (where a sibling + # tool wrote the file) before falling back to a cwd-relative absolute path. + from chemgraph.tools.ase_core import _resolve_existing_path + + cif_path = Path(_resolve_existing_path(params.input_structure_file)).resolve() if not cif_path.exists(): raise FileNotFoundError(f"CIF file does not exist: {cif_path}") diff --git a/src/chemgraph/tools/xanes_core.py b/src/chemgraph/tools/xanes_core.py index 9edf1ede..700b0797 100644 --- a/src/chemgraph/tools/xanes_core.py +++ b/src/chemgraph/tools/xanes_core.py @@ -238,7 +238,11 @@ def run_xanes_core(params: xanes_input_schema) -> dict: "Set it to the path of the FDMNES executable." ) - input_path = Path(params.input_structure_file).resolve() + # Resolve a bare relative name against CHEMGRAPH_LOG_DIR (where a sibling + # tool wrote the file) before falling back to a cwd-relative absolute path. + from chemgraph.tools.ase_core import _resolve_existing_path + + input_path = Path(_resolve_existing_path(params.input_structure_file)).resolve() if not input_path.exists(): raise FileNotFoundError(f"Input structure file not found: {input_path}") diff --git a/tests/test_graspa_tools.py b/tests/test_graspa_tools.py index 5bddcc20..89fa485e 100644 --- a/tests/test_graspa_tools.py +++ b/tests/test_graspa_tools.py @@ -43,4 +43,44 @@ def test_run_graspa_core_execution(mock_parser, mock_subproc, mock_cif): assert result["uptake_in_mol_kg"] == 1.5 # Cleanup (optional with tmp_path) + shutil.rmtree(sim_dir) + + +@patch("subprocess.run") +@patch("chemgraph.tools.graspa_core._read_graspa_sycl_output") +def test_run_graspa_core_resolves_bare_name( + mock_parser, mock_subproc, tmp_path, monkeypatch +): + """A bare CIF name is resolved against CHEMGRAPH_LOG_DIR. + + A small model may pass just "test_mof.cif" for a file a sibling tool + wrote into the log dir. run_graspa_core must resolve it there instead + of raising 'CIF file does not exist'. + """ + monkeypatch.setenv("CHEMGRAPH_LOG_DIR", str(tmp_path)) + cif_file = tmp_path / "test_mof.cif" + cif_file.write_text( + "data_test\n_cell_length_a 10\n_cell_length_b 10\n_cell_length_c 10\n" + "_cell_angle_alpha 90\n_cell_angle_beta 90\n_cell_angle_gamma 90\n" + "loop_\n_atom_site_label\n_atom_site_type_symbol\n_atom_site_fract_x\n" + "_atom_site_fract_y\n_atom_site_fract_z\nC C 0 0 0" + ) + + params = graspa_input_schema( + input_structure_file="test_mof.cif", # bare name, not absolute + adsorbate="CO2", + temperature=298.0, + pressure=100000.0, + n_cycles=100, + output_result_file="raspa.log", + ) + mock_subproc.return_value = MagicMock(returncode=0) + mock_parser.return_value = {"status": "success", "uptake_in_mol_kg": 2.0} + + result = run_graspa_core(params) + + # Resolved to the log-dir file: the run proceeds instead of raising. + assert result["uptake_in_mol_kg"] == 2.0 + sim_dir = tmp_path / "test_mof--CO2-298.0-100000" + assert sim_dir.exists() shutil.rmtree(sim_dir) \ No newline at end of file diff --git a/tests/test_mcp.py b/tests/test_mcp.py index 86d32558..20ab13e2 100644 --- a/tests/test_mcp.py +++ b/tests/test_mcp.py @@ -257,3 +257,107 @@ async def test_aggregate_and_rank(tmp_path): assert "Analysis Complete" in text assert "mof_1.cif" in text assert "mof_2.cif" in text # Should find both due to tolerance + + +# --------------------------------------------------------------------------- +# Bare-name path resolution across MCP tools (small-model safety net). +# A file written by a sibling tool lands in CHEMGRAPH_LOG_DIR; a small model +# may echo back just the bare name. These tools must resolve it there. +# --------------------------------------------------------------------------- + + +@pytest.mark.asyncio +async def test_mcp_run_ase_resolves_bare_name_from_log_dir(monkeypatch, tmp_path): + """The general MCP run_ase resolves a bare input name via the log dir.""" + from ase.build import molecule + from ase.io import write as ase_write + + monkeypatch.setenv("CHEMGRAPH_LOG_DIR", str(tmp_path)) + ase_write(str(tmp_path / "h2o.xyz"), molecule("H2O")) + + input_data = { + "input_structure_file": "h2o.xyz", # bare name, not an absolute path + "output_results_file": "h2o_energy.json", + "driver": "energy", + "calculator": {"calculator_type": "emt"}, + } + async with Client(mcp) as client: + res = await client.call_tool("run_ase", {"params": input_data}) + result_dict = json.loads(res.content[0].text) + assert result_dict["status"] == "success" + + +@pytest.mark.asyncio +async def test_mcp_extract_output_json_resolves_bare_name(monkeypatch, tmp_path): + """extract_output_json resolves a bare JSON name via the log dir.""" + monkeypatch.setenv("CHEMGRAPH_LOG_DIR", str(tmp_path)) + (tmp_path / "result.json").write_text('{"ok": true}', encoding="utf-8") + + async with Client(mcp) as client: + res = await client.call_tool("extract_output_json", {"json_file": "result.json"}) + payload = json.loads(res.content[0].text) + assert payload["ok"] is True + + +def test_embed_inline_resolves_bare_name(monkeypatch, tmp_path): + """_embed_inline_if_local resolves a bare name and embeds the structure.""" + from ase.build import molecule + from ase.io import write as ase_write + + from chemgraph.mcp import mace_mcp_hpc + + monkeypatch.setenv("CHEMGRAPH_LOG_DIR", str(tmp_path)) + ase_write(str(tmp_path / "h2o.xyz"), molecule("H2O")) + + job = {"input_structure_file": "h2o.xyz"} # bare name + mace_mcp_hpc._embed_inline_if_local(job) + + assert job["input_structure_file"] == str(tmp_path / "h2o.xyz") + assert "inline_structure" in job + + +def test_embed_inline_leaves_remote_and_missing_alone(monkeypatch, tmp_path): + """Remote paths and genuinely missing files are deferred to the worker.""" + from chemgraph.mcp import mace_mcp_hpc + + monkeypatch.setenv("CHEMGRAPH_LOG_DIR", str(tmp_path)) + + remote_job = { + "input_structure_file": "x.xyz", + "remote_structure_file": "/remote/x.xyz", + } + mace_mcp_hpc._embed_inline_if_local(remote_job) + assert "inline_structure" not in remote_job + + missing_job = {"input_structure_file": "not_here.xyz"} + mace_mcp_hpc._embed_inline_if_local(missing_job) + assert "inline_structure" not in missing_job + + +def test_data_analysis_aggregate_resolves_bare_name(monkeypatch, tmp_path): + """aggregate_simulation_results reads a bare-name file from the log dir.""" + from chemgraph.mcp import data_analysis_mcp + + monkeypatch.setenv("CHEMGRAPH_LOG_DIR", str(tmp_path)) + (tmp_path / "sim.jsonl").write_text( + json.dumps( + { + "status": "success", + "cif_path": "/abs/mof_1.cif", + "uptake_in_mol_kg": 5.0, + "temperature_in_K": 298.0, + "pressure_in_Pa": 1e5, + } + ) + + "\n", + encoding="utf-8", + ) + out_csv = tmp_path / "agg.csv" + + # Tool function with a bare input name (resolved against the log dir). + msg = data_analysis_mcp.aggregate_simulation_results( + file_paths=["sim.jsonl"], + output_csv_path=str(out_csv), + ) + assert "Success" in msg + assert out_csv.exists()