diff --git a/.agents/skills/curate-aroma-data/SKILL.md b/.agents/skills/curate-aroma-data/SKILL.md index 99650f1..01ac87a 100644 --- a/.agents/skills/curate-aroma-data/SKILL.md +++ b/.agents/skills/curate-aroma-data/SKILL.md @@ -1,6 +1,6 @@ --- name: curate-aroma-data -description: Use AromaNexus to validate, normalize, enrich, and export flavor, odor, aroma, and chemical-sensory datasets with source-level provenance. Use for XLSX, CSV, or TSV workbooks containing CAS numbers, compound names, calculated retention indices, sensory descriptors, odor thresholds, or olfactory-receptor evidence; for selecting among NIST, PubChem, Pyrfume, M2OR, MFFI, and the permission-gated ChemicalBook compatibility connector; and for verifying analysis-ready outputs. Do not use to bypass access controls, relicense source data, or make biomedical or machine-learning claims unsupported by the exported evidence. +description: Use AromaNexus to validate, normalize, enrich, and export flavor, odor, aroma, and chemical-sensory datasets with source-level provenance. Use for XLSX, CSV, or TSV files containing CAS numbers, compound names, calculated retention indices, sensory descriptors, odor thresholds, or olfactory-receptor evidence; for selecting among NIST, PubChem, Pyrfume, M2OR, MFFI, and the permission-gated ChemicalBook compatibility connector; and for verifying analysis-ready outputs. Do not use to bypass access controls, relicense source data, or make biomedical or machine-learning claims unsupported by the exported evidence. --- # Curate Aroma Data @@ -10,8 +10,9 @@ Build traceable compound tables through the repository's `aromanexus` CLI. Keep ## Workflow 1. Inspect the input without modifying it. - - Run `python .agents/skills/curate-aroma-data/scripts/inspect_workbook.py INPUT` from the repository root. - - Confirm the row count, exact column names, identifier quality, duplicates, and formula-like cells. + - For XLSX, run `python .agents/skills/curate-aroma-data/scripts/inspect_workbook.py INPUT --sheet "SHEET"` from the repository root. Omit `--sheet` only when the first worksheet is the intended target. + - For CSV or TSV, run the inspector without `--sheet`; flat files have no worksheets. + - Confirm the worksheet order and exact target when applicable, row count, exact column names, identifier quality, duplicates, source formulas, workbook properties, per-sheet content digests, styles, dimensions, and reported features. - Identify section labels, headers, totals, and other structural rows before provider calls. Define an explicit dataset-specific skip rule; do not assume that text such as `C6` is globally non-chemical. 2. Choose the smallest provider set that supplies the requested fields. - Read [references/provider-matrix.md](references/provider-matrix.md) before any network or browser operation. @@ -20,8 +21,9 @@ 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, new output path, selected provider, expected columns, skip patterns, approximate request count, cache behavior, and material access caveats. - - Write a sibling output by default. Do not pass `--force` or overwrite the input unless the user explicitly requests that exact replacement. + - 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. + - 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+$'` @@ -30,9 +32,11 @@ Build traceable compound tables through the repository's `aromanexus` CLI. Keep - Curated descriptors: `aromanexus pyrfume INPUT --archives aromadb,superscent` - Receptor evidence: `aromanexus m2or INPUT --cas-column "CAS Number"` - Source inventory: `aromanexus sources` + - 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 output. + - 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. + - 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. - Treat `http_error`, `network_error`, `parse_error`, `missing_data`, `data_error`, `partial`, `blocked`, and `skipped` separately from `not_found`. @@ -46,5 +50,7 @@ Build traceable compound tables through the repository's `aromanexus` CLI. Keep - Do not run ChemicalBook automation unless the user confirms documented permission; its current robots policy excludes the legacy routes. - Do not describe the toolkit as AI-powered. Say that structured exports can support downstream statistics, cheminformatics, or machine-learning experiments. - Do not bundle or republish downloaded Pyrfume or M2OR data in the repository. -- Preserve remote text as literal spreadsheet cells to prevent formula execution. +- Preserve newly fetched remote text as literal spreadsheet cells to prevent formula execution. Preserve legitimate source formulas outside cells explicitly targeted by an output field. +- 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`. diff --git a/.agents/skills/curate-aroma-data/references/output-schema.md b/.agents/skills/curate-aroma-data/references/output-schema.md index 133b7f0..f88a9a8 100644 --- a/.agents/skills/curate-aroma-data/references/output-schema.md +++ b/.agents/skills/curate-aroma-data/references/output-schema.md @@ -58,3 +58,9 @@ After every run, confirm: 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`. +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. +11. The target worksheet used for post-run inspection is the same exact worksheet selected for enrichment, and per-sheet content digests are compared. + +For XLSX input, the default target is the first worksheet in workbook order. Pass `--sheet "Name"` to both the command and inspection helper when another worksheet is intended. Never pass `--sheet` for CSV or TSV; flat output cannot satisfy workbook-level preservation checks. XLSX preflight rejects known unsafe features and any OOXML package part dropped by its in-memory trial write. Excel's optional calculation chain may be removed and rebuilt by spreadsheet software. diff --git a/.agents/skills/curate-aroma-data/scripts/inspect_workbook.py b/.agents/skills/curate-aroma-data/scripts/inspect_workbook.py index 7575b8a..5f7069b 100644 --- a/.agents/skills/curate-aroma-data/scripts/inspect_workbook.py +++ b/.agents/skills/curate-aroma-data/scripts/inspect_workbook.py @@ -3,24 +3,106 @@ from __future__ import annotations import argparse +import hashlib import json import sys from pathlib import Path +from openpyxl import load_workbook + REPO_ROOT = Path(__file__).resolve().parents[4] if str(REPO_ROOT) not in sys.path: sys.path.insert(0, str(REPO_ROOT)) -from aromanexus.excel_io import FORMULA_PREFIXES, read_table # noqa: E402 +from aromanexus.excel_io import ( # noqa: E402 + FORMULA_PREFIXES, + read_table, + workbook_sheet_names, +) from aromanexus.identifiers import is_valid_cas, normalize_cas # noqa: E402 -def inspect(path: Path, cas_column: str | None = None) -> dict[str, object]: - frame = read_table(path) +def _stable_cell_value(value: object) -> str | None: + if value is None: + return None + return value.isoformat() if hasattr(value, "isoformat") else str(value) + + +def _sheet_summary(worksheet, cached_worksheet) -> dict[str, object]: + digest = hashlib.sha256() + formula_cache_digest = hashlib.sha256() + formula_cells = 0 + cached_formula_results = 0 + styled_cells = 0 + nonempty_cells = 0 + for row in worksheet.iter_rows(): + for cell in row: + if cell.has_style: + styled_cells += 1 + if cell.value is None: + continue + nonempty_cells += 1 + formula_cells += int(cell.data_type == "f") + value = _stable_cell_value(cell.value) + record = json.dumps( + [cell.coordinate, cell.data_type, value], + ensure_ascii=False, + separators=(",", ":"), + ) + digest.update(record.encode("utf-8")) + digest.update(b"\n") + if cell.data_type == "f": + cached_cell = cached_worksheet[cell.coordinate] + cached_value = _stable_cell_value(cached_cell.value) + cached_formula_results += int(cached_value is not None) + cached_record = json.dumps( + [cell.coordinate, cached_cell.data_type, cached_value], + ensure_ascii=False, + separators=(",", ":"), + ) + formula_cache_digest.update(cached_record.encode("utf-8")) + formula_cache_digest.update(b"\n") + return { + "max_row": worksheet.max_row, + "max_column": worksheet.max_column, + "nonempty_cells": nonempty_cells, + "formula_cells": formula_cells, + "cached_formula_results": cached_formula_results, + "formula_cache_sha256": formula_cache_digest.hexdigest(), + "styled_cells": styled_cells, + "content_sha256": digest.hexdigest(), + "row_heights": { + str(index): dimension.height + for index, dimension in worksheet.row_dimensions.items() + if dimension.height is not None + }, + "column_widths": { + name: dimension.width + for name, dimension in worksheet.column_dimensions.items() + if dimension.width is not None + }, + "freeze_panes": str(worksheet.freeze_panes or ""), + "auto_filter": str(worksheet.auto_filter.ref or ""), + "merged_ranges": [str(item) for item in worksheet.merged_cells.ranges], + "tables": list(worksheet.tables), + "data_validations": len(worksheet.data_validations.dataValidation), + "conditional_formatting_ranges": len(worksheet.conditional_formatting), + } + + +def inspect( + path: Path, + cas_column: str | None = None, + sheet_name: str | None = None, +) -> dict[str, object]: + frame = read_table(path, sheet_name=sheet_name) detected_cas = cas_column or next( (name for name in ("CAS Number", "CAS", "cas", "cas_number") if name in frame.columns), None, ) + if detected_cas is not None and detected_cas not in frame.columns: + available = ", ".join(map(str, frame.columns)) + raise ValueError(f"CAS column {detected_cas!r} was not found. Available: {available}") formula_like = 0 for column in frame.columns: formula_like += int( @@ -39,6 +121,42 @@ def inspect(path: Path, cas_column: str | None = None) -> dict[str, object]: for column in frame.columns }, } + if path.suffix.lower() == ".xlsx": + sheets = workbook_sheet_names(path) + selected_sheet = sheet_name or sheets[0] + workbook = load_workbook( + path, + read_only=False, + data_only=False, + keep_links=True, + rich_text=True, + ) + cached_workbook = load_workbook( + path, + read_only=False, + data_only=True, + keep_links=True, + rich_text=True, + ) + try: + sheet_summaries = { + worksheet.title: _sheet_summary( + worksheet, + cached_workbook[worksheet.title], + ) + for worksheet in workbook.worksheets + } + report["workbook"] = { + "title": workbook.properties.title or "", + "creator": workbook.properties.creator or "", + "sheets": sheets, + "selected_sheet": selected_sheet, + "sheet_summaries": sheet_summaries, + "selected_sheet_summary": sheet_summaries[selected_sheet], + } + finally: + workbook.close() + cached_workbook.close() if detected_cas: normalized = frame[detected_cas].map(normalize_cas) valid_mask = normalized.map(is_valid_cas) @@ -61,8 +179,19 @@ def main() -> int: parser = argparse.ArgumentParser(description=__doc__) parser.add_argument("input", type=Path) parser.add_argument("--cas-column") + parser.add_argument("--sheet", help="XLSX worksheet to inspect (defaults to the first)") args = parser.parse_args() - print(json.dumps(inspect(args.input, args.cas_column), ensure_ascii=False, indent=2)) + try: + print( + json.dumps( + inspect(args.input, args.cas_column, args.sheet), + ensure_ascii=False, + indent=2, + ) + ) + except (FileNotFoundError, ValueError) as exc: + print(f"Error: {exc}", file=sys.stderr) + return 2 return 0 diff --git a/README-CN.md b/README-CN.md index 2e3020b..a301acf 100644 --- a/README-CN.md +++ b/README-CN.md @@ -7,7 +7,7 @@ 一套重视数据来源追踪的化学—感官数据整理工具：把化合物工作簿扩充为可核查、可继续分析的数据表。 -AromaNexus 将化学身份、气相色谱保留指数、气味描述、阈值，以及可选的嗅觉受体实验结果串联起来。它会保留原始表格，规范化不同来源的结果，并记录每项扩充数据来自哪里。输出可作为后续统计分析、化学信息学与边界清晰的机器学习实验输入。 +AromaNexus 将化学身份、气相色谱保留指数、气味描述、阈值，以及可选的嗅觉受体实验结果串联起来。对于 XLSX 到 XLSX 的流程，它只更新指定工作表并保留受支持的工作簿内容，同时规范化不同来源的结果并记录每项扩充数据来自哪里。输出可作为后续统计分析、化学信息学与边界清晰的机器学习实验输入。 ```text XLSX / CSV / TSV @@ -27,6 +27,7 @@ XLSX / CSV / TSV - 默认记录状态、来源 URL、获取时间、缓存、版本、许可链接和诊断信息； - 使用保守的访问间隔、有限重试与持久缓存； - 原子写入、定期生成恢复检查点，默认不覆盖已有文件； +- 以工作簿为单位写入 XLSX，保留非目标工作表、未被输出字段指向的原公式、格式及常见工作表功能； - 在原有 NIST、MFFI、ChemicalBook 流程之外，增加 PubChem、Pyrfume 与 M2OR 扩充。 ## 安装 @@ -69,6 +70,9 @@ aromanexus pubchem compounds.xlsx --identifier-column "CAS Number" # 在名称查询前跳过当前数据集中的结构标签 aromanexus pubchem compounds.xlsx --identifier-column "Name" --skip-pattern '^C\d+$' +# 按精确名称选择工作表 +aromanexus pubchem compounds.xlsx --sheet "Data" --identifier-column "Name" + # 在 NIST 中寻找最接近实验计算值的保留指数 aromanexus nist-ri data.xlsx \ --cas-column "CAS Number" \ @@ -111,10 +115,16 @@ aromanexus chemicalbook-legacy compounds.xlsx --cas-column "CAS Number" aromanexus --cache-dir .cache/aromanexus --timeout 30 pubchem compounds.xlsx ``` +对于 XLSX 输入，所有表格命令默认处理工作簿顺序中的第一个工作表。使用 `--sheet "Data"` 可按区分大小写的精确名称选择其他工作表。若名称不存在，命令会在调用数据源前报错；CSV/TSV 输入不接受 `--sheet`。 + ### 输出、检查点与覆盖保护 所有表格命令默认在输入文件旁生成新文件，保留原有行序和列，再添加数据源字段。例如，PubChem 会将 `compounds.xlsx` 输出为 `compounds_pubchem.xlsx`。 +当输入与输出均为 XLSX 时，AromaNexus 从源文件的不可变副本出发，只在指定工作表叠加扩充单元格。非目标工作表的 XML 保持不变；受支持的原值、样式、行高、列宽、冻结窗格、筛选器、表格、数据验证、条件格式、工作簿属性、公式及公式缓存结果也会保留。已有公式只在某个输出字段明确指向该单元格时才会按请求替换，其他原公式不会被改写。目标表格矩形之外的合并单元格会保留；与该矩形相交的合并区域会在访问数据源前被拒绝。新获取且形似公式的文本会被转义；`.partial.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`。 ```bash @@ -128,7 +138,7 @@ aromanexus pubchem compounds.xlsx --checkpoint-every 10 aromanexus pubchem compounds.xlsx --output compounds_pubchem.xlsx --force ``` -检查点形如 `compounds_pubchem.partial.xlsx`：运行期间定期刷新，中断后保留，最终文件写入成功后删除。若目标文件已存在，命令会停止，除非显式传入 `--force`。建议输出到新文件，不要直接覆盖输入。 +检查点形如 `compounds_pubchem.partial.xlsx`：需要使用时会在访问数据源前验证，运行期间定期刷新，中断后保留，最终文件写入成功后删除。AromaNexus 只会删除本次运行自己创建且未被外部替换的检查点；无关的 `.partial` 文件不会被碰。若目标文件或本次必需的检查点路径已存在，命令会停止，除非显式传入 `--force`。即使使用 `--force`，输入路径或同一文件的别名也不能作为输出或检查点路径。 成功的 HTTP 响应与下载快照默认缓存到 `~/.cache/aromanexus`。如需更改位置，可设置 `AROMANEXUS_CACHE_DIR`，或在子命令之前传入 `--cache-dir`；更名前的缓存环境变量仍可兼容使用。 @@ -158,14 +168,14 @@ $curate-aroma-data 也可以直接运行其中只读的工作簿检查工具： ```bash -python .agents/skills/curate-aroma-data/scripts/inspect_workbook.py compounds.xlsx +python .agents/skills/curate-aroma-data/scripts/inspect_workbook.py compounds.xlsx --sheet "Data" ``` ## 旧版兼容入口 更名前的 `flavor-data` 命令与 `flavor_data_crawler` Python 命名空间继续作为兼容别名。新集成建议使用 `aromanexus`，现有自动化无需立刻重写。 -原有脚本与 Windows 启动器仍然保留，继续支持固定的工作簿布局： +原有脚本与 Windows 启动器仍然保留，继续支持固定的工作簿布局。它们默认处理第一个工作表；需要选择其他工作表时请使用统一 CLI。 | 启动器 | 脚本 | 预期工作簿 | 必需列 | 输出 | | --- | --- | --- | --- | --- | diff --git a/README.md b/README.md index 7ab462b..486ba23 100644 --- a/README.md +++ b/README.md @@ -7,7 +7,7 @@ A provenance-aware toolkit for turning compound workbooks into traceable chemical-sensory datasets. -AromaNexus connects chemical identity, gas-chromatographic retention indices, odor descriptors, thresholds, and optional olfactory-receptor assay evidence. It preserves the original table, normalizes provider results, and records where each enrichment came from. The resulting tables are practical inputs for downstream statistics, cheminformatics, and carefully scoped machine-learning experiments. +AromaNexus connects chemical identity, gas-chromatographic retention indices, odor descriptors, thresholds, and optional olfactory-receptor assay evidence. For XLSX-to-XLSX runs it updates one selected worksheet while preserving supported workbook content, normalizes provider results, and records where each enrichment came from. The resulting tables are practical inputs for downstream statistics, cheminformatics, and carefully scoped machine-learning experiments. ```text XLSX / CSV / TSV @@ -27,6 +27,7 @@ The original four workbook scripts remain available, but the package now provide - source-level status, URL, retrieval time, cache, version, license, and message fields; - conservative request pacing, bounded retries, and persistent caching; - atomic writes, periodic recovery checkpoints, and no accidental overwrite by default; +- workbook-aware XLSX output that retains non-target sheets, untargeted source formulas, formatting, and common worksheet features; - optional PubChem, Pyrfume, and M2OR enrichment alongside the original NIST, MFFI, and ChemicalBook workflows. ## Installation @@ -69,6 +70,9 @@ 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+$' +# Select a worksheet by its exact name +aromanexus pubchem compounds.xlsx --sheet "Data" --identifier-column "Name" + # Closest NIST RI to an experimentally calculated RI aromanexus nist-ri data.xlsx \ --cas-column "CAS Number" \ @@ -111,10 +115,16 @@ Run `aromanexus COMMAND --help` for column and provider-specific options. Global aromanexus --cache-dir .cache/aromanexus --timeout 30 pubchem compounds.xlsx ``` +For XLSX input, every table command processes the first worksheet in workbook order by default. Pass `--sheet "Data"` to select another worksheet by its exact, case-sensitive name. A missing worksheet is reported before any provider call; `--sheet` is not valid for CSV or TSV input. + ### Output, checkpoints, and overwrite safety Every table command writes a sibling file by default, keeps the original row order and columns, and adds provider fields. For example, `compounds.xlsx` becomes `compounds_pubchem.xlsx` after a PubChem run. +For XLSX-to-XLSX runs, AromaNexus starts from an immutable copy of the source package and overlays only enrichment cells on the selected worksheet. Non-target worksheet XML remains intact, as do supported source values, styles, row heights, column widths, freeze panes, filters, tables, data validation, conditional formatting, workbook properties, formulas, and cached formula results. Existing formulas are preserved outside cells explicitly targeted by an output field; deliberately targeting an existing formula cell replaces that cell as requested. Merged cells outside the selected tabular rectangle are preserved, while a merge intersecting that rectangle is rejected before provider access. Newly fetched formula-like text is escaped. The same preservation rules apply to `.partial.xlsx` checkpoints. + +[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. ```bash @@ -128,7 +138,7 @@ aromanexus pubchem compounds.xlsx --checkpoint-every 10 aromanexus pubchem compounds.xlsx --output compounds_pubchem.xlsx --force ``` -Checkpoints are named like `compounds_pubchem.partial.xlsx`. They are refreshed during the run, preserved if processing is interrupted, and removed after the final output succeeds. Existing destinations cause the command to stop unless `--force` is supplied. Prefer a new output path instead of overwriting the input. +Checkpoints are named like `compounds_pubchem.partial.xlsx`. A required checkpoint path is validated before provider access, refreshed during the run, preserved if processing is interrupted, and removed after the final output succeeds. AromaNexus deletes only a checkpoint created by the current run; an unrelated or externally replaced `.partial` file is left alone. Existing destinations and required checkpoint paths cause the command to stop unless `--force` is supplied. The input path—or an alias of the same file—can never be used as an output or checkpoint path, including with `--force`. Successful HTTP responses and downloaded snapshots are cached under `~/.cache/aromanexus` by default. Set `AROMANEXUS_CACHE_DIR` or pass `--cache-dir` before the subcommand to use another location; the pre-rename cache environment variables remain accepted for compatibility. @@ -158,14 +168,14 @@ The skill inspects a workbook, chooses the smallest suitable provider set, previ You can run its read-only workbook inspection helper directly: ```bash -python .agents/skills/curate-aroma-data/scripts/inspect_workbook.py compounds.xlsx +python .agents/skills/curate-aroma-data/scripts/inspect_workbook.py compounds.xlsx --sheet "Data" ``` ## Legacy compatibility The pre-rename `flavor-data` command and `flavor_data_crawler` Python namespace remain compatibility aliases. New integrations should use `aromanexus`; existing automation does not need an immediate rewrite. -The original scripts and Windows launchers are retained for existing workbook layouts: +The original scripts and Windows launchers are retained for existing workbook layouts. They process the first worksheet; use the unified CLI when another worksheet must be selected. | Launcher | Script | Expected workbook | Required column(s) | Output | | --- | --- | --- | --- | --- | diff --git a/aromanexus/cli.py b/aromanexus/cli.py index 90208b4..e9df9c3 100644 --- a/aromanexus/cli.py +++ b/aromanexus/cli.py @@ -15,7 +15,11 @@ ) from aromanexus.sources.mffi import MffiClient from aromanexus.workflows import ( + CHEMICALBOOK_VALUE_COLUMNS, + MFFI_VALUE_COLUMNS, RunSummary, + preflight_table_run, + provenance_column_names, run_chemicalbook_legacy, run_m2or, run_mffi, @@ -42,6 +46,11 @@ def _add_table_arguments(parser: argparse.ArgumentParser) -> None: parser.add_argument("input", type=Path, help="Input .xlsx, .csv, or .tsv file") + parser.add_argument( + "--sheet", + metavar="SHEET_NAME", + help="XLSX worksheet to enrich (defaults to the first worksheet in workbook order)", + ) parser.add_argument( "-o", "--output", type=Path, help="Output path (defaults to a sibling file)" ) @@ -158,6 +167,7 @@ def build_parser() -> argparse.ArgumentParser: def _common_kwargs(args: argparse.Namespace) -> dict[str, Any]: return { "output_path": args.output, + "sheet_name": args.sheet, "include_provenance": not args.no_provenance, "checkpoint_every": args.checkpoint_every, "force": args.force, @@ -244,6 +254,19 @@ def _handle_m2or(args: argparse.Namespace) -> RunSummary: def _handle_mffi(args: argparse.Namespace) -> RunSummary: + preflight_table_run( + args.input, + output_path=args.output, + suffix="_mffi_result", + sheet_name=args.sheet, + checkpoint_every=args.checkpoint_every, + force=args.force, + required_columns=(args.cas_column,), + planned_columns=( + *MFFI_VALUE_COLUMNS, + *(provenance_column_names("MFFI") if not args.no_provenance else ()), + ), + ) with MffiClient(timeout=args.timeout, headless=args.headless) as client: return run_mffi(args.input, client, cas_column=args.cas_column, **_common_kwargs(args)) @@ -260,6 +283,19 @@ def _confirm_chemicalbook_permission(args: argparse.Namespace) -> bool: def _handle_chemicalbook(args: argparse.Namespace) -> RunSummary | int: + preflight_table_run( + args.input, + output_path=args.output, + suffix="_cb_result", + sheet_name=args.sheet, + checkpoint_every=args.checkpoint_every, + force=args.force, + required_columns=(args.cas_column,), + planned_columns=( + *CHEMICALBOOK_VALUE_COLUMNS, + *(provenance_column_names("ChemicalBook") if not args.no_provenance else ()), + ), + ) if not _confirm_chemicalbook_permission(args): print("ChemicalBook compatibility run cancelled.", file=sys.stderr) return 2 diff --git a/aromanexus/excel_io.py b/aromanexus/excel_io.py index 0165e8d..4c5344c 100644 --- a/aromanexus/excel_io.py +++ b/aromanexus/excel_io.py @@ -3,19 +3,80 @@ from __future__ import annotations import os +import posixpath +import re import tempfile +import warnings +from copy import copy +from dataclasses import dataclass, field +from io import BytesIO +from numbers import Integral from pathlib import Path from typing import Any +from xml.etree import ElementTree +from zipfile import BadZipFile, ZipFile import pandas as pd +from openpyxl import load_workbook +from openpyxl.cell.cell import MergedCell +from openpyxl.utils import get_column_letter SUPPORTED_INPUTS = {".csv", ".tsv", ".xlsx"} FORMULA_PREFIXES = ("=", "+", "-", "@") +MAX_EXCEL_COLUMNS = 16_384 +_CONTEXT_ATTRIBUTE = "_aromanexus_table_context" +_UNSUPPORTED_PART_MARKERS = { + "activex": "ActiveX controls", + "comments": "cell comments", + "ctrlprops": "form controls", + "embeddings": "embedded OLE objects", + "persons": "threaded-comment authors", + "slicercaches": "slicer caches", + "slicers": "slicers", + "threadedcomments": "threaded comments", + "vml": "VML drawings", + "_xmlsignatures": "digital signatures", +} +_UNSUPPORTED_DRAWING_ELEMENTS = {"contentPart", "cxnSp", "grpSp", "sp"} +_SAFE_REMOVED_PARTS = {"xl/calcChain.xml", "xl/sharedStrings.xml"} +_SPREADSHEET_NAMESPACE = "http://schemas.openxmlformats.org/spreadsheetml/2006/main" +_OFFICE_RELATIONSHIP_NAMESPACE = ( + "http://schemas.openxmlformats.org/officeDocument/2006/relationships" +) +_PACKAGE_RELATIONSHIP_NAMESPACE = "http://schemas.openxmlformats.org/package/2006/relationships" +_CELL_BLOCK = re.compile( + rb']*\br="(?P[A-Z]{1,3}[1-9][0-9]*)")[^>]*>.*?', + re.DOTALL, +) +_FORMULA_ELEMENT = re.compile(rb"]*)?(?:/>|>.*?)", re.DOTALL) +_VALUE_ELEMENT = re.compile(rb"]*)?(?:/>|>.*?)", re.DOTALL) +_CELL_TYPE_ATTRIBUTE = re.compile(rb'\s+t="(?P[^"]*)"') -def read_table(path: str | Path) -> pd.DataFrame: - """Read a supported workbook or delimited text file.""" +@dataclass(slots=True) +class TableContext: + """Immutable input snapshot plus the cells intentionally changed by a workflow.""" + source_path: Path + sheet_name: str | None + original_columns: tuple[Any, ...] + row_count: int + template_bytes: bytes | None = None + touched_cells: set[tuple[int, Any]] = field(default_factory=set) + preservation_validated: bool = False + planned_checkpoint_path: Path | None = None + owned_checkpoint_path: Path | None = None + owned_checkpoint_identity: tuple[int, int] | None = None + checkpoint_replace_existing: bool = False + + def record_touch(self, frame: pd.DataFrame, index: Any, column: Any) -> None: + location = frame.index.get_loc(index) + if not isinstance(location, Integral): + raise ValueError("AromaNexus requires uniquely indexed input rows.") + self.touched_cells.add((int(location), column)) + + +def _validate_source(path: str | Path) -> tuple[Path, str]: source = Path(path) if not source.is_file(): raise FileNotFoundError(f"Input file does not exist: {source}") @@ -23,10 +84,85 @@ def read_table(path: str | Path) -> pd.DataFrame: if suffix not in SUPPORTED_INPUTS: supported = ", ".join(sorted(SUPPORTED_INPUTS)) raise ValueError(f"Unsupported input type {suffix!r}; expected one of: {supported}") + return source, suffix + + +def _worksheet_names(template: bytes) -> list[str]: + workbook = load_workbook( + BytesIO(template), + read_only=True, + data_only=False, + keep_links=True, + rich_text=True, + ) + try: + return [worksheet.title for worksheet in workbook.worksheets] + finally: + workbook.close() + + +def workbook_sheet_names(path: str | Path) -> list[str]: + """Return XLSX worksheet names in workbook order without modifying the file.""" + + source, suffix = _validate_source(path) + if suffix != ".xlsx": + return [] + return _worksheet_names(source.read_bytes()) + + +def read_table_with_context( + path: str | Path, + *, + sheet_name: str | None = None, +) -> tuple[pd.DataFrame, TableContext]: + """Read a table and retain an immutable snapshot for workbook-aware output.""" + + source, suffix = _validate_source(path) + template: bytes | None = None + selected_sheet: str | None = None if suffix == ".xlsx": - return pd.read_excel(source, dtype=object) - separator = "\t" if suffix == ".tsv" else "," - return pd.read_csv(source, sep=separator, dtype=object, keep_default_na=False) + template = source.read_bytes() + available = _worksheet_names(template) + if not available: + raise ValueError(f"XLSX workbook contains no worksheets: {source}") + selected_sheet = sheet_name or available[0] + if selected_sheet not in available: + choices = ", ".join(available) + raise ValueError( + f"Worksheet {selected_sheet!r} was not found in {source}. Available: {choices}" + ) + frame = pd.read_excel(BytesIO(template), sheet_name=selected_sheet, dtype=object) + else: + if sheet_name is not None: + raise ValueError("--sheet is only valid for XLSX input files.") + separator = "\t" if suffix == ".tsv" else "," + frame = pd.read_csv(source, sep=separator, dtype=object, keep_default_na=False) + + context = TableContext( + source_path=source, + sheet_name=selected_sheet, + original_columns=tuple(frame.columns), + row_count=len(frame), + template_bytes=template, + ) + frame.attrs[_CONTEXT_ATTRIBUTE] = context + return frame, context + + +def read_table(path: str | Path, *, sheet_name: str | None = None) -> pd.DataFrame: + """Read a supported workbook sheet or delimited text file.""" + + frame, _ = read_table_with_context(path, sheet_name=sheet_name) + frame.attrs.pop(_CONTEXT_ATTRIBUTE, None) + return frame + + +def record_touched_cell(frame: pd.DataFrame, index: Any, column: Any) -> None: + """Record a cell that a workflow intentionally created or replaced.""" + + context = frame.attrs.get(_CONTEXT_ATTRIBUTE) + if isinstance(context, TableContext): + context.record_touch(frame, index, column) def require_columns(frame: pd.DataFrame, *columns: str) -> None: @@ -63,18 +199,468 @@ def sanitize_frame(frame: pd.DataFrame) -> pd.DataFrame: return safe -def write_table(frame: pd.DataFrame, path: str | Path, *, force: bool = False) -> Path: - """Atomically write a table, refusing accidental overwrite by default.""" +def _same_path(first: Path, second: Path) -> bool: + if second.exists(): + try: + if os.path.samefile(first, second): + return True + except OSError: + pass + first_path = os.path.normcase(str(first.resolve())) + second_path = os.path.normcase(str(second.resolve())) + return first_path == second_path + + +def _unsupported_parts(template: bytes) -> list[str]: + features: set[str] = set() + try: + with ZipFile(BytesIO(template)) as archive: + names = archive.namelist() + for name in names: + lowered = name.lower() + for marker, label in _UNSUPPORTED_PART_MARKERS.items(): + if marker in lowered: + features.add(label) + for name in names: + lowered = name.lower() + if not (lowered.startswith("xl/drawings/") and lowered.endswith(".xml")): + continue + root = ElementTree.fromstring(archive.read(name)) + for element in root.iter(): + local_name = element.tag.rsplit("}", 1)[-1] + if local_name in _UNSUPPORTED_DRAWING_ELEMENTS: + features.add("drawing shapes") + break + except (BadZipFile, ElementTree.ParseError) as exc: + raise ValueError(f"Invalid XLSX package: {exc}") from exc + return sorted(features) + + +def _openpyxl_roundtrip_issues(template: bytes) -> list[str]: + workbook = None + roundtrip = BytesIO() + try: + with warnings.catch_warnings(record=True) as caught: + warnings.simplefilter("always") + workbook = load_workbook( + BytesIO(template), + read_only=False, + data_only=False, + keep_links=True, + rich_text=True, + ) + workbook.save(roundtrip) + finally: + if workbook is not None: + workbook.close() + + issues = { + str(item.message) + for item in caught + if "not supported" in str(item.message).lower() + or "will be removed" in str(item.message).lower() + } + with ZipFile(BytesIO(template)) as source_archive: + source_parts = {name for name in source_archive.namelist() if not name.endswith("/")} + with ZipFile(BytesIO(roundtrip.getvalue())) as saved_archive: + saved_parts = {name for name in saved_archive.namelist() if not name.endswith("/")} + removed_parts = sorted(source_parts - saved_parts - _SAFE_REMOVED_PARTS) + if removed_parts: + preview = ", ".join(removed_parts[:8]) + if len(removed_parts) > 8: + preview += f", and {len(removed_parts) - 8} more" + issues.add(f"openpyxl round-trip would remove package parts: {preview}") + return sorted(issues) + + +def _validate_target_sheet_layout( + context: TableContext, + *, + minimum_new_columns: int, +) -> None: + if context.template_bytes is None or context.sheet_name is None: + return + workbook = load_workbook( + BytesIO(context.template_bytes), + read_only=False, + data_only=False, + keep_links=True, + rich_text=True, + ) + try: + worksheet = workbook[context.sheet_name] + if minimum_new_columns and worksheet.max_column + minimum_new_columns > MAX_EXCEL_COLUMNS: + raise ValueError( + f"Worksheet {context.sheet_name!r} has no room for AromaNexus output columns " + f"within Excel's {MAX_EXCEL_COLUMNS:,}-column limit." + ) + last_table_row = context.row_count + 1 + last_table_column = len(context.original_columns) + for merged_range in worksheet.merged_cells.ranges: + intersects_rows = merged_range.min_row <= last_table_row and merged_range.max_row >= 1 + intersects_columns = ( + merged_range.min_col <= last_table_column and merged_range.max_col >= 1 + ) + if intersects_rows and intersects_columns: + raise ValueError( + f"Worksheet {context.sheet_name!r} has merged range {merged_range} inside " + "the selected tabular data; unmerge it before enrichment." + ) + finally: + workbook.close() + + +def validate_table_output( + context: TableContext, + path: str | Path, + *, + force: bool = False, + minimum_new_columns: int = 0, +) -> Path: + """Validate an output path and XLSX preservation limits before provider calls.""" destination = Path(path) suffix = destination.suffix.lower() if suffix not in SUPPORTED_INPUTS: supported = ", ".join(sorted(SUPPORTED_INPUTS)) raise ValueError(f"Unsupported output type {suffix!r}; expected one of: {supported}") + if _same_path(context.source_path, destination): + raise ValueError( + "Input and output paths must differ; --force cannot replace the source file." + ) if destination.exists() and not force: raise FileExistsError(f"Output already exists: {destination}. Pass --force to replace it.") + preserve_xlsx = ( + suffix == ".xlsx" + and context.source_path.suffix.lower() == ".xlsx" + and context.template_bytes is not None + ) + if preserve_xlsx: + _validate_target_sheet_layout( + context, + minimum_new_columns=minimum_new_columns, + ) + if preserve_xlsx and not context.preservation_validated: + unsupported = _unsupported_parts(context.template_bytes) + roundtrip_issues = _openpyxl_roundtrip_issues(context.template_bytes) + if unsupported or roundtrip_issues: + details = unsupported + roundtrip_issues + raise ValueError( + "XLSX contains features that cannot be preserved safely: " + "; ".join(details) + ) + context.preservation_validated = True + return destination + + +def _worksheet_part_map(archive: ZipFile) -> dict[str, str]: + workbook_root = ElementTree.fromstring(archive.read("xl/workbook.xml")) + relationships_root = ElementTree.fromstring(archive.read("xl/_rels/workbook.xml.rels")) + targets = { + relationship.attrib["Id"]: relationship.attrib["Target"] + for relationship in relationships_root.findall( + f"{{{_PACKAGE_RELATIONSHIP_NAMESPACE}}}Relationship" + ) + } + parts: dict[str, str] = {} + for sheet in workbook_root.findall( + f"{{{_SPREADSHEET_NAMESPACE}}}sheets/{{{_SPREADSHEET_NAMESPACE}}}sheet" + ): + relationship_id = sheet.attrib[f"{{{_OFFICE_RELATIONSHIP_NAMESPACE}}}id"] + target = targets[relationship_id].replace("\\", "/") + if target.startswith("/"): + part = target.lstrip("/") + else: + part = posixpath.normpath(posixpath.join("xl", target)) + parts[sheet.attrib["name"]] = part + return parts + + +def _formula_cached_values(worksheet_xml: bytes) -> dict[bytes, tuple[bytes, bytes | None]]: + cached: dict[bytes, tuple[bytes, bytes | None]] = {} + root = ElementTree.fromstring(worksheet_xml) + for cell in root.iter(): + if cell.tag.rsplit("}", 1)[-1] != "c": + continue + coordinate = cell.attrib.get("r") + formula = None + value = None + for child in cell: + local_name = child.tag.rsplit("}", 1)[-1] + if local_name == "f": + formula = child + elif local_name == "v": + value = child + if coordinate and formula is not None and value is not None: + cached_value = ElementTree.Element("v") + cached_value.text = value.text + cached[coordinate.encode("ascii")] = ( + ElementTree.tostring( + cached_value, + encoding="utf-8", + short_empty_elements=False, + ) + if value.text is not None + else b"", + cell.attrib.get("t", "").encode("utf-8") or None, + ) + return cached + + +def _restore_cached_values( + worksheet_xml: bytes, + cached_values: dict[bytes, tuple[bytes, bytes | None]], + touched_coordinates: set[str], +) -> bytes: + touched = {coordinate.encode("ascii") for coordinate in touched_coordinates} + + def restore(match: re.Match[bytes]) -> bytes: + coordinate = match.group("coordinate") + block = match.group(0) + cached = cached_values.get(coordinate) + if coordinate in touched or cached is None or _FORMULA_ELEMENT.search(block) is None: + return block + cached_value, cached_type = cached + opening_end = block.index(b">") + opening_tag = _CELL_TYPE_ATTRIBUTE.sub(b"", block[:opening_end]) + if cached_type is not None: + opening_tag += b' t="' + cached_type + b'"' + block = opening_tag + block[opening_end:] + current = _VALUE_ELEMENT.search(block) + if current is not None: + return block[: current.start()] + cached_value + block[current.end() :] + return block[:-4] + cached_value + block[-4:] + + return _CELL_BLOCK.sub(restore, worksheet_xml) + + +def _sheet_relationship_part(worksheet_part: str) -> str: + directory, filename = posixpath.split(worksheet_part) + return posixpath.join(directory, "_rels", f"{filename}.rels") + + +def _remove_calc_chain_reference(part_name: str, payload: bytes) -> bytes: + if part_name == "[Content_Types].xml": + root = ElementTree.fromstring(payload) + changed = False + for child in list(root): + part = child.attrib.get("PartName", "") + content_type = child.attrib.get("ContentType", "") + if part.casefold() == "/xl/calcchain.xml" or content_type.endswith("calcChain+xml"): + root.remove(child) + changed = True + if changed: + return ElementTree.tostring(root, encoding="utf-8", xml_declaration=True) + elif part_name == "xl/_rels/workbook.xml.rels": + root = ElementTree.fromstring(payload) + changed = False + for child in list(root): + if child.attrib.get("Type", "").endswith("/calcChain"): + root.remove(child) + changed = True + if changed: + return ElementTree.tostring(root, encoding="utf-8", xml_declaration=True) + return payload + + +def _build_preserved_package( + template: bytes, + saved: bytes, + *, + selected_sheet: str, + touched_coordinates: set[str], +) -> bytes: + replacements: dict[str, bytes] = {} + with ZipFile(BytesIO(template)) as source_archive, ZipFile(BytesIO(saved)) as saved_archive: + source_parts = _worksheet_part_map(source_archive) + saved_parts = _worksheet_part_map(saved_archive) + source_part = source_parts[selected_sheet] + saved_part = saved_parts[selected_sheet] + if posixpath.dirname(source_part) != posixpath.dirname(saved_part): + raise ValueError("XLSX worksheet relationship paths changed during preservation.") + cached = _formula_cached_values(source_archive.read(source_part)) + replacements[source_part] = _restore_cached_values( + saved_archive.read(saved_part), + cached, + touched_coordinates, + ) + source_relationships = _sheet_relationship_part(source_part) + saved_relationships = _sheet_relationship_part(saved_part) + if saved_relationships in saved_archive.namelist(): + replacements[source_relationships] = saved_archive.read(saved_relationships) + elif source_relationships in source_archive.namelist(): + raise ValueError("XLSX worksheet relationships were lost during preservation.") + + rewritten = BytesIO() + with ZipFile(rewritten, "w") as output_archive: + written: set[str] = set() + for item in source_archive.infolist(): + if item.filename == "xl/calcChain.xml": + continue + payload = replacements.get(item.filename, source_archive.read(item.filename)) + payload = _remove_calc_chain_reference(item.filename, payload) + output_archive.writestr( + item, + payload, + ) + written.add(item.filename) + for part_name, payload in replacements.items(): + if part_name not in written: + output_archive.writestr(part_name, payload) + return rewritten.getvalue() + + +def _excel_value(value: Any) -> Any: + if value is None: + return None + try: + if bool(pd.isna(value)): + return None + except (TypeError, ValueError): + pass + return sanitize_excel_cell(value) + + +def _column_width(frame: pd.DataFrame, column: Any) -> float: + values = [column, *frame[column].tolist()] + longest = max((len(str(value)) for value in values if value is not None), default=0) + return float(min(max(longest + 2, 12), 50)) + + +def _write_preserved_xlsx( + frame: pd.DataFrame, + temporary: Path, + context: TableContext, +) -> None: + if context.template_bytes is None or context.sheet_name is None: + raise ValueError("XLSX preservation context is incomplete.") + original_count = len(context.original_columns) + if tuple(frame.columns[:original_count]) != context.original_columns: + raise ValueError( + "A workflow reordered or removed original columns; refusing unsafe XLSX output." + ) + if len(frame.columns) > MAX_EXCEL_COLUMNS: + raise ValueError(f"XLSX output exceeds Excel's {MAX_EXCEL_COLUMNS:,}-column limit.") + + workbook = load_workbook( + BytesIO(context.template_bytes), + read_only=False, + data_only=False, + keep_links=True, + rich_text=True, + ) + try: + worksheet = workbook[context.sheet_name] + existing_positions = { + column: position for position, column in enumerate(context.original_columns, 1) + } + new_columns = list(frame.columns[original_count:]) + append_start = max(original_count, worksheet.max_column) + 1 + if append_start - 1 + len(new_columns) > MAX_EXCEL_COLUMNS: + raise ValueError(f"XLSX output exceeds Excel's {MAX_EXCEL_COLUMNS:,}-column limit.") + new_positions = {column: append_start + offset for offset, column in enumerate(new_columns)} + + for column, position in new_positions.items(): + header = worksheet.cell(row=1, column=position) + header.value = _excel_value(column) + if original_count: + source_header = worksheet.cell(row=1, column=original_count) + if source_header.has_style: + header._style = copy(source_header._style) + column_dimension = worksheet.column_dimensions[get_column_letter(position)] + # Excel-generated files can use a non-zero base style. A fresh + # ColumnDimension otherwise registers a new all-zero style whose + # index exists only in openpyxl's rewritten styles.xml. Reuse the + # workbook's real base style so the immutable source styles part + # remains valid and byte-preservable. + column_dimension._style = copy(workbook._cell_styles[0]) + column_dimension.width = _column_width(frame, column) + + frame_positions = {column: position for position, column in enumerate(frame.columns)} + touched_coordinates: set[str] = set() + for row_position, column in sorted( + context.touched_cells, + key=lambda item: (item[0], str(item[1])), + ): + output_column = existing_positions.get(column, new_positions.get(column)) + frame_column = frame_positions.get(column) + if output_column is None or frame_column is None: + raise ValueError(f"Touched column {column!r} is missing from the output frame.") + cell = worksheet.cell(row=row_position + 2, column=output_column) + if isinstance(cell, MergedCell): + raise ValueError( + f"Cannot write {column!r} at row {row_position + 2}: " + "cell is inside a merged range." + ) + cell.value = _excel_value(frame.iat[row_position, frame_column]) + touched_coordinates.add(f"{get_column_letter(output_column)}{row_position + 2}") + + workbook.save(temporary) + finally: + workbook.close() + + temporary.write_bytes( + _build_preserved_package( + context.template_bytes, + temporary.read_bytes(), + selected_sheet=context.sheet_name, + touched_coordinates=touched_coordinates, + ) + ) + + validation = load_workbook( + temporary, + read_only=True, + data_only=False, + keep_links=True, + rich_text=True, + ) + try: + if context.sheet_name not in validation.sheetnames: + raise ValueError(f"Saved XLSX is missing worksheet {context.sheet_name!r}.") + finally: + validation.close() + + +def _commit_temporary( + temporary: Path, + destination: Path, + *, + force: bool, +) -> None: + if force: + os.replace(temporary, destination) + return + try: + if os.name == "nt": + os.rename(temporary, destination) + else: + os.link(temporary, destination) + temporary.unlink() + except FileExistsError as exc: + raise FileExistsError( + f"Output already exists: {destination}. Pass --force to replace it." + ) from exc + + +def write_table( + frame: pd.DataFrame, + path: str | Path, + *, + force: bool = False, + context: TableContext | None = None, +) -> Path: + """Atomically write a table, preserving a source XLSX when context is supplied.""" + + destination = Path(path) + suffix = destination.suffix.lower() + if suffix not in SUPPORTED_INPUTS: + supported = ", ".join(sorted(SUPPORTED_INPUTS)) + raise ValueError(f"Unsupported output type {suffix!r}; expected one of: {supported}") + if context is not None: + validate_table_output(context, destination, force=force) + elif destination.exists() and not force: + raise FileExistsError(f"Output already exists: {destination}. Pass --force to replace it.") destination.parent.mkdir(parents=True, exist_ok=True) - safe = sanitize_frame(frame) handle = tempfile.NamedTemporaryFile( prefix=f".{destination.stem}-", suffix=destination.suffix, @@ -84,13 +670,22 @@ def write_table(frame: pd.DataFrame, path: str | Path, *, force: bool = False) - temporary = Path(handle.name) handle.close() try: - if suffix == ".xlsx": - safe.to_excel(temporary, index=False) + preserve_xlsx = ( + suffix == ".xlsx" + and context is not None + and context.source_path.suffix.lower() == ".xlsx" + and context.template_bytes is not None + ) + if preserve_xlsx: + _write_preserved_xlsx(frame, temporary, context) else: - separator = "\t" if suffix == ".tsv" else "," - safe.to_csv(temporary, sep=separator, index=False, encoding="utf-8-sig") - os.replace(temporary, destination) - except Exception: + safe = sanitize_frame(frame) + if suffix == ".xlsx": + safe.to_excel(temporary, index=False) + else: + separator = "\t" if suffix == ".tsv" else "," + safe.to_csv(temporary, sep=separator, index=False, encoding="utf-8-sig") + _commit_temporary(temporary, destination, force=force) + finally: temporary.unlink(missing_ok=True) - raise return destination diff --git a/aromanexus/workflows.py b/aromanexus/workflows.py index e559440..ad5d8ee 100644 --- a/aromanexus/workflows.py +++ b/aromanexus/workflows.py @@ -14,9 +14,12 @@ import pandas as pd from aromanexus.excel_io import ( + TableContext, derive_output_path, - read_table, + read_table_with_context, + record_touched_cell, require_columns, + validate_table_output, write_table, ) from aromanexus.identifiers import clean_text, is_valid_cas, normalize_cas @@ -24,6 +27,33 @@ from aromanexus.sources.chemicalbook import ManualVerificationRequired ProgressCallback = Callable[[int, int, str], None] +PROVENANCE_COLUMN_SUFFIXES = ( + "Status", + "Source URL", + "Retrieved At", + "Cache Hit", + "Version", + "License URL", + "Message", +) +M2OR_VALUE_COLUMNS = ( + "M2OR Pair Count", + "M2OR Responsive Count", + "M2OR Species", + "M2OR Human Responsive Receptors", + "M2OR DOIs", +) +MFFI_VALUE_COLUMNS = ( + "Chinese Name", + "English Name", + "Sensory Characteristics", + "In Water", +) +CHEMICALBOOK_VALUE_COLUMNS = ( + "CB_Odor_Desc", + "CB_Odor_Threshold", + "CB_Odor_Type", +) @dataclass(slots=True) @@ -47,6 +77,12 @@ def _flatten(value: Any) -> Any: return value +def provenance_column_names(prefix: str) -> tuple[str, ...]: + """Return the stable flattened provenance schema for one provider.""" + + return tuple(f"{prefix} {suffix}" for suffix in PROVENANCE_COLUMN_SUFFIXES) + + def _set_cell(frame: pd.DataFrame, index: Any, column: str, value: Any) -> None: """Create enrichment columns as object dtype so mixed values remain valid.""" @@ -54,6 +90,7 @@ def _set_cell(frame: pd.DataFrame, index: Any, column: str, value: Any) -> None: frame[column] = pd.Series([None] * len(frame), index=frame.index, dtype=object) elif frame[column].dtype != object: frame[column] = frame[column].astype(object) + record_touched_cell(frame, index, column) frame.at[index, column] = _flatten(value) @@ -89,37 +126,112 @@ def _prepare_run( output_path: str | Path | None, *, suffix: str, + sheet_name: str | None, + checkpoint_every: int, force: bool, -) -> tuple[pd.DataFrame, Path]: - frame = read_table(input_path) + planned_columns: Iterable[str] = (), +) -> tuple[pd.DataFrame, Path, TableContext]: + frame, context = read_table_with_context(input_path, sheet_name=sheet_name) destination = Path(output_path) if output_path else derive_output_path(input_path, suffix) - if destination.exists() and not force: - raise FileExistsError(f"Output already exists: {destination}. Pass --force to replace it.") - return frame, destination + minimum_new_columns = ( + sum(column not in frame.columns for column in dict.fromkeys(planned_columns)) + if len(frame) + else 0 + ) + validate_table_output( + context, + destination, + force=force, + minimum_new_columns=minimum_new_columns, + ) + if checkpoint_every > 0 and len(frame) >= checkpoint_every: + partial = destination.with_name(f"{destination.stem}.partial{destination.suffix}") + validate_table_output( + context, + partial, + force=force, + minimum_new_columns=minimum_new_columns, + ) + context.planned_checkpoint_path = partial + context.checkpoint_replace_existing = force + return frame, destination, context + + +def preflight_table_run( + input_path: str | Path, + *, + output_path: str | Path | None, + suffix: str, + sheet_name: str | None, + checkpoint_every: int, + force: bool, + required_columns: tuple[str, ...] = (), + planned_columns: tuple[str, ...] = (), +) -> Path: + """Validate a CLI table run before launching an interactive provider.""" + + frame, destination, _ = _prepare_run( + input_path, + output_path, + suffix=suffix, + sheet_name=sheet_name, + checkpoint_every=checkpoint_every, + force=force, + planned_columns=planned_columns, + ) + require_columns(frame, *required_columns) + return destination + + +def _path_identity(path: Path) -> tuple[int, int] | None: + try: + metadata = path.stat() + except FileNotFoundError: + return None + return metadata.st_dev, metadata.st_ino def _checkpoint( frame: pd.DataFrame, destination: Path, + context: TableContext, current: int, checkpoint_every: int, ) -> None: if checkpoint_every <= 0 or current % checkpoint_every: return - partial = destination.with_name(f"{destination.stem}.partial{destination.suffix}") - write_table(frame, partial, force=True) + partial = context.planned_checkpoint_path + if partial is None: + raise RuntimeError("Checkpoint path was not validated before provider processing.") + still_owned = ( + context.owned_checkpoint_path == partial + and context.owned_checkpoint_identity is not None + and _path_identity(partial) == context.owned_checkpoint_identity + ) + replace_existing = True if still_owned else context.checkpoint_replace_existing + write_table(frame, partial, force=replace_existing, context=context) + context.owned_checkpoint_path = partial + context.owned_checkpoint_identity = _path_identity(partial) def _finish( frame: pd.DataFrame, destination: Path, + context: TableContext, statuses: Counter[str], *, force: bool, ) -> RunSummary: - write_table(frame, destination, force=force) - partial = destination.with_name(f"{destination.stem}.partial{destination.suffix}") - partial.unlink(missing_ok=True) + write_table(frame, destination, force=force, context=context) + partial = context.owned_checkpoint_path + if ( + partial is not None + and context.owned_checkpoint_identity is not None + and _path_identity(partial) == context.owned_checkpoint_identity + ): + partial.unlink(missing_ok=True) + context.owned_checkpoint_path = None + context.owned_checkpoint_identity = None return RunSummary(destination, len(frame), dict(sorted(statuses.items()))) @@ -133,6 +245,7 @@ def run_nist_ri( client: Any, *, output_path: str | Path | None = None, + sheet_name: str | None = None, cas_column: str = "CAS Number", calculated_ri_column: str = "Calculated RI", result_column: str = "NIST RI", @@ -141,7 +254,22 @@ def run_nist_ri( force: bool = False, progress: ProgressCallback = console_progress, ) -> RunSummary: - frame, destination = _prepare_run(input_path, output_path, suffix="_nist_result", force=force) + frame, destination, context = _prepare_run( + input_path, + output_path, + suffix="_nist_result", + sheet_name=sheet_name, + checkpoint_every=checkpoint_every, + force=force, + planned_columns=( + result_column, + *( + ("NIST RI Candidates", *provenance_column_names("NIST")) + if include_provenance + else () + ), + ), + ) require_columns(frame, cas_column, calculated_ri_column) statuses: Counter[str] = Counter() total = len(frame) @@ -155,8 +283,8 @@ def run_nist_ri( _apply_provenance(frame, index, result, prefix="NIST") statuses[result.status] += 1 progress(current, total, f"NIST RI: {row[cas_column]} -> {frame.at[index, result_column]}") - _checkpoint(frame, destination, current, checkpoint_every) - return _finish(frame, destination, statuses, force=force) + _checkpoint(frame, destination, context, current, checkpoint_every) + return _finish(frame, destination, context, statuses, force=force) def _legacy_name_status(result: LookupResult) -> str: @@ -175,6 +303,7 @@ def run_resolve_cas( client: Any, *, output_path: str | Path | None = None, + sheet_name: str | None = None, name_column: str = "Name", result_column: str = "Found CAS", include_provenance: bool = True, @@ -182,7 +311,18 @@ def run_resolve_cas( force: bool = False, progress: ProgressCallback = console_progress, ) -> RunSummary: - frame, destination = _prepare_run(input_path, output_path, suffix="_with_cas", force=force) + frame, destination, context = _prepare_run( + input_path, + output_path, + suffix="_with_cas", + sheet_name=sheet_name, + checkpoint_every=checkpoint_every, + force=force, + planned_columns=( + result_column, + *(provenance_column_names("NIST") if include_provenance else ()), + ), + ) require_columns(frame, name_column) statuses: Counter[str] = Counter() total = len(frame) @@ -194,8 +334,8 @@ def run_resolve_cas( statuses[result.status] += 1 label = f"Resolve name: {row[name_column]} -> {frame.at[index, result_column]}" progress(current, total, label) - _checkpoint(frame, destination, current, checkpoint_every) - return _finish(frame, destination, statuses, force=force) + _checkpoint(frame, destination, context, current, checkpoint_every) + return _finish(frame, destination, context, statuses, force=force) PUBCHEM_COLUMN_MAP = { @@ -283,6 +423,7 @@ def run_pubchem( client: Any, *, output_path: str | Path | None = None, + sheet_name: str | None = None, identifier_column: str = "CAS Number", resolved_cas_column: str = "Resolved CAS", skip_patterns: Iterable[str] | str | None = None, @@ -293,7 +434,21 @@ def run_pubchem( progress: ProgressCallback = console_progress, ) -> RunSummary: compiled_skip_patterns = _compile_skip_patterns(skip_patterns) - frame, destination = _prepare_run(input_path, output_path, suffix="_pubchem", force=force) + frame, destination, context = _prepare_run( + input_path, + output_path, + suffix="_pubchem", + sheet_name=sheet_name, + checkpoint_every=checkpoint_every, + force=force, + planned_columns=( + *PUBCHEM_COLUMN_MAP.values(), + "PubChem CAS Resolution", + "PubChem CAS Candidate Count", + resolved_cas_column, + *(provenance_column_names("PubChem") if include_provenance else ()), + ), + ) require_columns(frame, identifier_column) statuses: Counter[str] = Counter() total = len(frame) @@ -329,8 +484,8 @@ def run_pubchem( ) statuses[result.status] += 1 progress(current, total, f"PubChem: {identifier} -> {result.status}") - _checkpoint(frame, destination, current, checkpoint_every) - return _finish(frame, destination, statuses, force=force) + _checkpoint(frame, destination, context, current, checkpoint_every) + return _finish(frame, destination, context, statuses, force=force) def run_pyrfume( @@ -339,6 +494,7 @@ def run_pyrfume( *, pubchem_client: Any | None = None, output_path: str | Path | None = None, + sheet_name: str | None = None, cid_column: str = "PubChem CID", identifier_column: str = "CAS Number", archives: list[str] | None = None, @@ -347,7 +503,43 @@ def run_pyrfume( force: bool = False, progress: ProgressCallback = console_progress, ) -> RunSummary: - frame, destination = _prepare_run(input_path, output_path, suffix="_pyrfume", force=force) + frame, destination, context = _prepare_run( + input_path, + output_path, + suffix="_pyrfume", + sheet_name=sheet_name, + checkpoint_every=checkpoint_every, + force=force, + planned_columns=( + *( + f"Pyrfume {archive} {field}" + for archive in ( + str(item).strip().casefold() + for item in (archives or ("aromadb", "flavornet", "superscent")) + ) + for field in ( + "Source Title", + "Source Reference", + "Source Authors", + "Source Notes", + "License Note", + "Manifest URL", + "Present", + "Name", + "IUPAC Name", + "Descriptors", + ) + ), + "Pyrfume Archives Matched", + *(PUBCHEM_COLUMN_MAP.values() if pubchem_client is not None else ()), + *(provenance_column_names("Pyrfume") if include_provenance else ()), + *( + provenance_column_names("PubChem") + if include_provenance and pubchem_client is not None + else () + ), + ), + ) if cid_column not in frame.columns: require_columns(frame, identifier_column) if pubchem_client is None: @@ -383,8 +575,8 @@ def run_pyrfume( ) statuses[result.status] += 1 progress(current, total, f"Pyrfume CID {cid}: {result.status}") - _checkpoint(frame, destination, current, checkpoint_every) - return _finish(frame, destination, statuses, force=force) + _checkpoint(frame, destination, context, current, checkpoint_every) + return _finish(frame, destination, context, statuses, force=force) def run_m2or( @@ -392,13 +584,25 @@ def run_m2or( client: Any, *, output_path: str | Path | None = None, + sheet_name: str | None = None, cas_column: str = "CAS Number", include_provenance: bool = True, checkpoint_every: int = 25, force: bool = False, progress: ProgressCallback = console_progress, ) -> RunSummary: - frame, destination = _prepare_run(input_path, output_path, suffix="_m2or", force=force) + frame, destination, context = _prepare_run( + input_path, + output_path, + suffix="_m2or", + sheet_name=sheet_name, + checkpoint_every=checkpoint_every, + force=force, + planned_columns=( + *M2OR_VALUE_COLUMNS, + *(provenance_column_names("M2OR") if include_provenance else ()), + ), + ) require_columns(frame, cas_column) statuses: Counter[str] = Counter() total = len(frame) @@ -413,8 +617,8 @@ def run_m2or( ) statuses[result.status] += 1 progress(current, total, f"M2OR: {row[cas_column]} -> {result.status}") - _checkpoint(frame, destination, current, checkpoint_every) - return _finish(frame, destination, statuses, force=force) + _checkpoint(frame, destination, context, current, checkpoint_every) + return _finish(frame, destination, context, statuses, force=force) def run_mffi( @@ -422,6 +626,7 @@ def run_mffi( client: Any, *, output_path: str | Path | None = None, + sheet_name: str | None = None, cas_column: str = "CAS Number", include_provenance: bool = True, checkpoint_every: int = 10, @@ -430,7 +635,18 @@ def run_mffi( progress: ProgressCallback = console_progress, sleep: Callable[[float], None] = time.sleep, ) -> RunSummary: - frame, destination = _prepare_run(input_path, output_path, suffix="_mffi_result", force=force) + frame, destination, context = _prepare_run( + input_path, + output_path, + suffix="_mffi_result", + sheet_name=sheet_name, + checkpoint_every=checkpoint_every, + force=force, + planned_columns=( + *MFFI_VALUE_COLUMNS, + *(provenance_column_names("MFFI") if include_provenance else ()), + ), + ) require_columns(frame, cas_column) statuses: Counter[str] = Counter() total = len(frame) @@ -452,10 +668,10 @@ def run_mffi( ) statuses[result.status] += 1 progress(current, total, f"MFFI: {row[cas_column]} -> {result.status}") - _checkpoint(frame, destination, current, checkpoint_every) + _checkpoint(frame, destination, context, current, checkpoint_every) if delay > 0 and current < total: sleep(delay) - return _finish(frame, destination, statuses, force=force) + return _finish(frame, destination, context, statuses, force=force) def run_chemicalbook_legacy( @@ -463,6 +679,7 @@ def run_chemicalbook_legacy( client: Any, *, output_path: str | Path | None = None, + sheet_name: str | None = None, cas_column: str = "CAS Number", include_provenance: bool = True, checkpoint_every: int = 5, @@ -472,7 +689,18 @@ def run_chemicalbook_legacy( prompt: Callable[[str], str] = input, sleep: Callable[[float], None] = time.sleep, ) -> RunSummary: - frame, destination = _prepare_run(input_path, output_path, suffix="_cb_result", force=force) + frame, destination, context = _prepare_run( + input_path, + output_path, + suffix="_cb_result", + sheet_name=sheet_name, + checkpoint_every=checkpoint_every, + force=force, + planned_columns=( + *CHEMICALBOOK_VALUE_COLUMNS, + *(provenance_column_names("ChemicalBook") if include_provenance else ()), + ), + ) require_columns(frame, cas_column) statuses: Counter[str] = Counter() total = len(frame) @@ -508,7 +736,7 @@ def run_chemicalbook_legacy( ) statuses[result.status] += 1 progress(current, total, f"ChemicalBook: {row[cas_column]} -> {result.status}") - _checkpoint(frame, destination, current, checkpoint_every) + _checkpoint(frame, destination, context, current, checkpoint_every) if delay > 0 and current < total: sleep(delay) - return _finish(frame, destination, statuses, force=force) + return _finish(frame, destination, context, statuses, force=force) diff --git a/tests/test_cli.py b/tests/test_cli.py index e77aa85..d6875e4 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -1,3 +1,5 @@ +import pandas as pd + from aromanexus.cli import main @@ -42,6 +44,8 @@ def fake_run_pubchem(input_file, client, **kwargs): "^Total$", "--resolved-cas-column", "Curated CAS", + "--sheet", + "Data", ] ) @@ -49,3 +53,42 @@ 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"]["sheet_name"] == "Data" + + +def test_mffi_preflight_runs_before_browser_construction(monkeypatch, tmp_path): + input_path = tmp_path / "input.xlsx" + pd.DataFrame({"CAS Number": ["100-52-7"]}).to_excel(input_path, index=False) + state = {"constructed": 0} + + class BrowserMustNotStart: + def __init__(self, **_kwargs): + state["constructed"] += 1 + raise AssertionError("MFFI browser started before workbook preflight") + + monkeypatch.setattr("aromanexus.cli.MffiClient", BrowserMustNotStart) + + code = main(["mffi", str(input_path), "--sheet", "Missing"]) + + assert code == 2 + assert state["constructed"] == 0 + + +def test_chemicalbook_preflight_runs_before_permission_prompt(monkeypatch, tmp_path): + input_path = tmp_path / "input.xlsx" + pd.DataFrame({"CAS Number": ["100-52-7"]}).to_excel(input_path, index=False) + state = {"prompted": 0} + + def permission_must_not_be_requested(_args): + state["prompted"] += 1 + raise AssertionError("Permission prompt ran before workbook preflight") + + monkeypatch.setattr( + "aromanexus.cli._confirm_chemicalbook_permission", + permission_must_not_be_requested, + ) + + code = main(["chemicalbook-legacy", str(input_path), "--sheet", "Missing"]) + + assert code == 2 + assert state["prompted"] == 0 diff --git a/tests/test_skill.py b/tests/test_skill.py index 6c9e43a..7b101f9 100644 --- a/tests/test_skill.py +++ b/tests/test_skill.py @@ -1,9 +1,14 @@ import json +import re import subprocess import sys +from io import BytesIO from pathlib import Path +from zipfile import ZipFile +import openpyxl import pandas as pd +from openpyxl.styles import Font REPO_ROOT = Path(__file__).resolve().parents[1] SKILL_ROOT = REPO_ROOT / ".agents" / "skills" / "curate-aroma-data" @@ -32,5 +37,102 @@ def test_skill_inspector_reports_identifier_quality(tmp_path: Path): ) report = json.loads(completed.stdout) assert report["rows"] == 3 + assert report["workbook"]["sheets"] == ["Sheet1"] + assert report["workbook"]["selected_sheet"] == "Sheet1" assert report["cas"]["valid"] == 1 assert report["cas"]["invalid"] == 1 + + +def test_skill_inspector_reports_workbook_fidelity_evidence_and_clean_errors(tmp_path: Path): + workbook_path = tmp_path / "input.xlsx" + workbook = openpyxl.Workbook() + cover = workbook.active + cover.title = "Cover" + cover["A1"] = "Read me" + data = workbook.create_sheet("Data") + data.append(["CAS Number", "Value"]) + data.append(["100-52-7", 10]) + data.append(["", "=B2*2"]) + data["A1"].font = Font(bold=True) + data.column_dimensions["A"].width = 20 + data.row_dimensions[1].height = 24 + data.freeze_panes = "A2" + data.auto_filter.ref = "A1:B3" + data.merge_cells("A5:B5") + data["A5"] = "Outside table" + workbook.properties.title = "Inspector fixture" + workbook.properties.creator = "AromaNexus tests" + workbook.save(workbook_path) + workbook.close() + rewritten = BytesIO() + with ZipFile(BytesIO(workbook_path.read_bytes())) as source, ZipFile(rewritten, "w") as output: + for item in source.infolist(): + payload = source.read(item.filename) + if item.filename == "xl/worksheets/sheet2.xml": + payload, replaced = re.subn( + rb"|>\s*)", + b"20", + payload, + count=1, + ) + assert replaced == 1 + output.writestr(item, payload) + workbook_path.write_bytes(rewritten.getvalue()) + script = SKILL_ROOT / "scripts" / "inspect_workbook.py" + + completed = subprocess.run( + [sys.executable, str(script), str(workbook_path), "--sheet", "Data"], + cwd=REPO_ROOT, + check=True, + capture_output=True, + text=True, + ) + report = json.loads(completed.stdout) + workbook_report = report["workbook"] + assert workbook_report["title"] == "Inspector fixture" + assert workbook_report["creator"] == "AromaNexus tests" + assert workbook_report["selected_sheet"] == "Data" + assert set(workbook_report["sheet_summaries"]) == {"Cover", "Data"} + selected = workbook_report["selected_sheet_summary"] + assert selected["formula_cells"] == 1 + assert selected["cached_formula_results"] == 1 + assert len(selected["formula_cache_sha256"]) == 64 + assert selected["styled_cells"] >= 1 + assert selected["row_heights"] == {"1": 24.0} + assert selected["column_widths"] == {"A": 20.0} + assert selected["freeze_panes"] == "A2" + assert selected["auto_filter"] == "A1:B3" + assert selected["merged_ranges"] == ["A5:B5"] + assert len(selected["content_sha256"]) == 64 + + missing = subprocess.run( + [sys.executable, str(script), str(workbook_path), "--sheet", "Missing"], + cwd=REPO_ROOT, + check=False, + capture_output=True, + text=True, + ) + assert missing.returncode == 2 + assert missing.stdout == "" + assert missing.stderr.startswith("Error: Worksheet 'Missing' was not found") + assert "Traceback" not in missing.stderr + + missing_cas = subprocess.run( + [ + sys.executable, + str(script), + str(workbook_path), + "--sheet", + "Data", + "--cas-column", + "Missing CAS", + ], + cwd=REPO_ROOT, + check=False, + capture_output=True, + text=True, + ) + assert missing_cas.returncode == 2 + assert missing_cas.stdout == "" + assert missing_cas.stderr.startswith("Error: CAS column 'Missing CAS' was not found") + assert "Traceback" not in missing_cas.stderr diff --git a/tests/test_xlsx_preservation.py b/tests/test_xlsx_preservation.py new file mode 100644 index 0000000..7af06cc --- /dev/null +++ b/tests/test_xlsx_preservation.py @@ -0,0 +1,748 @@ +import os +import re +from io import BytesIO +from pathlib import Path +from zipfile import ZipFile + +import openpyxl +import pandas as pd +import pytest +from openpyxl.comments import Comment +from openpyxl.formatting.rule import CellIsRule +from openpyxl.styles import Alignment, Border, Font, PatternFill, Side +from openpyxl.worksheet.datavalidation import DataValidation +from openpyxl.worksheet.table import Table, TableStyleInfo + +from aromanexus import excel_io +from aromanexus.models import LookupResult +from aromanexus.workflows import run_pubchem + + +class NoLookupPubChem: + def __init__(self) -> None: + self.calls = 0 + + def lookup(self, identifier, include_odor=True): + self.calls += 1 + raise AssertionError(f"Unexpected PubChem lookup: {identifier!r}") + + +class FormulaPubChem: + def lookup(self, identifier, include_odor=True): + return LookupResult( + provider="PubChem", + values={ + "title": '=HYPERLINK("https://example.test")', + "cas_numbers": ["110-54-3"], + }, + ) + + +class InterruptedPubChem: + def __init__(self) -> None: + self.calls = 0 + + def lookup(self, identifier, include_odor=True): + self.calls += 1 + if self.calls == 2: + raise RuntimeError("simulated interruption") + return LookupResult( + provider="PubChem", + values={"title": "First result", "cas_numbers": ["110-54-3"]}, + ) + + +def _silent(*_): + return None + + +def _create_preservation_workbook(path: Path, *, cover_first: bool = False) -> None: + workbook = openpyxl.Workbook() + data = workbook.active + data.title = "Data" + if cover_first: + cover = workbook.create_sheet("Cover", 0) + cover["A1"] = "AromaNexus preservation fixture" + cover["A2"] = "Do not modify" + + data.append(["Name", "Measured value", "Calculated value"]) + data.append(["C6", 10, "=B2*2"]) + data.append(["C7", 20, "=B3*2"]) + + header_fill = PatternFill("solid", fgColor="1F4E78") + row_fill = PatternFill("solid", fgColor="FFF2CC") + thin = Side(style="thin", color="000000") + for cell in data[1]: + cell.font = Font(bold=True, color="FFFFFF") + cell.fill = header_fill + cell.alignment = Alignment(horizontal="center") + cell.border = Border(bottom=thin) + for cell in data[2]: + cell.fill = row_fill + data["B2"].number_format = "0.00" + data.column_dimensions["A"].width = 18 + data.column_dimensions["A"].font = Font(italic=True) + data.column_dimensions["B"].width = 22 + data.column_dimensions["C"].width = 24 + data.row_dimensions[1].height = 28 + data.row_dimensions[2].height = 21 + data.freeze_panes = "A2" + data.auto_filter.ref = "A1:C3" + + table = Table(displayName="DataTable", ref="A1:C3") + table.tableStyleInfo = TableStyleInfo( + name="TableStyleMedium2", + showFirstColumn=False, + showLastColumn=False, + showRowStripes=True, + showColumnStripes=False, + ) + data.add_table(table) + + validation = DataValidation( + type="whole", + operator="between", + formula1="0", + formula2="100", + ) + validation.add("B2:B3") + data.add_data_validation(validation) + data.conditional_formatting.add( + "B2:B3", + CellIsRule( + operator="greaterThan", + formula=["15"], + fill=PatternFill("solid", fgColor="C6EFCE"), + ), + ) + + notes = workbook.create_sheet("Notes") + notes.append(["Field", "Value"]) + notes.append(["Owner", "Tianyuan Chen"]) + notes.append(["Name length", "=LEN(B2)"]) + notes.merge_cells("A4:B4") + notes["A4"] = "Preservation regression fixture" + + workbook.properties.title = "AromaNexus preservation fixture" + workbook.properties.creator = "Tianyuan Chen" + workbook.save(path) + workbook.close() + + +def _header_positions(worksheet) -> dict[object, int]: + return {cell.value: cell.column for cell in worksheet[1] if cell.value is not None} + + +def _rewrite_xlsx( + path: Path, + replacements: dict[str, bytes], + additions: dict[str, bytes] | None = None, +) -> None: + rewritten = BytesIO() + with ZipFile(BytesIO(path.read_bytes())) as source, ZipFile(rewritten, "w") as output: + existing = set(source.namelist()) + for item in source.infolist(): + output.writestr(item, replacements.get(item.filename, source.read(item.filename))) + for name, payload in (additions or {}).items(): + if name not in existing: + output.writestr(name, payload) + path.write_bytes(rewritten.getvalue()) + + +def _xlsx_part(path: Path, name: str) -> bytes: + with ZipFile(path) as archive: + return archive.read(name) + + +def _inject_formula_cache( + worksheet_xml: bytes, + coordinate: str, + value: str, + *, + cell_type: str | None = None, +) -> bytes: + pattern = re.compile( + rb']*\br="' + coordinate.encode("ascii") + rb'")[^>]*>.*?', + re.DOTALL, + ) + + def replace(match: re.Match[bytes]) -> bytes: + block = match.group(0) + assert b"") + opening = re.sub(rb'\s+t="[^"]*"', b"", block[:opening_end]) + if cell_type is not None: + opening += f' t="{cell_type}"'.encode() + block = opening + block[opening_end:] + cached = f"{value}".encode() + block, count = re.subn(rb"]*)?(?:/>|>.*?)", cached, block, count=1) + assert count == 1 + return block + + result, count = pattern.subn(replace, worksheet_xml, count=1) + assert count == 1 + return result + + +def test_all_skipped_xlsx_run_preserves_workbook_structure(tmp_path: Path): + source = tmp_path / "preservation_input.xlsx" + destination = tmp_path / "preservation_output.xlsx" + _create_preservation_workbook(source) + client = NoLookupPubChem() + + run_pubchem( + source, + client, + output_path=destination, + sheet_name="Data", + identifier_column="Name", + skip_patterns=[r"^C\d+$"], + include_odor=False, + include_provenance=False, + checkpoint_every=1, + progress=_silent, + ) + + assert client.calls == 0 + assert not (tmp_path / "preservation_output.partial.xlsx").exists() + workbook = openpyxl.load_workbook(destination, data_only=False) + try: + assert workbook.sheetnames == ["Data", "Notes"] + assert workbook.properties.title == "AromaNexus preservation fixture" + assert workbook.properties.creator == "Tianyuan Chen" + data = workbook["Data"] + notes = workbook["Notes"] + assert [data.cell(1, column).value for column in range(1, 4)] == [ + "Name", + "Measured value", + "Calculated value", + ] + assert [data.cell(row, 1).value for row in range(2, 4)] == ["C6", "C7"] + assert data["C2"].value == "=B2*2" + assert data["C3"].value == "=B3*2" + assert data["C2"].data_type == "f" + assert data["A1"].font.bold is True + assert data["A1"].fill.fgColor.rgb == "001F4E78" + assert data["A2"].fill.fgColor.rgb == "00FFF2CC" + assert data["A1"].border.bottom.style == "thin" + assert data["B2"].number_format == "0.00" + assert data.column_dimensions["A"].width == 18 + assert data.column_dimensions["A"].font.italic is True + assert data.column_dimensions["B"].width == 22 + assert data.column_dimensions["C"].width == 24 + assert data.row_dimensions[1].height == 28 + assert data.row_dimensions[2].height == 21 + assert data.freeze_panes == "A2" + assert data.auto_filter.ref == "A1:C3" + assert list(data.tables) == ["DataTable"] + assert data.tables["DataTable"].ref == "A1:C3" + validation = data.data_validations.dataValidation[0] + assert (str(validation.sqref), validation.type, validation.operator) == ( + "B2:B3", + "whole", + "between", + ) + conditional_range = next(iter(data.conditional_formatting)) + assert str(conditional_range.sqref) == "B2:B3" + assert notes["B2"].value == "Tianyuan Chen" + assert notes["B3"].value == "=LEN(B2)" + assert notes["B3"].data_type == "f" + assert "A4:B4" in {str(item) for item in notes.merged_cells.ranges} + headers = _header_positions(data) + assert set(headers) >= { + "PubChem CAS Resolution", + "PubChem CAS Candidate Count", + "Resolved CAS", + } + finally: + workbook.close() + + +def test_formula_injection_applies_only_to_new_enrichment(tmp_path: Path): + source = tmp_path / "input.xlsx" + destination = tmp_path / "output.xlsx" + _create_preservation_workbook(source) + + run_pubchem( + source, + FormulaPubChem(), + output_path=destination, + sheet_name="Data", + identifier_column="Name", + include_odor=False, + include_provenance=False, + checkpoint_every=0, + progress=_silent, + ) + + workbook = openpyxl.load_workbook(destination, data_only=False) + try: + data = workbook["Data"] + title_column = _header_positions(data)["PubChem Title"] + assert data["C2"].value == "=B2*2" + assert data["C2"].data_type == "f" + assert data.cell(2, title_column).value == '\'=HYPERLINK("https://example.test")' + assert data.cell(2, title_column).data_type == "s" + finally: + workbook.close() + + +def test_explicit_sheet_selects_non_first_worksheet(tmp_path: Path): + source = tmp_path / "input.xlsx" + destination = tmp_path / "output.xlsx" + _create_preservation_workbook(source, cover_first=True) + client = NoLookupPubChem() + + run_pubchem( + source, + client, + output_path=destination, + sheet_name="Data", + identifier_column="Name", + skip_patterns=[r"^C\d+$"], + include_provenance=False, + checkpoint_every=0, + progress=_silent, + ) + + workbook = openpyxl.load_workbook(destination, data_only=False) + try: + assert workbook.sheetnames == ["Cover", "Data", "Notes"] + assert workbook["Cover"]["A2"].value == "Do not modify" + assert workbook["Cover"].max_column == 1 + assert "PubChem CAS Resolution" in _header_positions(workbook["Data"]) + finally: + workbook.close() + + +def test_invalid_sheet_and_same_path_fail_before_provider_calls(tmp_path: Path): + source = tmp_path / "input.xlsx" + _create_preservation_workbook(source) + original = source.read_bytes() + client = NoLookupPubChem() + + with pytest.raises(ValueError, match="Available: Data, Notes"): + run_pubchem( + source, + client, + output_path=tmp_path / "missing.xlsx", + sheet_name="Missing", + progress=_silent, + ) + with pytest.raises(ValueError, match="Input and output paths must differ"): + run_pubchem( + source, + client, + output_path=source, + sheet_name="Data", + force=True, + progress=_silent, + ) + + assert client.calls == 0 + assert source.read_bytes() == original + + +def test_interrupted_checkpoint_is_a_complete_workbook(tmp_path: Path): + source = tmp_path / "input.xlsx" + destination = tmp_path / "output.xlsx" + partial = tmp_path / "output.partial.xlsx" + _create_preservation_workbook(source) + + with pytest.raises(RuntimeError, match="simulated interruption"): + run_pubchem( + source, + InterruptedPubChem(), + output_path=destination, + sheet_name="Data", + identifier_column="Name", + include_odor=False, + include_provenance=False, + checkpoint_every=1, + progress=_silent, + ) + + assert not destination.exists() + assert partial.exists() + workbook = openpyxl.load_workbook(partial, data_only=False) + try: + assert workbook.sheetnames == ["Data", "Notes"] + data = workbook["Data"] + assert data["C2"].value == "=B2*2" + assert data.freeze_panes == "A2" + assert data.tables["DataTable"].ref == "A1:C3" + title_column = _header_positions(data)["PubChem Title"] + assert data.cell(2, title_column).value == "First result" + assert data.cell(3, title_column).value is None + finally: + workbook.close() + + +def test_unsupported_comments_fail_before_provider_calls(tmp_path: Path): + source = tmp_path / "input.xlsx" + destination = tmp_path / "output.xlsx" + _create_preservation_workbook(source) + workbook = openpyxl.load_workbook(source) + workbook["Data"]["A2"].comment = Comment("Keep this formatting", "AromaNexus") + workbook.save(source) + workbook.close() + client = NoLookupPubChem() + + with pytest.raises(ValueError, match="cell comments"): + run_pubchem( + source, + client, + output_path=destination, + sheet_name="Data", + progress=_silent, + ) + + assert client.calls == 0 + assert not destination.exists() + + +def test_csv_formula_safety_is_unchanged_and_sheet_is_rejected(tmp_path: Path): + source = tmp_path / "input.csv" + destination = tmp_path / "output.csv" + source.write_text("Name,Original\nC6,=1+1\n", encoding="utf-8") + client = NoLookupPubChem() + + run_pubchem( + source, + client, + output_path=destination, + identifier_column="Name", + skip_patterns=[r"^C\d+$"], + include_provenance=False, + checkpoint_every=0, + progress=_silent, + ) + + assert destination.read_bytes().startswith(b"\xef\xbb\xbf") + output = pd.read_csv(destination, keep_default_na=False) + assert output.loc[0, "Original"] == "'=1+1" + assert client.calls == 0 + + invalid_destination = tmp_path / "invalid.csv" + with pytest.raises(ValueError, match="only valid for XLSX"): + run_pubchem( + source, + client, + output_path=invalid_destination, + sheet_name="Data", + progress=_silent, + ) + assert not invalid_destination.exists() + + +def test_formula_caches_and_non_target_package_parts_are_preserved(tmp_path: Path): + source = tmp_path / "input.xlsx" + destination = tmp_path / "output.xlsx" + _create_preservation_workbook(source) + workbook = openpyxl.load_workbook(source) + workbook["Notes"]["B3"] = '="cached note"' + workbook.save(source) + workbook.close() + + data_xml = _inject_formula_cache( + _xlsx_part(source, "xl/worksheets/sheet1.xml"), + "C2", + "20", + ) + assert b'r="C2"' in data_xml + data_xml = data_xml.replace(b'r="C2"', b"r='C2'", 1) + notes_xml = _inject_formula_cache( + _xlsx_part(source, "xl/worksheets/sheet2.xml"), + "B3", + "cached note", + cell_type="str", + ) + _rewrite_xlsx( + source, + { + "xl/worksheets/sheet1.xml": data_xml, + "xl/worksheets/sheet2.xml": notes_xml, + }, + ) + original_workbook_xml = _xlsx_part(source, "xl/workbook.xml") + original_styles_xml = _xlsx_part(source, "xl/styles.xml") + original_notes_xml = _xlsx_part(source, "xl/worksheets/sheet2.xml") + + run_pubchem( + source, + NoLookupPubChem(), + output_path=destination, + sheet_name="Data", + identifier_column="Name", + skip_patterns=[r"^C\d+$"], + include_odor=False, + include_provenance=False, + checkpoint_every=0, + progress=_silent, + ) + + assert _xlsx_part(destination, "xl/workbook.xml") == original_workbook_xml + assert _xlsx_part(destination, "xl/styles.xml") == original_styles_xml + assert _xlsx_part(destination, "xl/worksheets/sheet2.xml") == original_notes_xml + formulas = openpyxl.load_workbook(destination, data_only=False) + cached = openpyxl.load_workbook(destination, data_only=True) + try: + assert formulas["Data"]["C2"].value == "=B2*2" + assert cached["Data"]["C2"].value == 20 + assert formulas["Notes"]["B3"].value == '="cached note"' + assert cached["Notes"]["B3"].value == "cached note" + finally: + formulas.close() + cached.close() + + +def test_overwritten_formula_does_not_regain_stale_cache(tmp_path: Path): + source = tmp_path / "input.xlsx" + destination = tmp_path / "output.xlsx" + _create_preservation_workbook(source) + data_xml = _inject_formula_cache( + _xlsx_part(source, "xl/worksheets/sheet1.xml"), + "C2", + "20", + ) + _rewrite_xlsx(source, {"xl/worksheets/sheet1.xml": data_xml}) + + run_pubchem( + source, + FormulaPubChem(), + output_path=destination, + sheet_name="Data", + identifier_column="Name", + resolved_cas_column="Calculated value", + include_odor=False, + include_provenance=False, + checkpoint_every=0, + progress=_silent, + ) + + formulas = openpyxl.load_workbook(destination, data_only=False) + cached = openpyxl.load_workbook(destination, data_only=True) + try: + assert formulas["Data"]["C2"].value == "110-54-3" + assert formulas["Data"]["C2"].data_type == "s" + assert cached["Data"]["C2"].value == "110-54-3" + finally: + formulas.close() + cached.close() + + +def test_unrelated_partial_files_are_never_deleted(tmp_path: Path): + source = tmp_path / "result.partial.xlsx" + destination = tmp_path / "result.xlsx" + unrelated_partial = tmp_path / "short.partial.xlsx" + short_destination = tmp_path / "short.xlsx" + _create_preservation_workbook(source) + original_source = source.read_bytes() + + run_pubchem( + source, + NoLookupPubChem(), + output_path=destination, + sheet_name="Data", + identifier_column="Name", + skip_patterns=[r"^C\d+$"], + include_provenance=False, + checkpoint_every=0, + progress=_silent, + ) + assert source.read_bytes() == original_source + + unrelated_partial.write_bytes(b"user-owned checkpoint sentinel") + run_pubchem( + source, + NoLookupPubChem(), + output_path=short_destination, + sheet_name="Data", + identifier_column="Name", + skip_patterns=[r"^C\d+$"], + include_provenance=False, + checkpoint_every=25, + progress=_silent, + ) + assert unrelated_partial.read_bytes() == b"user-owned checkpoint sentinel" + + +def test_existing_enabled_checkpoint_fails_before_provider(tmp_path: Path): + source = tmp_path / "input.xlsx" + destination = tmp_path / "output.xlsx" + partial = tmp_path / "output.partial.xlsx" + _create_preservation_workbook(source) + partial.write_bytes(b"user-owned checkpoint sentinel") + client = NoLookupPubChem() + + with pytest.raises(FileExistsError, match="output.partial.xlsx"): + run_pubchem( + source, + client, + output_path=destination, + sheet_name="Data", + identifier_column="Name", + include_provenance=False, + checkpoint_every=1, + progress=_silent, + ) + + assert client.calls == 0 + assert partial.read_bytes() == b"user-owned checkpoint sentinel" + assert not destination.exists() + + +def test_merged_table_body_and_column_limit_fail_before_provider(tmp_path: Path): + merged_source = tmp_path / "merged.xlsx" + _create_preservation_workbook(merged_source) + workbook = openpyxl.load_workbook(merged_source) + data = workbook["Data"] + data["D1"] = "Resolved CAS" + data.merge_cells("C2:D2") + workbook.save(merged_source) + workbook.close() + merged_client = NoLookupPubChem() + + with pytest.raises(ValueError, match="merged range C2:D2"): + run_pubchem( + merged_source, + merged_client, + output_path=tmp_path / "merged-output.xlsx", + sheet_name="Data", + identifier_column="Name", + include_provenance=False, + checkpoint_every=0, + progress=_silent, + ) + assert merged_client.calls == 0 + + wide_source = tmp_path / "wide.xlsx" + workbook = openpyxl.Workbook() + worksheet = workbook.active + worksheet.append(["Name"]) + worksheet.append(["C6"]) + worksheet["XFC1"] = "Reserved edge cell" + workbook.save(wide_source) + workbook.close() + wide_client = NoLookupPubChem() + + with pytest.raises(ValueError, match="column limit"): + run_pubchem( + wide_source, + wide_client, + output_path=tmp_path / "wide-output.xlsx", + identifier_column="Name", + include_odor=False, + include_provenance=False, + checkpoint_every=0, + progress=_silent, + ) + assert wide_client.calls == 0 + + +def test_custom_xml_part_is_rejected_before_provider(tmp_path: Path): + source = tmp_path / "custom-xml.xlsx" + _create_preservation_workbook(source) + content_types = _xlsx_part(source, "[Content_Types].xml").replace( + b"", + (b''), + ) + root_relationships = _xlsx_part(source, "_rels/.rels").replace( + b"", + ( + b'' + b"" + ), + ) + _rewrite_xlsx( + source, + { + "[Content_Types].xml": content_types, + "_rels/.rels": root_relationships, + }, + {"customXml/item1.xml": b'preserve me'}, + ) + client = NoLookupPubChem() + + with pytest.raises(ValueError, match="customXml/item1.xml"): + run_pubchem( + source, + client, + output_path=tmp_path / "custom-output.xlsx", + sheet_name="Data", + identifier_column="Name", + checkpoint_every=0, + progress=_silent, + ) + assert client.calls == 0 + + +def test_same_file_alias_is_rejected_before_provider(tmp_path: Path): + source = tmp_path / "input.xlsx" + alias = tmp_path / "hardlink.xlsx" + _create_preservation_workbook(source) + try: + os.link(source, alias) + except OSError as exc: + pytest.skip(f"Hard links are unavailable: {exc}") + client = NoLookupPubChem() + + with pytest.raises(ValueError, match="Input and output paths must differ"): + run_pubchem( + source, + client, + output_path=alias, + sheet_name="Data", + identifier_column="Name", + force=True, + checkpoint_every=0, + progress=_silent, + ) + assert client.calls == 0 + + +def test_atomic_no_replace_wins_destination_race(tmp_path: Path, monkeypatch): + source = tmp_path / "input.xlsx" + destination = tmp_path / "output.xlsx" + _create_preservation_workbook(source) + frame, context = excel_io.read_table_with_context(source, sheet_name="Data") + original_writer = excel_io._write_preserved_xlsx + + def racing_writer(frame, temporary, table_context): + original_writer(frame, temporary, table_context) + destination.write_bytes(b"competing writer") + + monkeypatch.setattr(excel_io, "_write_preserved_xlsx", racing_writer) + with pytest.raises(FileExistsError, match="Pass --force"): + excel_io.write_table(frame, destination, context=context) + + assert destination.read_bytes() == b"competing writer" + assert not list(tmp_path.glob(".output-*.xlsx")) + + +def test_keyboard_interrupt_cleans_temporary_file(tmp_path: Path, monkeypatch): + source = tmp_path / "input.xlsx" + destination = tmp_path / "output.xlsx" + _create_preservation_workbook(source) + frame, context = excel_io.read_table_with_context(source, sheet_name="Data") + + def interrupting_writer(*_args, **_kwargs): + raise KeyboardInterrupt + + monkeypatch.setattr(excel_io, "_write_preserved_xlsx", interrupting_writer) + with pytest.raises(KeyboardInterrupt): + excel_io.write_table(frame, destination, context=context) + + assert not destination.exists() + assert not list(tmp_path.glob(".output-*.xlsx")) + + +def test_public_read_table_does_not_retain_private_workbook_snapshot(tmp_path: Path): + source = tmp_path / "input.xlsx" + _create_preservation_workbook(source) + + frame = excel_io.read_table(source, sheet_name="Data") + + assert frame.attrs == {}