From 105d85d80890ea29c7454bf500308f756e28b78d Mon Sep 17 00:00:00 2001 From: basil-chen <192173459+rastagan-git@users.noreply.github.com> Date: Wed, 22 Jul 2026 21:01:41 +0800 Subject: [PATCH] fix: complete issue 8 CLI and provenance semantics --- .agents/skills/curate-aroma-data/SKILL.md | 11 +- .../references/output-schema.md | 17 +- README-CN.md | 15 +- README.md | 15 +- aromanexus/__main__.py | 6 + aromanexus/cli.py | 20 +- aromanexus/models.py | 2 + aromanexus/sources/pubchem.py | 1 + aromanexus/workflows.py | 78 +++++- tests/test_branding.py | 37 +++ tests/test_cli.py | 7 +- tests/test_pubchem.py | 22 +- tests/test_workflows.py | 238 +++++++++++++++++- tests/test_xlsx_preservation.py | 7 +- 14 files changed, 458 insertions(+), 18 deletions(-) create mode 100644 aromanexus/__main__.py diff --git a/.agents/skills/curate-aroma-data/SKILL.md b/.agents/skills/curate-aroma-data/SKILL.md index 01ac87a..f645c7b 100644 --- a/.agents/skills/curate-aroma-data/SKILL.md +++ b/.agents/skills/curate-aroma-data/SKILL.md @@ -7,6 +7,8 @@ description: Use AromaNexus to validate, normalize, enrich, and export flavor, o Build traceable compound tables through the repository's `aromanexus` CLI. Keep the skill as an orchestration layer; modify provider behavior in the Python package, not here. +If the console launcher is unavailable, replace `aromanexus ...` with the equivalent `python -m aromanexus ...` invocation. + ## Workflow 1. Inspect the input without modifying it. @@ -21,12 +23,14 @@ Build traceable compound tables through the repository's `aromanexus` CLI. Keep - Use Pyrfume only for explicitly selected archives after reviewing each manifest note. - Use M2OR only when receptor bioassay evidence is relevant; label species and assay scope. 3. Preview the operation. - - State the input, selected worksheet for XLSX, new output path, selected provider, expected columns, skip patterns, approximate request count, cache behavior, and material access caveats. + - State the input, selected worksheet for XLSX, new output path, selected provider, expected columns, skip patterns, whether odor annotations are requested, any existing-CAS confirmation column, approximate request count, cache behavior, and material access caveats. - Write a sibling output by default. Never reuse the input path as the output path; `--force` is only for a separate existing destination. - Keep XLSX input and output when worksheet formulas, formatting, or other workbook content must survive; CSV/TSV output is a flat export. 4. Run one focused command. - Identity and odor metadata: `aromanexus pubchem INPUT --identifier-column "CAS Number"` - Name lookup with dataset-specific structural rows: `aromanexus pubchem INPUT --identifier-column "Name" --skip-pattern '^C\d+$'` + - Name lookup with a conservative existing-CAS signal: `aromanexus pubchem INPUT --identifier-column "Name" --existing-cas-column "Existing CAS"` + - Identity without PUG-View requests or odor-only output columns: `aromanexus pubchem INPUT --no-odor` - Retention indices: `aromanexus nist-ri INPUT --cas-column "CAS Number" --calculated-ri-column "Calculated RI"` - Names to CAS: `aromanexus resolve-cas INPUT --name-column "Name"` - Curated descriptors: `aromanexus pyrfume INPUT --archives aromadb,superscent` @@ -35,10 +39,10 @@ Build traceable compound tables through the repository's `aromanexus` CLI. Keep - For XLSX only, append `--sheet "SHEET"` to any table command when the target is not the first worksheet. Never pass `--sheet` for CSV or TSV. 5. Verify the result. - Re-run the inspection script on the same worksheet for XLSX, or without `--sheet` for CSV/TSV. - - Confirm identical row order and row count, expected new fields, typed status counts, source URL, retrieval time, version, and license/access fields. + - Confirm identical row order and row count, expected new fields, typed status counts, source URL, retrieval time, version, and license/access fields. Treat a blank retrieval time as correct when no provider or cached representation was obtained, including an explicit pre-request skip. - For XLSX output, also compare worksheet order and names, per-sheet content digests, workbook properties, non-target-sheet content, untargeted source formulas and cached values, styles, dimensions, and reported workbook features. - Treat `PubChem Status` as provider execution state, not proof of a uniquely resolved CAS. Check `PubChem CAS Resolution`, candidate count, and `Resolved CAS` separately. - - Leave `multiple` and `missing` CAS resolutions unresolved; retain all candidates and route only the affected rows to a targeted fallback source or manual review. + - Leave `multiple`, `missing`, `input_cas_conflict`, and `input_cas_invalid` CAS resolutions unresolved; retain all candidates and route only the affected rows to a targeted fallback source or manual review. - Treat `http_error`, `network_error`, `parse_error`, `missing_data`, `data_error`, `partial`, `blocked`, and `skipped` separately from `not_found`. - Consult [references/output-schema.md](references/output-schema.md) when reconciling columns or statuses. 6. Report the output path, provider versions, status counts, partial failures, and any source terms the user must still review. @@ -54,3 +58,4 @@ Build traceable compound tables through the repository's `aromanexus` CLI. Keep - Preserve merged cells outside the selected tabular rectangle; stop before provider calls when a merge intersects that rectangle. - Stop before provider calls if XLSX preflight reports a known unsafe feature or any OOXML package part that the in-memory trial write would discard. - Never select the first PubChem CAS candidate merely because the provider status is `ok`. +- Use an existing CAS column only to confirm a returned candidate for a non-CAS query. Never overwrite it or use a conflicting/invalid value to force resolution. diff --git a/.agents/skills/curate-aroma-data/references/output-schema.md b/.agents/skills/curate-aroma-data/references/output-schema.md index f88a9a8..a983fe2 100644 --- a/.agents/skills/curate-aroma-data/references/output-schema.md +++ b/.agents/skills/curate-aroma-data/references/output-schema.md @@ -15,11 +15,14 @@ Keep provider execution and CAS curation in separate columns: - `PubChem CAS Numbers`: every checksum-valid candidate retained for review. - `PubChem CAS Candidate Count`: the number of distinct valid candidates. - `PubChem CAS Resolution`: the conservative resolution decision. -- `Resolved CAS`: populate only for `query_confirmed` or `unique`. +- `Resolved CAS`: populate only for `query_confirmed`, `input_cas_confirmed`, or `unique`. Interpret `PubChem CAS Resolution` as follows: - `query_confirmed`: the input was a checksum-valid CAS and PubChem resolved its record. +- `input_cas_confirmed`: for a non-CAS query, the optional existing CAS was valid and appeared among the returned candidates. +- `input_cas_conflict`: the optional existing CAS was valid but absent from the returned candidates; keep `Resolved CAS` empty. +- `input_cas_invalid`: the optional existing CAS was nonblank but invalid; keep `Resolved CAS` empty. - `unique`: a successful name lookup returned exactly one valid CAS candidate. - `multiple`: more than one valid candidate remains; keep `Resolved CAS` empty. - `missing`: a successful record returned no valid CAS candidate; keep `Resolved CAS` empty. @@ -43,10 +46,20 @@ Interpret `PubChem CAS Resolution` as follows: Do not merge an access, transport, HTTP, snapshot, parse, or partial failure into `not_found`. +When the optional existing-CAS cell is blank or missing, use the normal `unique`, `multiple`, or `missing` rule. A valid CAS query itself always takes precedence as `query_confirmed`. Never modify the existing-CAS column or remove candidates from `PubChem CAS Numbers`. + +For a `partial` provider result, accept only positive `query_confirmed` or `input_cas_confirmed` evidence. Do not infer `unique`, `multiple`, `missing`, `input_cas_conflict`, or `input_cas_invalid` from a potentially incomplete secondary response; use `not_evaluated` instead. + +## Optional odor columns + +PubChem odor enrichment is enabled by default. With `--no-odor`, skip PUG-View requests and do not add or update `PubChem Odor`, `PubChem Odor Annotations`, `PubChem Odor Sources`, `PubChem Odor Source URLs`, or `PubChem Odor License URLs`. Preserve any such columns already present in the input. + ## Provenance Keep provider status, source URL, retrieval time, cache-hit flag, pinned version or snapshot, license URL, and message columns. For PubChem odor text, also keep contributor source names, URLs, and license URLs. +`Retrieved At` is the timestamp of an actual provider or cached representation. Leave it empty for outcomes decided before any representation was obtained, including explicit skips, local input validation failures, and transport failures before a response. A received HTTP or parse-error response retains its retrieval timestamp. + ## Workbook QA After every run, confirm: @@ -57,7 +70,7 @@ After every run, confirm: 4. Every processed row has a typed status. 5. Remote strings beginning with `=`, `+`, `-`, or `@` are stored as literal text. 6. Partial outputs are reported separately if a run is interrupted. -7. `multiple`, `missing`, `not_evaluated`, and `skipped` PubChem CAS resolutions never contain an automatic `Resolved CAS`. +7. `multiple`, `missing`, `input_cas_conflict`, `input_cas_invalid`, `not_evaluated`, and `skipped` PubChem CAS resolutions never contain an automatic `Resolved CAS`. 8. XLSX worksheet order and names match the input, and supported non-target worksheet content and features are unchanged. 9. Source formulas and cached results outside explicitly targeted output cells, plus styles, dimensions, freeze panes, filters, tables, data validation, conditional formatting, and workbook properties, remain present where applicable. 10. Merged cells outside the selected tabular rectangle remain present; a merge intersecting that rectangle is rejected before provider access. diff --git a/README-CN.md b/README-CN.md index a301acf..4077241 100644 --- a/README-CN.md +++ b/README-CN.md @@ -57,6 +57,9 @@ python -m pip install -e . ```bash aromanexus --version aromanexus sources + +# 控制台启动器不可用时，可使用等价的模块入口 +python -m aromanexus --version ``` ## 快速开始 @@ -70,6 +73,12 @@ aromanexus pubchem compounds.xlsx --identifier-column "CAS Number" # 在名称查询前跳过当前数据集中的结构标签 aromanexus pubchem compounds.xlsx --identifier-column "Name" --skip-pattern '^C\d+$' +# 只把已有 CAS 列作为名称查询候选的保守确认信号 +aromanexus pubchem compounds.xlsx --identifier-column "Name" --existing-cas-column "Existing CAS" + +# 跳过 PUG-View 请求，并省略仅用于气味信息的输出列组 +aromanexus pubchem compounds.xlsx --no-odor + # 按精确名称选择工作表 aromanexus pubchem compounds.xlsx --sheet "Data" --identifier-column "Name" @@ -125,7 +134,11 @@ aromanexus --cache-dir .cache/aromanexus --timeout 30 pubchem compounds.xlsx [Openpyxl 无法保留所有 OOXML 功能](https://openpyxl.readthedocs.io/en/3.1/tutorial.html)。因此，AromaNexus 会先在内存中试写一遍；若检测到绘图形状、批注、ActiveX/OLE 控件、切片器、线程批注、VML、数字签名等已知高风险内容，或任何会被试写丢弃的 OOXML 包部件，就会在调用数据源前停止。Excel 的可选计算链可能会被移除，以便表格软件重新生成。若显式输出 CSV/TSV，结果只是扁平表格，无法保留 Excel 专属内容。 -默认来源记录包括数据源状态、来源 URL、获取时间、是否命中缓存、固定版本、许可 URL 与诊断信息。PubChem 会单独报告 CAS 解析状态；仅当输入 CAS 得到确认，或只剩一个校验有效的候选时，才填入 `Resolved CAS`。多个或缺失候选会保持未解析。只有在确实需要旧版形状时才使用 `--no-provenance`。 +默认来源记录包括数据源状态、来源 URL、获取时间、是否命中缓存、固定版本、许可 URL 与诊断信息。`Retrieved At` 只表示真正取得数据源响应或缓存表示的时间；显式 `skipped`、请求前发现的无效输入、尚未收到响应的网络失败等本地结果会留空。只有在确实需要旧版形状时才使用 `--no-provenance`。 + +PubChem 会单独报告 CAS 解析状态。仅当查询本身是已确认的 CAS、名称查询只剩一个校验有效的候选，或 `--existing-cas-column` 中的有效 CAS 确实出现在返回候选里时，才填入 `Resolved CAS`。已有 CAS 与候选冲突或本身无效时保持未解析；空单元格会回退到原有的 `unique`、`multiple` 或 `missing` 规则。对于 `partial` 数据源结果，只接受查询或已有 CAS 的正向确认；依赖完整候选集合的判断会保持 `not_evaluated`。原始标识符列和已有 CAS 列都不会被改写；若已有 CAS 列名与当前输出列重叠，CLI 会在请求前拒绝运行。 + +PubChem 气味扩充默认开启。`--no-odor` 会跳过 PUG-View 请求，并且不新增或更新 `PubChem Odor`、`PubChem Odor Annotations`、`PubChem Odor Sources`、`PubChem Odor Source URLs`、`PubChem Odor License URLs`。如果输入中原本就有这些列，它们会原样保留。 ```bash # 明确指定输出位置 diff --git a/README.md b/README.md index 486ba23..683fed0 100644 --- a/README.md +++ b/README.md @@ -57,6 +57,9 @@ Confirm the installation and review provider modes before a live run: ```bash aromanexus --version aromanexus sources + +# Equivalent module entry when the console launcher is unavailable +python -m aromanexus --version ``` ## Quick start @@ -70,6 +73,12 @@ aromanexus pubchem compounds.xlsx --identifier-column "CAS Number" # Skip dataset-specific structural labels before a name lookup aromanexus pubchem compounds.xlsx --identifier-column "Name" --skip-pattern '^C\d+$' +# Use an existing CAS column only to confirm candidates from a name lookup +aromanexus pubchem compounds.xlsx --identifier-column "Name" --existing-cas-column "Existing CAS" + +# Skip PUG-View requests and omit the odor-only output column group +aromanexus pubchem compounds.xlsx --no-odor + # Select a worksheet by its exact name aromanexus pubchem compounds.xlsx --sheet "Data" --identifier-column "Name" @@ -125,7 +134,11 @@ For XLSX-to-XLSX runs, AromaNexus starts from an immutable copy of the source pa [Openpyxl cannot preserve every OOXML feature](https://openpyxl.readthedocs.io/en/3.1/tutorial.html). AromaNexus therefore performs an in-memory trial round trip and stops before provider calls when it detects known unsafe content—such as drawing shapes, comments, ActiveX/OLE controls, slicers, threaded comments, VML, or digital signatures—or any package part that the trial would discard. Excel's optional calculation chain may be removed so spreadsheet software can rebuild it. An explicit CSV/TSV output is a flat export and cannot retain Excel-only content. -By default, provenance columns include provider status, source URL, retrieval timestamp, cache hit, pinned version, license URL, and a diagnostic message. PubChem reports CAS resolution separately and populates `Resolved CAS` only when the input is a confirmed CAS or exactly one checksum-valid candidate remains. Multiple or missing candidates stay unresolved. Use `--no-provenance` only for legacy-shaped output. +By default, provenance columns include provider status, source URL, retrieval timestamp, cache hit, pinned version, license URL, and a diagnostic message. `Retrieved At` records when a provider or cached representation was actually obtained. It stays empty for local pre-request outcomes such as an explicit `skipped` row, invalid input, or a network failure before any response. Use `--no-provenance` only for legacy-shaped output. + +PubChem reports CAS resolution separately and populates `Resolved CAS` only when the query itself is a confirmed CAS, a name lookup has exactly one checksum-valid candidate, or `--existing-cas-column` supplies a valid CAS that appears among the returned candidates. A conflicting or invalid existing CAS keeps the result unresolved; a blank cell falls back to the normal `unique`, `multiple`, or `missing` decision. For a `partial` provider result, only positive query or existing-CAS confirmation can resolve the row; decisions that depend on a complete candidate set remain `not_evaluated`. The original identifier and existing-CAS columns are never rewritten, and the CLI rejects an existing-CAS column name that overlaps an active output column. + +PubChem odor enrichment is enabled by default. `--no-odor` skips PUG-View requests and does not add or update `PubChem Odor`, `PubChem Odor Annotations`, `PubChem Odor Sources`, `PubChem Odor Source URLs`, or `PubChem Odor License URLs`. If those columns already exist in the input, they are preserved unchanged. ```bash # Choose an output explicitly diff --git a/aromanexus/__main__.py b/aromanexus/__main__.py new file mode 100644 index 0000000..e7f02df --- /dev/null +++ b/aromanexus/__main__.py @@ -0,0 +1,6 @@ +"""Module entry point for ``python -m aromanexus``.""" + +from aromanexus.cli import main + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/aromanexus/cli.py b/aromanexus/cli.py index e9df9c3..19cb790 100644 --- a/aromanexus/cli.py +++ b/aromanexus/cli.py @@ -118,9 +118,24 @@ def build_parser() -> argparse.ArgumentParser: pubchem.add_argument( "--resolved-cas-column", default="Resolved CAS", - help="Output column populated only for query-confirmed or unique CAS resolution", + help=( + "Output column populated only when the query, an existing CAS, or one unique " + "candidate is confirmed" + ), + ) + pubchem.add_argument( + "--existing-cas-column", + metavar="COLUMN", + help=( + "Optional existing CAS column used to confirm name-query candidates; " + "invalid or conflicting values remain unresolved" + ), + ) + pubchem.add_argument( + "--no-odor", + action="store_true", + help="Skip PUG-View requests and do not add or update odor-only output columns", ) - pubchem.add_argument("--no-odor", action="store_true", help="Skip PUG-View odor annotations") pubchem.set_defaults(handler=_handle_pubchem) pyrfume = subparsers.add_parser( @@ -221,6 +236,7 @@ def _handle_pubchem(args: argparse.Namespace) -> RunSummary: args.input, client, identifier_column=args.identifier_column, + existing_cas_column=args.existing_cas_column, resolved_cas_column=args.resolved_cas_column, skip_patterns=args.skip_pattern, include_odor=not args.no_odor, diff --git a/aromanexus/models.py b/aromanexus/models.py index 2e1c14f..84d530d 100644 --- a/aromanexus/models.py +++ b/aromanexus/models.py @@ -35,12 +35,14 @@ def failure( status: str, message: str, source_url: str = "", + retrieved_at: str = "", ) -> LookupResult: return cls( provider=provider, status=status, message=message, source_url=source_url, + retrieved_at=retrieved_at, ) def provenance_columns(self, prefix: str | None = None) -> dict[str, Any]: diff --git a/aromanexus/sources/pubchem.py b/aromanexus/sources/pubchem.py index cd6fb87..a2f4d29 100644 --- a/aromanexus/sources/pubchem.py +++ b/aromanexus/sources/pubchem.py @@ -411,6 +411,7 @@ def _failure(self, *, status: str, message: str, source_url: str = "") -> Lookup status=status, message=message, source_url=source_url, + retrieved_at="", version=PUBCHEM_VERSION, license_url=PUBCHEM_LICENSE_URL, ) diff --git a/aromanexus/workflows.py b/aromanexus/workflows.py index ad5d8ee..662cac4 100644 --- a/aromanexus/workflows.py +++ b/aromanexus/workflows.py @@ -6,7 +6,7 @@ import re import time from collections import Counter -from collections.abc import Callable, Iterable +from collections.abc import Callable, Collection, Iterable from dataclasses import dataclass from pathlib import Path from typing import Any @@ -102,9 +102,12 @@ def _apply_result( include_provenance: bool, prefix: str | None = None, key_map: dict[str, str] | None = None, + excluded_keys: Collection[str] = (), ) -> None: mapping = key_map or {} for key, value in result.values.items(): + if key in excluded_keys: + continue _set_cell(frame, index, mapping.get(key, key), value) if include_provenance: _apply_provenance(frame, index, result, prefix=prefix) @@ -339,6 +342,7 @@ def run_resolve_cas( PUBCHEM_COLUMN_MAP = { + "query": "query", "cid": "PubChem CID", "title": "PubChem Title", "iupac_name": "IUPAC Name", @@ -349,6 +353,7 @@ def run_resolve_cas( "inchi": "InChI", "inchikey": "InChIKey", "xlogp": "XLogP", + "pubchem_url": "pubchem_url", "synonyms": "PubChem Synonyms", "cas_numbers": "PubChem CAS Numbers", "odor": "PubChem Odor", @@ -357,6 +362,15 @@ def run_resolve_cas( "odor_source_urls": "PubChem Odor Source URLs", "odor_license_urls": "PubChem Odor License URLs", } +PUBCHEM_ODOR_KEYS = frozenset( + { + "odor", + "odor_annotations", + "odor_sources", + "odor_source_urls", + "odor_license_urls", + } +) def _compile_skip_patterns(patterns: Iterable[str] | str | None) -> tuple[re.Pattern[str], ...]: @@ -386,6 +400,19 @@ def _valid_cas_candidates(value: Any) -> list[str]: return candidates +def _has_cell_value(value: Any) -> bool: + """Return whether a scalar table cell contains a meaningful value.""" + + if value is None: + return False + try: + if bool(pd.isna(value)): + return False + except (TypeError, ValueError): + pass + return bool(clean_text(value)) + + def _apply_pubchem_resolution( frame: pd.DataFrame, index: Any, @@ -393,18 +420,38 @@ def _apply_pubchem_resolution( result: LookupResult, *, resolved_cas_column: str, + existing_cas: Any = None, ) -> None: candidates = _valid_cas_candidates(result.values.get("cas_numbers")) resolved_cas = "" + query_cas = normalize_cas(identifier) + has_existing_cas = _has_cell_value(existing_cas) + input_cas = normalize_cas(existing_cas) if has_existing_cas else "" if result.status == "skipped": resolution = "skipped" + elif result.status == "partial": + if is_valid_cas(query_cas): + resolution = "query_confirmed" + resolved_cas = query_cas + elif is_valid_cas(input_cas) and input_cas in candidates: + resolution = "input_cas_confirmed" + resolved_cas = input_cas + else: + resolution = "not_evaluated" elif result.status != "ok": resolution = "not_evaluated" else: - query_cas = normalize_cas(identifier) if is_valid_cas(query_cas): resolution = "query_confirmed" resolved_cas = query_cas + elif has_existing_cas: + if not is_valid_cas(input_cas): + resolution = "input_cas_invalid" + elif input_cas in candidates: + resolution = "input_cas_confirmed" + resolved_cas = input_cas + else: + resolution = "input_cas_conflict" elif len(candidates) == 1: resolution = "unique" resolved_cas = candidates[0] @@ -425,6 +472,7 @@ def run_pubchem( output_path: str | Path | None = None, sheet_name: str | None = None, identifier_column: str = "CAS Number", + existing_cas_column: str | None = None, resolved_cas_column: str = "Resolved CAS", skip_patterns: Iterable[str] | str | None = None, include_odor: bool = True, @@ -434,6 +482,11 @@ def run_pubchem( progress: ProgressCallback = console_progress, ) -> RunSummary: compiled_skip_patterns = _compile_skip_patterns(skip_patterns) + planned_pubchem_columns = tuple( + output_column + for key, output_column in PUBCHEM_COLUMN_MAP.items() + if include_odor or key not in PUBCHEM_ODOR_KEYS + ) frame, destination, context = _prepare_run( input_path, output_path, @@ -442,14 +495,29 @@ def run_pubchem( checkpoint_every=checkpoint_every, force=force, planned_columns=( - *PUBCHEM_COLUMN_MAP.values(), + *planned_pubchem_columns, "PubChem CAS Resolution", "PubChem CAS Candidate Count", resolved_cas_column, *(provenance_column_names("PubChem") if include_provenance else ()), ), ) - require_columns(frame, identifier_column) + required_columns = [identifier_column] + if existing_cas_column is not None: + required_columns.append(existing_cas_column) + require_columns(frame, *required_columns) + active_output_columns = { + *planned_pubchem_columns, + "PubChem CAS Resolution", + "PubChem CAS Candidate Count", + resolved_cas_column, + *(provenance_column_names("PubChem") if include_provenance else ()), + } + if existing_cas_column is not None and existing_cas_column in active_output_columns: + raise ValueError( + f"Existing CAS column {existing_cas_column!r} conflicts with an active PubChem " + "output column; choose distinct input and output column names." + ) statuses: Counter[str] = Counter() total = len(frame) for current, index, row in _indices(frame): @@ -474,6 +542,7 @@ def run_pubchem( include_provenance=include_provenance, prefix="PubChem", key_map=PUBCHEM_COLUMN_MAP, + excluded_keys=() if include_odor else PUBCHEM_ODOR_KEYS, ) _apply_pubchem_resolution( frame, @@ -481,6 +550,7 @@ def run_pubchem( identifier, result, resolved_cas_column=resolved_cas_column, + existing_cas=(row[existing_cas_column] if existing_cas_column is not None else None), ) statuses[result.status] += 1 progress(current, total, f"PubChem: {identifier} -> {result.status}") diff --git a/tests/test_branding.py b/tests/test_branding.py index db8ccb7..2be1790 100644 --- a/tests/test_branding.py +++ b/tests/test_branding.py @@ -3,6 +3,8 @@ import tomllib from pathlib import Path +import pytest + from aromanexus import __version__ from aromanexus.cli import main from aromanexus.models import LookupResult @@ -48,3 +50,38 @@ def test_legacy_module_cli_forwards_to_aromanexus(): text=True, ) assert completed.stdout.strip() == f"aromanexus {__version__}" + + +@pytest.mark.parametrize("argument", ["--version", "--help"]) +def test_primary_module_cli_matches_console_module(argument): + package_entry = subprocess.run( + [sys.executable, "-m", "aromanexus", argument], + cwd=REPO_ROOT, + check=False, + capture_output=True, + text=True, + ) + console_module = subprocess.run( + [sys.executable, "-m", "aromanexus.cli", argument], + cwd=REPO_ROOT, + check=False, + capture_output=True, + text=True, + ) + + assert package_entry.returncode == console_module.returncode == 0 + assert package_entry.stdout == console_module.stdout + assert package_entry.stderr == console_module.stderr + + +def test_primary_module_cli_preserves_argparse_error_exit_code(): + completed = subprocess.run( + [sys.executable, "-m", "aromanexus"], + cwd=REPO_ROOT, + check=False, + capture_output=True, + text=True, + ) + + assert completed.returncode == 2 + assert "command" in completed.stderr diff --git a/tests/test_cli.py b/tests/test_cli.py index d6875e4..42b451e 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -17,7 +17,7 @@ def test_missing_input_returns_user_error(capsys, tmp_path): assert "does not exist" in capsys.readouterr().err -def test_pubchem_cli_forwards_repeatable_skip_patterns_and_resolved_column(monkeypatch, tmp_path): +def test_pubchem_cli_forwards_resolution_skip_and_odor_options(monkeypatch, tmp_path): input_path = tmp_path / "input.csv" input_path.write_text("Name\nC6\n", encoding="utf-8") captured = {} @@ -44,6 +44,9 @@ def fake_run_pubchem(input_file, client, **kwargs): "^Total$", "--resolved-cas-column", "Curated CAS", + "--existing-cas-column", + "Existing CAS", + "--no-odor", "--sheet", "Data", ] @@ -53,6 +56,8 @@ def fake_run_pubchem(input_file, client, **kwargs): assert captured["input"] == input_path assert captured["kwargs"]["skip_patterns"] == [r"^C\d+$", "^Total$"] assert captured["kwargs"]["resolved_cas_column"] == "Curated CAS" + assert captured["kwargs"]["existing_cas_column"] == "Existing CAS" + assert captured["kwargs"]["include_odor"] is False assert captured["kwargs"]["sheet_name"] == "Data" diff --git a/tests/test_pubchem.py b/tests/test_pubchem.py index 6bc48b5..3a26d7a 100644 --- a/tests/test_pubchem.py +++ b/tests/test_pubchem.py @@ -3,7 +3,7 @@ import json from typing import Any -from aromanexus.http import HttpResponse, RetrievalMetadata +from aromanexus.http import HttpClientError, HttpResponse, RetrievalMetadata from aromanexus.sources.pubchem import ( PubChemClient, parse_pubchem_identifiers, @@ -131,6 +131,11 @@ def get(self, url: str, **kwargs: Any) -> HttpResponse: return self.responses.pop(0) +class FailingHttp: + def get(self, url: str, **_kwargs: Any) -> HttpResponse: + raise HttpClientError(url, 3, OSError("offline")) + + def test_pure_pubchem_parsers_handle_current_api_shapes() -> None: properties = parse_pubchem_properties(PROPERTIES) assert properties["cid"] == 240 @@ -221,10 +226,25 @@ def test_primary_not_found_and_malformed_payloads_are_explicit() -> None: malformed = client.lookup("also-not-real") assert missing.status == "not_found" + assert missing.retrieved_at == "2026-07-19T00:00:00+00:00" assert malformed.status == "parse_error" + assert malformed.retrieved_at == "2026-07-19T00:00:00+00:00" assert len(fake_http.calls) == 2 +def test_failure_before_any_pubchem_response_has_no_retrieval_timestamp() -> None: + invalid_client = PubChemClient(http_client=FakeHttp()) + network_client = PubChemClient(http_client=FailingHttp()) + + invalid = invalid_client.lookup(" ") + network = network_client.lookup("benzaldehyde") + + assert invalid.status == "invalid_input" + assert invalid.retrieved_at == "" + assert network.status == "network_error" + assert network.retrieved_at == "" + + def test_pubchem_default_rate_is_below_five_requests_per_second(tmp_path: Any) -> None: client = PubChemClient(cache_dir=tmp_path) assert client.http.min_interval > 0.2 diff --git a/tests/test_workflows.py b/tests/test_workflows.py index c78a5b9..f7bf0b0 100644 --- a/tests/test_workflows.py +++ b/tests/test_workflows.py @@ -5,7 +5,7 @@ from aromanexus.excel_io import read_table from aromanexus.models import LookupResult -from aromanexus.workflows import run_nist_ri, run_pubchem +from aromanexus.workflows import _apply_pubchem_resolution, run_nist_ri, run_pubchem class FakeNist: @@ -53,6 +53,12 @@ def _silent(*_): return None +def test_local_lookup_failure_has_no_retrieval_timestamp(): + result = LookupResult.failure("PubChem", status="skipped", message="local rule") + + assert result.retrieved_at == "" + + def test_legacy_nist_contract_preserves_rows_and_columns(tmp_path: Path): source = tmp_path / "input.xlsx" destination = tmp_path / "output.xlsx" @@ -120,10 +126,54 @@ def test_pubchem_skip_pattern_excludes_structural_rows_before_lookup(tmp_path: P output = read_table(destination) assert client.calls == [("n-Hexane", True)] assert output.loc[0, "PubChem Status"] == "skipped" + assert pd.isna(output.loc[0, "PubChem Retrieved At"]) assert output.loc[0, "PubChem CAS Resolution"] == "skipped" assert output.loc[0, "PubChem CAS Candidate Count"] == 0 assert pd.isna(output.loc[0, "Resolved CAS"]) assert output.loc[1, "Resolved CAS"] == "110-54-3" + assert pd.notna(output.loc[1, "PubChem Retrieved At"]) + + +def test_pubchem_no_odor_omits_optional_output_columns(tmp_path: Path): + source = tmp_path / "input.xlsx" + destination = tmp_path / "pubchem.xlsx" + pd.DataFrame({"Name": ["benzaldehyde"]}).to_excel(source, index=False) + client = ScriptedPubChem( + { + "benzaldehyde": LookupResult( + provider="PubChem", + values={ + "cid": 240, + "odor": ["almond"], + "odor_annotations": [{"text": "almond"}], + "odor_sources": ["Example"], + "odor_source_urls": ["https://example.test/source"], + "odor_license_urls": ["https://example.test/license"], + }, + ) + } + ) + + run_pubchem( + source, + client, + output_path=destination, + identifier_column="Name", + include_odor=False, + checkpoint_every=0, + progress=_silent, + ) + + output = read_table(destination) + assert client.calls == [("benzaldehyde", False)] + assert output.loc[0, "PubChem CID"] == 240 + assert not { + "PubChem Odor", + "PubChem Odor Annotations", + "PubChem Odor Sources", + "PubChem Odor Source URLs", + "PubChem Odor License URLs", + }.intersection(output.columns) def test_pubchem_separates_lookup_status_from_cas_resolution(tmp_path: Path): @@ -186,6 +236,192 @@ def test_pubchem_separates_lookup_status_from_cas_resolution(tmp_path: Path): assert output.loc[4, "PubChem Status"] == "not_found" +def test_pubchem_existing_cas_is_a_conservative_confirmation_signal(tmp_path: Path): + source = tmp_path / "input.xlsx" + destination = tmp_path / "pubchem.xlsx" + identifiers = [ + "Confirmed", + "Conflict", + "Invalid", + "Blank unique", + "Blank multiple", + "100-52-7", + ] + pd.DataFrame( + { + "Name": identifiers, + "Existing CAS": ["100527", "110-54-3", "100-52-8", None, " ", "64-17-5"], + } + ).to_excel(source, index=False) + client = ScriptedPubChem( + { + "Confirmed": LookupResult( + provider="PubChem", + values={"cas_numbers": ["64-17-5", "100-52-7"]}, + ), + "Conflict": LookupResult( + provider="PubChem", + values={"cas_numbers": ["66-25-1"]}, + ), + "Invalid": LookupResult( + provider="PubChem", + values={"cas_numbers": ["67-64-1"]}, + ), + "Blank unique": LookupResult( + provider="PubChem", + values={"cas_numbers": ["64-17-5"]}, + ), + "Blank multiple": LookupResult( + provider="PubChem", + values={"cas_numbers": ["64-17-5", "67-64-1"]}, + ), + "100-52-7": LookupResult( + provider="PubChem", + values={"cas_numbers": ["100-52-7", "64-17-5"]}, + ), + } + ) + + run_pubchem( + source, + client, + output_path=destination, + identifier_column="Name", + existing_cas_column="Existing CAS", + checkpoint_every=0, + progress=_silent, + ) + + output = read_table(destination) + assert output["PubChem CAS Resolution"].tolist() == [ + "input_cas_confirmed", + "input_cas_conflict", + "input_cas_invalid", + "unique", + "multiple", + "query_confirmed", + ] + assert output["PubChem CAS Candidate Count"].tolist() == [2, 1, 1, 1, 2, 2] + assert output.loc[0, "Resolved CAS"] == "100-52-7" + assert pd.isna(output.loc[1, "Resolved CAS"]) + assert pd.isna(output.loc[2, "Resolved CAS"]) + assert output.loc[3, "Resolved CAS"] == "64-17-5" + assert pd.isna(output.loc[4, "Resolved CAS"]) + assert output.loc[5, "Resolved CAS"] == "100-52-7" + assert output.loc[0, "PubChem CAS Numbers"] == "64-17-5; 100-52-7" + + +def test_pubchem_partial_results_only_use_positive_cas_confirmation(tmp_path: Path): + source = tmp_path / "input.xlsx" + destination = tmp_path / "pubchem.xlsx" + pd.DataFrame( + { + "Name": ["Confirmed", "Incomplete candidates"], + "Existing CAS": ["100-52-7", "110-54-3"], + } + ).to_excel(source, index=False) + client = ScriptedPubChem( + { + "Confirmed": LookupResult( + provider="PubChem", + values={"cas_numbers": ["64-17-5", "100-52-7"]}, + status="partial", + message="Odor annotations unavailable", + ), + "Incomplete candidates": LookupResult( + provider="PubChem", + values={"cas_numbers": ["64-17-5"]}, + status="partial", + message="CAS identifiers unavailable", + ), + } + ) + + run_pubchem( + source, + client, + output_path=destination, + identifier_column="Name", + existing_cas_column="Existing CAS", + checkpoint_every=0, + progress=_silent, + ) + + output = read_table(destination) + assert output["PubChem CAS Resolution"].tolist() == [ + "input_cas_confirmed", + "not_evaluated", + ] + assert output.loc[0, "Resolved CAS"] == "100-52-7" + assert pd.isna(output.loc[1, "Resolved CAS"]) + + +def test_pubchem_resolution_treats_pd_na_existing_cas_as_blank(): + frame = pd.DataFrame({"Name": ["benzaldehyde"]}) + result = LookupResult( + provider="PubChem", + values={"cas_numbers": ["100-52-7"]}, + ) + + _apply_pubchem_resolution( + frame, + 0, + "benzaldehyde", + result, + resolved_cas_column="Resolved CAS", + existing_cas=pd.NA, + ) + + assert frame.loc[0, "PubChem CAS Resolution"] == "unique" + assert frame.loc[0, "Resolved CAS"] == "100-52-7" + + +def test_missing_existing_cas_column_fails_before_provider_calls(tmp_path: Path): + source = tmp_path / "input.xlsx" + destination = tmp_path / "pubchem.xlsx" + pd.DataFrame({"Name": ["benzaldehyde"]}).to_excel(source, index=False) + client = ScriptedPubChem({}) + + with pytest.raises(ValueError, match="Existing CAS"): + run_pubchem( + source, + client, + output_path=destination, + identifier_column="Name", + existing_cas_column="Existing CAS", + checkpoint_every=0, + progress=_silent, + ) + + assert client.calls == [] + assert not destination.exists() + + +def test_existing_cas_column_cannot_overlap_active_output(tmp_path: Path): + source = tmp_path / "input.xlsx" + destination = tmp_path / "pubchem.xlsx" + pd.DataFrame({"Name": ["benzaldehyde"], "Existing CAS": ["100-52-7"]}).to_excel( + source, index=False + ) + client = ScriptedPubChem({}) + + with pytest.raises(ValueError, match="conflicts with an active PubChem output column"): + run_pubchem( + source, + client, + output_path=destination, + identifier_column="Name", + existing_cas_column="Existing CAS", + resolved_cas_column="Existing CAS", + checkpoint_every=0, + progress=_silent, + ) + + assert client.calls == [] + assert not destination.exists() + assert read_table(source).loc[0, "Existing CAS"] == "100-52-7" + + def test_invalid_pubchem_skip_pattern_fails_before_provider_calls(tmp_path: Path): source = tmp_path / "input.xlsx" destination = tmp_path / "pubchem.xlsx" diff --git a/tests/test_xlsx_preservation.py b/tests/test_xlsx_preservation.py index 7af06cc..d8c4bc9 100644 --- a/tests/test_xlsx_preservation.py +++ b/tests/test_xlsx_preservation.py @@ -15,7 +15,7 @@ from aromanexus import excel_io from aromanexus.models import LookupResult -from aromanexus.workflows import run_pubchem +from aromanexus.workflows import PUBCHEM_COLUMN_MAP, PUBCHEM_ODOR_KEYS, run_pubchem class NoLookupPubChem: @@ -621,7 +621,10 @@ def test_merged_table_body_and_column_limit_fail_before_provider(tmp_path: Path) worksheet = workbook.active worksheet.append(["Name"]) worksheet.append(["C6"]) - worksheet["XFC1"] = "Reserved edge cell" + assert {"query", "pubchem_url"}.issubset(PUBCHEM_COLUMN_MAP) + no_odor_output_count = sum(key not in PUBCHEM_ODOR_KEYS for key in PUBCHEM_COLUMN_MAP) + 3 + first_unsafe_column = excel_io.MAX_EXCEL_COLUMNS - no_odor_output_count + 1 + worksheet.cell(row=1, column=first_unsafe_column, value="Reserved edge cell") workbook.save(wide_source) workbook.close() wide_client = NoLookupPubChem()