From ca4b032d7314e3fdbc43983cde0ed8daa80b6d9a Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Thu, 1 Oct 2026 15:42:48 +0000 Subject: [PATCH 1/3] feat(update): skip raw-file stat when the metadata source's FileRef size/mtime match the load Raw FileRefs that carry size and/or mtime are kept on ExternalLink.file_refs (persisted in v9 meta.json, omitted when empty so older documents are unchanged). CellpyCell._raw_sources_changed consults them before building a FileID: when every recorded value equals what was loaded (size exact, mtime within 1 s, ISO-8601 or epoch), the file counts as unchanged and the remote stat is skipped. A differing or missing value, an unparsable mtime, or force=True falls back to today's path. Re-running fetch_meta on a loaded cell refreshes the hints; batch links built by pages_from_records carry them so Batch.refresh()/poll() benefit without facade changes. Refs #1124 Co-authored-by: Jan Petter Maehlen --- src/cellpy/batch/facade.py | 4 + src/cellpy/readers/cellreader.py | 109 +++++++- .../readers/metadata_sources/contract.py | 33 ++- tests/test_source_file_hints.py | 245 ++++++++++++++++++ 4 files changed, 388 insertions(+), 3 deletions(-) create mode 100644 tests/test_source_file_hints.py diff --git a/src/cellpy/batch/facade.py b/src/cellpy/batch/facade.py index f496fe9a..5d44d884 100644 --- a/src/cellpy/batch/facade.py +++ b/src/cellpy/batch/facade.py @@ -390,6 +390,10 @@ def refresh(self, labels: Sequence[str] | None = None, raise_errors: bool = Fals The combined-summary cache is cleared when any cell changed, so ``summaries`` / ``plot()`` / collectors see the new data. + Cells of a `from_source` batch whose record carried raw-file + ``size`` / ``mtime`` skip the remote ``stat`` while those values match + what was loaded (#1124); ``force=True`` reloads regardless. + Args: labels: subset of cell labels (default: every loaded cell). raise_errors: re-raise a cell's ``update`` error instead of diff --git a/src/cellpy/readers/cellreader.py b/src/cellpy/readers/cellreader.py index a15304a4..f1786249 100644 --- a/src/cellpy/readers/cellreader.py +++ b/src/cellpy/readers/cellreader.py @@ -147,6 +147,68 @@ def _align_dtypes(chunk, existing): return chunk.with_columns(casts) if casts else chunk +#: how far apart (seconds) a source-recorded mtime and the loaded ``st_mtime`` +#: may be and still count as the same file (ISO strings often drop sub-seconds) +SOURCE_MTIME_TOLERANCE = 1.0 + + +def _mtime_epoch(value): + """Epoch seconds for a `FileRef.mtime` (number or ISO-8601 text), else None. + + A naive timestamp is taken as UTC. + """ + if value is None: + return None + if isinstance(value, numbers.Real): + return float(value) + try: + moment = datetime.datetime.fromisoformat(str(value).strip()) + except ValueError: + return None + if moment.tzinfo is None: + moment = moment.replace(tzinfo=datetime.timezone.utc) + return moment.timestamp() + + +def _uri_basename(uri) -> str: + return str(uri).replace("\\", "/").rstrip("/").rsplit("/", 1)[-1] + + +def _uri_is_path(uri, full_name) -> bool: + """Does a `FileRef.uri` name the same file as a `FileID.full_name`?""" + uri = str(uri) + if not full_name: + return False + if uri == full_name: + return True + try: + return internals.OtherPath(uri).full_path == full_name + except Exception: # noqa: BLE001 - a URI OtherPath rejects is just not a match + return False + + +def _source_hint_matches_loaded(ref, fid) -> bool: + """True when every stat the source recorded equals what cellpy loaded. + + ``ref`` is a `FileRef` kept on an `ExternalLink` (#1124); ``fid`` the + `FileID` from the load. A ref without ``size`` and ``mtime``, an + unparsable mtime, or a fid without stats never matches, so the caller + falls back to stat-ing the file. + """ + if ref.size is None and ref.mtime is None: + return False + if ref.size is not None: + if fid.size is None or int(ref.size) != int(fid.size): + return False + if ref.mtime is not None: + recorded = _mtime_epoch(ref.mtime) + if recorded is None or fid.last_modified is None: + return False + if abs(recorded - float(fid.last_modified)) > SOURCE_MTIME_TOLERANCE: + return False + return True + + def normalize_summary_meta_fields(fields=None): """Normalize meta field names for ``SUMMARY_META_DEPENDENCIES`` / ``refresh_after``. @@ -1928,7 +1990,12 @@ def update(self, force=False, **loader_kwargs): Flow: 1. Change detection on the recorded raw files (size and mtime, as in - ``check_file_ids``). Unchanged and not ``force`` → no-op. + ``check_file_ids``). Unchanged and not ``force`` → no-op. A cell + that came from a metadata-source record whose `FileRef` carries + ``size`` / ``mtime`` (`external_links[...].file_refs`) is compared + against those first and the remote ``stat`` is skipped when they + match what was loaded (#1124); a differing or missing value falls + back to the stat. 2. Single source whose loader implements ``SupportsIncrementalLoad`` (arbin_res, arbin_sql, neware_txt, maccor_txt): read only the rows since the load marker and append them through core @@ -1992,11 +2059,17 @@ def _raw_sources_changed(self, fids) -> bool: """True if any recorded raw file differs from disk in size or mtime. Sources without file stats (databases, missing files) count as changed - so the caller still tries to refresh them. + so the caller still tries to refresh them. A file whose metadata-source + record (`external_links[*].file_refs`, #1124) carries the same size / + mtime as the load is taken as unchanged without touching the disk. """ for fid in fids: if getattr(fid, "is_db", False) or not fid.full_name: return True + hint = self._source_file_hint(fid) + if hint is not None and _source_hint_matches_loaded(hint[1], fid): + logging.debug(f"update: {fid.name} unchanged per {hint[0]!r} record; stat skipped") + continue current = ds.FileID(fid.full_name) if current.name is None: return True @@ -2008,6 +2081,31 @@ def _raw_sources_changed(self, fids) -> bool: return True return False + def _source_file_hint(self, fid): + """``(source_name, FileRef)`` recorded for this raw file, or None. + + Matches on the URI (as given, then as ``OtherPath.full_path``); when + no URI matches, a single ref sharing the file name is accepted. + """ + links = getattr(self.data, "external_links", None) or {} + by_name = [] + for source_name, link in links.items(): + for ref in getattr(link, "file_refs", ()) or (): + if _uri_is_path(ref.uri, fid.full_name): + return source_name, ref + if fid.name and _uri_basename(ref.uri) == fid.name: + by_name.append((source_name, ref)) + if len(by_name) == 1: + return by_name[0] + return None + + @staticmethod + def _ref_points_at(ref, full_names) -> bool: + if any(_uri_is_path(ref.uri, name) for name in full_names): + return True + base = _uri_basename(ref.uri) + return any(_uri_basename(name) == base for name in full_names) + def _ensure_loader_for_update(self, **loader_kwargs): """Recreate the loader from stored provenance when the tester changed. @@ -2247,6 +2345,13 @@ def _apply_meta_record(self, record, files=()) -> tuple: link = record.link(files=files) from dataclasses import replace + if not files: + # A re-fetch on a loaded cell: keep only the refs that point at + # the raw files this cell was read from, so they can serve as + # ``update()`` hints (#1124) without claiming files we never opened. + loaded = {str(f.full_name) for f in (self.data.raw_data_files or []) if f is not None and f.full_name} + if loaded: + link = replace(link, file_refs=tuple(ref for ref in link.file_refs if self._ref_points_at(ref, loaded))) link = replace(link, fields=applied) if not hasattr(self.data, "external_links") or self.data.external_links is None: self.data.external_links = {} diff --git a/src/cellpy/readers/metadata_sources/contract.py b/src/cellpy/readers/metadata_sources/contract.py index afabec09..d4a1186e 100644 --- a/src/cellpy/readers/metadata_sources/contract.py +++ b/src/cellpy/readers/metadata_sources/contract.py @@ -215,13 +215,28 @@ def cellpy_file(self) -> FileRef | None: return None def link(self, *, files: Iterable[str] = ()) -> "ExternalLink": + """The back-link for this record. + + ``files`` are the URIs cellpy opened. The raw refs among them that + carry ``size`` / ``mtime`` ride along as ``file_refs`` so a later + ``update()`` can skip stat-ing the file (#1124); with no ``files`` + every stat-carrying raw ref is kept. + """ + files = tuple(files) + wanted = set(files) + refs = tuple( + ref + for ref in self.raw_files() + if (ref.size is not None or ref.mtime is not None) and (not wanted or ref.uri in wanted) + ) return ExternalLink( source_name=self.source_name, external_id=self.external_id, source_uri=self.source_uri, fetched_at=self.fetched_at, fields=self.fields, - files=tuple(files), + files=files, + file_refs=refs, ) @@ -242,6 +257,12 @@ class ExternalLink: fields: tuple[str, ...] = () #: file URIs the source pointed at and cellpy opened (#1107) files: tuple[str, ...] = () + #: raw `FileRef`s with the ``size`` / ``mtime`` the source recorded (#1124); + #: ``update()`` skips the remote stat when they match what was loaded + file_refs: tuple[FileRef, ...] = () + + def __post_init__(self) -> None: + object.__setattr__(self, "file_refs", _coerce_files(self.file_refs)) def to_dict(self) -> dict[str, Any]: payload = { @@ -253,6 +274,8 @@ def to_dict(self) -> dict[str, Any]: } if self.files: payload["files"] = list(self.files) + if self.file_refs: + payload["file_refs"] = [ref.to_dict() for ref in self.file_refs] return payload @classmethod @@ -264,8 +287,16 @@ def from_dict(cls, payload: Mapping[str, Any]) -> "ExternalLink": fetched_at=payload.get("fetched_at"), fields=tuple(payload.get("fields") or ()), files=tuple(payload.get("files") or ()), + file_refs=tuple(payload.get("file_refs") or ()), ) + def file_ref_for(self, uri: str) -> FileRef | None: + """The recorded ref whose URI is ``uri``, if any (exact string match).""" + for ref in self.file_refs: + if ref.uri == uri: + return ref + return None + @runtime_checkable class MetadataSource(Protocol): diff --git a/tests/test_source_file_hints.py b/tests/test_source_file_hints.py new file mode 100644 index 00000000..1c28bfda --- /dev/null +++ b/tests/test_source_file_hints.py @@ -0,0 +1,245 @@ +"""Source-recorded file stats as ``update()`` hints (#1124, Epic M). + +A metadata-source record whose raw `FileRef` carries ``size`` / ``mtime`` +lets `CellpyCell.update()` skip the (possibly remote) ``stat`` when those +values equal what cellpy loaded. Anything without such values, or with a +differing value, goes through today's stat-based path. +""" + +from __future__ import annotations + +import datetime +import logging +import pathlib + +import pytest + +import cellpy +from cellpy import batch as batch_api +from cellpy import log +from cellpy.batch.policy import LoadPolicy, SourcePreference +from cellpy.batch.source import SESSION_KEY +from cellpy.internals.otherpath import OtherPath +from cellpy.readers import metadata_sources as ms +from cellpy.readers.cellreader import _mtime_epoch, _source_hint_matches_loaded +from cellpy.readers.metadata_sources import ExternalLink, FileRef, MetaRecord +from cellpy.readers.metadata_sources import registry as registry_module +from cellpy.readers.metadata_sources.testing import DictMetadataSource +from tests import fdv + +log.setup_logging(default_level=logging.DEBUG, testing=True) + +RES = pathlib.Path(fdv.res_file_path) +RES_URI = RES.as_posix() +RES_STAT = RES.stat() +RES_MTIME_ISO = datetime.datetime.fromtimestamp(RES_STAT.st_mtime, tz=datetime.timezone.utc).isoformat() + + +def _record(**ref_kwargs) -> MetaRecord: + files = (FileRef("raw", RES_URI, loader="arbin_res", **ref_kwargs),) if ref_kwargs else () + return MetaRecord( + "labdb", + external_id="42", + cell={"mass": 1.23}, + test={"cell_name": fdv.run_name}, + files=files or (FileRef("raw", RES_URI, loader="arbin_res"),), + ) + + +@pytest.fixture +def clean_registry(monkeypatch): + monkeypatch.setattr(registry_module, "_iter_entry_points", lambda: ()) + ms.clear_registry() + yield + ms.clear_registry() + + +@pytest.fixture +def labdb(clean_registry) -> DictMetadataSource: + source = DictMetadataSource( + { + ("tag", "MATCH"): _record(size=RES_STAT.st_size, mtime=RES_MTIME_ISO), + ("tag", "SIZE_OFF"): _record(size=RES_STAT.st_size + 1, mtime=RES_MTIME_ISO), + ("tag", "MTIME_ONLY"): _record(mtime=RES_STAT.st_mtime), + ("tag", "BAD_MTIME"): _record(size=RES_STAT.st_size, mtime="yesterday-ish"), + ("tag", "NO_STATS"): _record(), + }, + name="labdb", + ) + ms.register(source) + return source + + +@pytest.fixture +def no_stat(monkeypatch): + """Count ``OtherPath.stat`` calls; the hint path must not reach it.""" + calls: list[str] = [] + original = OtherPath.stat + + def _stat(self, *args, **kwargs): + calls.append(str(self)) + return original(self, *args, **kwargs) + + monkeypatch.setattr(OtherPath, "stat", _stat) + return calls + + +def _load(tag): + return cellpy.get(source="labdb", key=tag, kind="tag", testing=True) + + +# -- comparison rule ----------------------------------------------------------- + + +@pytest.mark.essential +@pytest.mark.parametrize( + "value, expected", + [ + (None, None), + (1_700_000_000, 1_700_000_000.0), + ("2023-11-14T22:13:20+00:00", 1_700_000_000.0), + ("2023-11-14T22:13:20Z", 1_700_000_000.0), + ("2023-11-14T22:13:20", 1_700_000_000.0), # naive → UTC + ("not a date", None), + ], +) +def test_mtime_epoch(value, expected): + assert _mtime_epoch(value) == expected + + +class _Fid: + def __init__(self, size, last_modified): + self.size = size + self.last_modified = last_modified + + +@pytest.mark.essential +def test_hint_matches_only_when_every_recorded_stat_agrees(): + fid = _Fid(100, 1_700_000_000.0) + assert _source_hint_matches_loaded(FileRef("raw", "x", size=100, mtime=1_700_000_000.4), fid) + assert _source_hint_matches_loaded(FileRef("raw", "x", size=100), fid) + assert _source_hint_matches_loaded(FileRef("raw", "x", mtime="2023-11-14T22:13:20Z"), fid) + assert not _source_hint_matches_loaded(FileRef("raw", "x"), fid) + assert not _source_hint_matches_loaded(FileRef("raw", "x", size=101), fid) + assert not _source_hint_matches_loaded(FileRef("raw", "x", size=100, mtime=1_700_000_002.0), fid) + assert not _source_hint_matches_loaded(FileRef("raw", "x", size=100, mtime="???"), fid) + assert not _source_hint_matches_loaded(FileRef("raw", "x", size=100), _Fid(None, None)) + + +# -- back-link ----------------------------------------------------------------- + + +@pytest.mark.essential +def test_link_keeps_stat_carrying_raw_refs_and_round_trips(): + record = MetaRecord( + "labdb", + files=( + FileRef("raw", "a.res", size=1, mtime="2026-01-01T00:00:00Z"), + FileRef("raw", "b.res"), + FileRef("cellpy", "a.cellpy", size=5), + ), + ) + link = record.link(files=("a.res", "b.res")) + assert [r.uri for r in link.file_refs] == ["a.res"] + assert link.file_ref_for("a.res").size == 1 + assert link.file_ref_for("b.res") is None + again = ExternalLink.from_dict(link.to_dict()) + assert again == link + assert again.file_refs[0].mtime == "2026-01-01T00:00:00Z" + # pre-#1124 documents stay byte-identical + assert "file_refs" not in ExternalLink("labdb", files=("b.res",)).to_dict() + assert record.link(files=("b.res",)).file_refs == () + + +# -- cell path ----------------------------------------------------------------- + + +@pytest.mark.essential +def test_matching_hint_skips_stat(labdb, no_stat): + c = _load("MATCH") + assert c.external_links["labdb"].file_refs[0].size == RES_STAT.st_size + no_stat.clear() + + assert c.update() is False + assert no_stat == [] + + +@pytest.mark.essential +def test_differing_size_falls_back_to_stat(labdb, no_stat): + c = _load("SIZE_OFF") + no_stat.clear() + + assert c.update() is False # the file itself did not change + assert no_stat, "stat must run when the source's size differs" + + +def test_mtime_only_hint_is_enough(labdb, no_stat): + c = _load("MTIME_ONLY") + no_stat.clear() + assert c.update() is False + assert no_stat == [] + + +def test_unparsable_mtime_falls_back_to_stat(labdb, no_stat): + c = _load("BAD_MTIME") + no_stat.clear() + assert c.update() is False + assert no_stat + + +@pytest.mark.essential +def test_record_without_stats_behaves_as_today(labdb, no_stat): + c = _load("NO_STATS") + assert c.external_links["labdb"].file_refs == () + no_stat.clear() + assert c.update() is False + assert no_stat + + +def test_force_ignores_the_hint(labdb, no_stat): + c = _load("MATCH") + n_raw = len(c.data.raw) + assert c.update(force=True) is True + assert len(c.data.raw) == n_raw + + +def test_hints_survive_save_and_load(labdb, no_stat, tmp_path): + c = _load("MATCH") + path = tmp_path / "hinted.cellpy" + c.save(path) + + loaded = cellpy.get(path, testing=True) + link = loaded.external_links["labdb"] + assert link.file_refs[0].uri == RES_URI + assert link.file_refs[0].size == RES_STAT.st_size + no_stat.clear() + assert loaded.update() is False + assert no_stat == [] + + +def test_refetch_on_a_loaded_cell_replaces_the_hints(labdb, no_stat): + c = _load("SIZE_OFF") + assert c.external_links["labdb"].file_refs[0].size == RES_STAT.st_size + 1 + + c.fetch_meta("labdb", "MATCH", kind="tag") + + assert c.external_links["labdb"].file_refs[0].size == RES_STAT.st_size + no_stat.clear() + assert c.update() is False + assert no_stat == [] + + +# -- batch path ---------------------------------------------------------------- + + +def test_batch_links_carry_hints_and_refresh_skips_stat(labdb, no_stat): + b = batch_api.from_source("labdb", "MATCH", policy=LoadPolicy(source=SourcePreference.NEWEST)) + assert b.journal.session[SESSION_KEY][fdv.run_name]["file_refs"][0]["size"] == RES_STAT.st_size + + b.update(progress=False, testing=True) + cell = b.cells[fdv.run_name] + assert cell.external_links["labdb"].file_refs[0].size == RES_STAT.st_size + no_stat.clear() + + assert b.refresh() == {fdv.run_name: False} + assert no_stat == [] From f66aa467b4f204bc7d2f2b4f203f1cc16a519fea Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Thu, 1 Oct 2026 15:42:48 +0000 Subject: [PATCH 2/3] docs: document source-recorded file stats as update() hints (#1124) Changelog bullet, agent usage notes (docs/agents/index.md, AGENTS.md), the metadata-sources design doc section recording where the hints live and the comparison rule, and test-registry rows for the new essential tests. Co-authored-by: Jan Petter Maehlen --- .../04-designs-and-guides/metadata-sources.md | 16 +++++++++++++++- .../04-designs-and-guides/test-registry.md | 7 +++++++ AGENTS.md | 3 +++ HISTORY.md | 9 +++++++++ docs/agents/index.md | 6 ++++++ 5 files changed, 40 insertions(+), 1 deletion(-) diff --git a/.issueflows/04-designs-and-guides/metadata-sources.md b/.issueflows/04-designs-and-guides/metadata-sources.md index 6ece5f68..6f3f1461 100644 --- a/.issueflows/04-designs-and-guides/metadata-sources.md +++ b/.issueflows/04-designs-and-guides/metadata-sources.md @@ -38,7 +38,21 @@ capacity, project without cellpy depending on one lab's API. | Back-link | `ExternalLink.files: tuple[str, ...]` = URIs cellpy opened because the record pointed at them; `to_dict` omits the key when empty so pre-#1107 `meta.json` documents are byte-identical. | | Batch path | `Batch.from_source(source, key, *, kind="tag", project, name, policy, file_search=True, file_search_kwargs, strict=True, **extra)`; module `batch.from_source` + `utils.batch` shim. `batch/source.py::pages_from_records` builds one row per record (`filename`/`label` = `test.cell_name` → `external_id` → `cell_NNN`, de-duplicated with `_2` suffixes; mass/area/loading/nom_cap/nom_cap_specifics/cycle_mode; `instrument` = first raw `loader`; `raw_file_names` / `cellpy_file_name`; `raw_file_size` / `raw_file_mtime` when known; `external_id`, `source_uri`). Rows without pointers ⇒ `_dbengine.find_files` (the `journal_from_db` call) on just those rows; `file_search=False` leaves `None` (#1017 rule). `project` defaults to the source name (no `project` field on `CellMeta`); `name` to `__` slug. | | Batch provenance | Links kept in `journal.session["external_links"]` (`{label: ExternalLink.to_dict()}`, survives `write_journal`); `Batch.update()` → `_stamp_external_links()` copies them onto loaded cells. Values are **not** re-applied (journal precedence intact); per-field `Resolution.origin_of` stays a cell-path feature. | -| Deferred | `size`/`mtime` short-circuit in `update()` / `refresh()` (data is carried, logic is a follow-up); cellpy-connectors adapter mapping BatBase `files[]` → `FileRef` (follow-up issue there). | +| Deferred | cellpy-connectors adapter mapping BatBase `files[]` → `FileRef` (follow-up issue there). | + +## Stat skip on `update()` (#1124) + +| Topic | Decision | +| --- | --- | +| Where the hints live | `ExternalLink.file_refs: tuple[FileRef, ...]` — the record's **raw** refs that carry `size` and/or `mtime`, restricted to the URIs cellpy opened (`MetaRecord.link(files=…)`). Chosen over `Data._provenance` because the link is already the per-source persisted object that `_stamp_external_links`, `apply_meta_document` and `from_cell` copy. `to_dict` writes `"file_refs"` only when non-empty, so pre-#1124 `meta.json` stays byte-identical. `ExternalLink.file_ref_for(uri)` looks one up. | +| Check | `CellpyCell._raw_sources_changed` asks `_source_file_hint(fid)` (URI equal to `fid.full_name`, or `OtherPath(uri).full_path` equal; else a *single* ref with the same basename) before building `ds.FileID(...)`. `_source_hint_matches_loaded(ref, fid)`: every value the ref carries must match — `size` as `int ==`, `mtime` as epoch (number, or ISO-8601 via `fromisoformat`; naive ⇒ UTC) within `SOURCE_MTIME_TOLERANCE` (1 s) of `fid.last_modified`. A ref with neither value, an unparsable mtime or a fid without stats never matches ⇒ stat as before. `force=True` bypasses the whole check (unchanged). `checksum` is never used. | +| Re-fetch | `c.fetch_meta(source, key, kind=…)` on a loaded cell rebuilds the link; with no opened-URI list the refs are filtered to those pointing at `raw_data_files` (`_ref_points_at`), so the source's fresher `size`/`mtime` become the new hints. Batch: `pages_from_records` → session links → `_stamp_external_links` → `refresh()` / `poll()` benefit without facade changes; a new `batch.from_source(...)` is the batch-side re-fetch. | +| Not done | `check_file_ids` (raw-vs-cellpy stat on the first `cellpy.get` / batch load) could consult the journal `raw_file_size` / `raw_file_mtime` the same way — separate follow-up. | + +Semantics: the source is the system of record for the file. While its +recorded stats equal what cellpy loaded, cellpy trusts it and does not touch +the share; a tester that keeps writing is noticed once the source re-scans +(and cellpy re-fetches) or when `force=True`. Alternatives rejected: a new `Layer` for "source files" (files are not metadata; they select *what to load*); re-running `_apply_meta_record` per batch cell (would put the source above journal overrides); always writing `files` into `ExternalLink.to_dict()` (breaks byte-for-byte stability of old documents for no gain). diff --git a/.issueflows/04-designs-and-guides/test-registry.md b/.issueflows/04-designs-and-guides/test-registry.md index d6f9b31f..6de8a63e 100644 --- a/.issueflows/04-designs-and-guides/test-registry.md +++ b/.issueflows/04-designs-and-guides/test-registry.md @@ -275,6 +275,13 @@ current issue**. `/iflow-doctor` may audit the whole suite against this table. | tests/test_batch_from_source.py::test_from_source_builds_pages_like_a_journal | yes | | Batch.from_source | #1107 | | | tests/test_batch_from_source.py::test_update_loads_pointed_files_and_stamps_links | yes | | Batch.update → _stamp_external_links | #1107 | loads testdata | | tests/test_batch_from_source.py (other 7) | no | | naming, duplicates, file_search=False, session round-trip, strict/no-record errors | #1107 | offline | +| tests/test_source_file_hints.py::test_mtime_epoch | yes | yes | cellreader._mtime_epoch | #1124 | offline, parametrized | +| tests/test_source_file_hints.py::test_hint_matches_only_when_every_recorded_stat_agrees | yes | yes | cellreader._source_hint_matches_loaded | #1124 | offline | +| tests/test_source_file_hints.py::test_link_keeps_stat_carrying_raw_refs_and_round_trips | yes | yes | MetaRecord.link / ExternalLink.file_refs dict round trip | #1124 | byte-identical old docs | +| tests/test_source_file_hints.py::test_matching_hint_skips_stat | yes | yes | CellpyCell._raw_sources_changed hint path | #1124 | OtherPath.stat spy, loads testdata .res | +| tests/test_source_file_hints.py::test_differing_size_falls_back_to_stat | yes | yes | hint mismatch → stat | #1124 | proves the spy sees stat | +| tests/test_source_file_hints.py::test_record_without_stats_behaves_as_today | yes | yes | no-hint parity | #1124 | today's behaviour kept | +| tests/test_source_file_hints.py (other 6) | no | | mtime-only, bad mtime, force, save/load, re-fetch, batch refresh | #1124 | uses testdata .res | **Columns** diff --git a/AGENTS.md b/AGENTS.md index 820eb730..0babf717 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -388,6 +388,9 @@ Quick facts: `cellpy.get(source="batbase", key="SAL_010", kind="tag")` opens the record's file pointers (`filefinder` only as fallback); `batch.from_source("batbase", "SAL_010")` does the same for a whole tag. + Raw pointers with `size`/`mtime` stay on `external_links[src].file_refs`; + `update()` / `refresh()` skip the remote `stat` while they match what was + loaded (`force=True` or a differing value → stat/reload as usual). - Frames: `c.data.raw` / `.steps` / `.summary`; columns via `c.schema.*`. After a raw load, each cycle's raw capacity starts at 0. A forgotten tester reset that 1.x plotted as doubled capacity is rebased on load for every diff --git a/HISTORY.md b/HISTORY.md index 68c60063..318b86c7 100644 --- a/HISTORY.md +++ b/HISTORY.md @@ -72,6 +72,15 @@ troubleshooting), wired into the guides index, How-do-I and `llms.txt`. (#1023) +* Epic M: use `FileRef` size/mtime from the metadata source to skip stat-ing + raw files on `update()`. Raw pointers that carry `size` / `mtime` are kept + on `ExternalLink.file_refs` (persisted in v9 `meta.json`, omitted when + empty) and `CellpyCell.update()` / `Batch.refresh()` / `poll()` treat the + file as unchanged without a remote `stat` while they equal what was + loaded; a differing or missing value, or `force=True`, falls back to + today's path. A re-run of `fetch_meta` / `batch.from_source` refreshes the + hints. (#1124) + ## [2.1.5.post6] - 2026-09-25 * `summary_collector(...).plot()` keeps a lone charge or discharge series diff --git a/docs/agents/index.md b/docs/agents/index.md index faf36624..ddd07f6e 100644 --- a/docs/agents/index.md +++ b/docs/agents/index.md @@ -204,6 +204,12 @@ Useful methods on `CellpyCell` (non-exhaustive): URIs used. Same as `CellpyCell.from_source(source, key, kind=)`. Batch: `batch.from_source("batbase", "SAL_010")` (kind `"tag"`) builds the journal pages from the records, then `b.update()`. + When a raw pointer carries `size` / `mtime`, it is kept on + `c.external_links[source].file_refs` and `c.update()` / `b.refresh()` / + `poll()` skip the remote `stat` while those values equal what was loaded + (a differing or missing value falls back to the stat; `force=True` + always reloads). Re-run `c.fetch_meta(...)` or `batch.from_source(...)` + to pick up fresher values from the source. - `save` / `to_csv` / Excel helpers — persist for the user's workflow Deeper shape docs: [Data structure](../fundamentals/data_structure.md). From 86fea20dc8d800124c6546553ecc163e372ba82e Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Thu, 1 Oct 2026 15:42:48 +0000 Subject: [PATCH 3/3] =?UTF-8?q?chore(issueflow):=20close=20#1124=20?= =?UTF-8?q?=E2=80=94=20capture,=20plan,=20status=20archived=20as=20solved?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Jan Petter Maehlen --- .../03-solved-issues/issue1124_original.md | 26 ++++ .../03-solved-issues/issue1124_plan.md | 123 ++++++++++++++++++ .../03-solved-issues/issue1124_status.md | 45 +++++++ 3 files changed, 194 insertions(+) create mode 100644 .issueflows/03-solved-issues/issue1124_original.md create mode 100644 .issueflows/03-solved-issues/issue1124_plan.md create mode 100644 .issueflows/03-solved-issues/issue1124_status.md diff --git a/.issueflows/03-solved-issues/issue1124_original.md b/.issueflows/03-solved-issues/issue1124_original.md new file mode 100644 index 00000000..e0309b07 --- /dev/null +++ b/.issueflows/03-solved-issues/issue1124_original.md @@ -0,0 +1,26 @@ +# Issue #1124: Epic M: use FileRef size/mtime from the metadata source to skip stat-ing raw files on update() + +Source: https://github.com/jepegit/cellpy/issues/1124 + +## Original issue text + +## Context + +#1107 (PR #1119) added `FileRef(size, mtime, checksum, …)` on `MetaRecord.files`, and `batch.from_source` already carries them into the journal pages as `raw_file_size` / `raw_file_mtime`. Nothing consumes them yet: `CellpyCell.update()` / `Batch.refresh()` still `stat` every raw file (slow on `scp://`/SFTP shares) to decide whether anything changed. + +## Spec + +1. When a cell was loaded via a source record whose raw `FileRef`s carry `size` and/or `mtime`, store them next to the back-link (`ExternalLink` or `Data` provenance — pick one, document it) so they survive save/load. +2. `update()` / `refresh()` / `poll()`: if the source's recorded `size`+`mtime` equal what cellpy already loaded, skip the remote `stat` and treat the file as unchanged. A changed value, or a missing one, falls back to today's `stat`-based path. `checksum` is informational only (no hashing of remote files). +3. Batch: `Batch.refresh()` may consult `raw_file_size` / `raw_file_mtime` the same way; `batch.from_source(...).update()` followed by a second `from_source` fetch can short-circuit per cell. +4. Never skip when the user forces a reload (`recalc=True`, `RAW_ONLY`, explicit `update(force=…)` if that exists). + +## Acceptance + +- A cell loaded from a `DictMetadataSource` record with `size`/`mtime` does not call the path `stat` on `update()` when the values match (monkeypatched `OtherPath.stat` asserts not called), and does when they differ. +- Records without `size`/`mtime` behave exactly as today. +- Round-trip: the recorded values survive `.cellpy` save/load. + +## Related + +#1107, #783 (Epic M), ife-bat/batbase#474 (`size`/`mtime` on `TestDataFile`), incremental-load protocol (#779 / #164). diff --git a/.issueflows/03-solved-issues/issue1124_plan.md b/.issueflows/03-solved-issues/issue1124_plan.md new file mode 100644 index 00000000..9e8fca22 --- /dev/null +++ b/.issueflows/03-solved-issues/issue1124_plan.md @@ -0,0 +1,123 @@ +# Issue #1124 — plan + +Branch: `cursor/1124-fileref-skip-stat-ae40` (cloud-agent branch policy; the +issue-flow `-` form is not available here). Repo: `jepegit/cellpy`. + +## Goal + +A cell (or batch cell) that came from a metadata-source record carrying +`FileRef.size` / `FileRef.mtime` skips the remote `stat` in +`CellpyCell.update()` when those values equal what cellpy already loaded. +Everything without such values behaves exactly as today. + +## Constraints + +- Follow [metadata-sources.md](../04-designs-and-guides/metadata-sources.md) + and [incremental-load-protocol.md](../04-designs-and-guides/incremental-load-protocol.md): + `ExternalLink.to_dict()` must stay byte-identical for documents that carry + no new data (omit the new key when empty); `raw` payloads never persisted. +- `checksum` is informational only — no hashing of remote files. +- `force=True` already bypasses change detection; keep that. `recalc` / + `RAW_ONLY` live on the `cellpy.get` / batch-load path, which this change + does not touch. +- No cellpycore change; no new dependency. +- KISS: one dataclass field, one comparison helper, one hook in + `_raw_sources_changed`. No new module. + +### Prior art + +- `CellpyCell._raw_sources_changed` / `_refresh_fid` (`readers/cellreader.py`) + — the stat-based check `update()` uses today; the hook goes here. +- `ds.FileID.populate` (`readers/data_structures.py`) — `size` (int) and + `last_modified` (epoch float from `st_mtime`) are what "cellpy already + loaded" means. Coexist. +- `MetaRecord.link(files=…)` / `ExternalLink.files` (`metadata_sources/contract.py`) + — the back-link that already records opened URIs; extend, do not replace. +- `batch/source.py::pages_from_records` — already derives + `raw_file_size` / `raw_file_mtime` journal columns from the refs and builds + the link dicts stamped by `Batch._stamp_external_links`. Reuse: links built + there pick up the new field automatically. +- `check_file_ids` (raw-vs-cellpy stat on first batch load) — same stats, but + a different path; out of scope here (see Open questions). +- Toolbox (`00-tools/`): nothing relevant. Graph: `graphify-out/` absent. + +## Approach + +1. **Persist the hints on the back-link** (`contract.py`). + `ExternalLink.file_refs: tuple[FileRef, ...] = ()` — the record's `raw` + refs that carry `size` and/or `mtime`. `to_dict` writes `"file_refs"` + (list of `FileRef.to_dict()`) only when non-empty; `from_dict` reads it + back. `MetaRecord.link(files=…)` fills it from `raw_files()` restricted to + the given URIs (when `files` is empty: every raw ref with a stat). This is + the "pick one, document it" answer: the link, not `Data._provenance`, + because the link already is the per-source persisted object and + `_stamp_external_links` / `apply_meta_document` / `from_cell` all copy it. +2. **Cell path** (`cellreader.py`): `_apply_meta_record(record, files=())` + unchanged in signature; when `files` is empty (a later `c.fetch_meta(...)` + on an already loaded cell) match the record's raw refs against the cell's + `raw_data_files` full names so a re-fetch refreshes the hints. +3. **Change detection** (`cellreader.py`): in `_raw_sources_changed`, before + building `ds.FileID(fid.full_name)` ask `_source_file_hint(fid)` — scan + `data.external_links[*].file_refs` for a ref whose + `OtherPath(uri).full_path` equals `fid.full_name` (fallback: same `name` + when unique). If a hint exists and `_hint_matches_loaded(hint, fid)` is + true, log at debug (`"update: unchanged per record; stat + skipped"`) and treat that file as unchanged. Otherwise fall through to the + stat path exactly as today. + `_hint_matches_loaded`: every value the ref carries must match — `size` + as `int ==`; `mtime` via `_mtime_epoch(value)` (int/float epoch, or ISO + 8601 through `datetime.fromisoformat`; naive → UTC) compared with + `fid.last_modified` within 1 s. A ref with neither value, an unparsable + mtime, or a `fid` without stats never matches. +4. **Batch**: `pages_from_records` already calls `record.link(files=used)`, so + the session link dicts gain `file_refs`; `_stamp_external_links` restores + them onto loaded cells; `Batch.refresh()` / `poll()` call `c.update()` and + so skip the stat per cell. No facade change needed beyond a docstring + line. Re-fetching to refresh the hints is `c.fetch_meta(...)` (step 2) or + a new `batch.from_source(...)`; no extra API. +5. **Docs**: new row in `metadata-sources.md` (decision + comparison rule); + one line in `docs/agents/index.md` and the `AGENTS.md` quick fact for + `c.update()`; `update()` docstring step 1. HISTORY entry at close. + +## Files to touch + +- `src/cellpy/readers/metadata_sources/contract.py` — `ExternalLink.file_refs`, + dict round trip, `MetaRecord.link` filling it. +- `src/cellpy/readers/cellreader.py` — `_source_file_hint`, + `_hint_matches_loaded` (+ `_mtime_epoch` helper), hook in + `_raw_sources_changed`, `_apply_meta_record` re-fetch matching, docstring. +- `src/cellpy/batch/facade.py` — `refresh` docstring note only. +- `tests/test_metadata_source_files.py` (or new `tests/test_source_file_hints.py` + if it grows past ~120 lines) — see test strategy. +- `.issueflows/04-designs-and-guides/metadata-sources.md`, + `docs/agents/index.md`, `AGENTS.md` — one row / one line each. +- `.issueflows/01-current-issues/issue1124_status.md`. + +## Test strategy + +`uv run pytest tests/test_metadata_source_files.py tests/test_cell_update.py tests/test_batch_from_source.py tests/test_batch_live.py` +then `uv run pytest -m essential`. Fixture: `DictMetadataSource` records with +`FileRef(size=, mtime=)`; monkeypatch +`cellpy.internals.otherpath.OtherPath.stat` to raise `AssertionError`. + +- matching `size`+`mtime` → `c.update()` is `False`, `stat` not called. +- `size` off by one → `stat` called (falls back; file unchanged → `False`). +- `mtime` only, matching → skipped; `mtime` unparsable → `stat` called. +- record without `size`/`mtime` → `stat` called (today's behaviour). +- `update(force=True)` still reloads (existing test in `test_cell_update.py` + stays green). +- round trip: `c.save()` → `cellpy.get(.cellpy)` keeps `file_refs`; old + `ExternalLink.to_dict()` without refs has no `"file_refs"` key. +- batch: `Batch.from_source` session link carries `file_refs`; after + `update()` the loaded cell's link has them and `b.refresh()` skips `stat`. +- re-fetch on a loaded cell (`c.fetch_meta`) replaces the hints. + +## Open questions + +- `check_file_ids` (raw-vs-cellpy stat on first batch load / `cellpy.get` + with a cellpy file) could use the same journal `raw_file_size` / + `raw_file_mtime`. Recommendation: separate follow-up issue; this one is + scoped to `update()` / `refresh()` / `poll()` as titled. +- mtime tolerance 1 s and naive-ISO-means-UTC: recommended defaults; a + mismatch only costs one `stat`, never a wrong skip in the other direction + except a source whose clock is off by < 1 s. diff --git a/.issueflows/03-solved-issues/issue1124_status.md b/.issueflows/03-solved-issues/issue1124_status.md new file mode 100644 index 00000000..b5b26975 --- /dev/null +++ b/.issueflows/03-solved-issues/issue1124_status.md @@ -0,0 +1,45 @@ +# Issue #1124 — status + +- [x] Done + +Branch: `cursor/1124-fileref-skip-stat-ae40` (cloud-agent branch policy; not +the `-` form). Plan: `issue1124_plan.md` (accepted 2026-10-01, +followed as written). + +## What's done + +- `ExternalLink.file_refs: tuple[FileRef, ...]` — raw refs with `size` / + `mtime`, restricted to opened URIs; `to_dict` emits `"file_refs"` only when + non-empty (pre-#1124 `meta.json` byte-identical); `file_ref_for(uri)`. + `MetaRecord.link(files=…)` fills it, so `cellpy.get(source=…)`, + `fetch_meta`, `pages_from_records` → `_stamp_external_links` all carry it. +- `CellpyCell._raw_sources_changed`: `_source_file_hint(fid)` (URI / + `OtherPath.full_path` / unique basename) + module-level + `_source_hint_matches_loaded` (`size` int-equal, `mtime` epoch or ISO-8601 + within `SOURCE_MTIME_TOLERANCE` = 1 s, naive ⇒ UTC; every carried value + must match) skips `ds.FileID(...)` (no `is_file` / `stat`) and logs at + debug. `force=True` unchanged. +- `_apply_meta_record` on a loaded cell (re-fetch) keeps only refs pointing + at `raw_data_files`, so fresher source stats replace the hints. +- Tests: `tests/test_source_file_hints.py` (17; 6 essential, registry rows + added). Related suites green: `test_metadata_source_files`, + `test_metadata_sources`, `test_cell_update`, `test_batch_from_source`, + `test_batch_live`, `test_live_poll` (88 passed). +- Docs: `metadata-sources.md` new section; `docs/agents/index.md`, + `AGENTS.md`, `update()` and `Batch.refresh()` docstrings; HISTORY bullet. + +## Verification + +- `uv run pytest -m essential --ignore=tests/test_arbin_variants_two_stage.py --ignore=tests/test_load_since.py`: + 953 passed, 74 skipped, 2 failed. Both failures + (`tests/test_filefinder.py::test_find_by_project_cellpy_range`, + `::test_find_by_project_otherpath_local`) fail identically on a clean + `master` checkout in this environment — not from this change. The two + ignored modules fail to import here (`pyodbc` needs `libodbc.so.2`), also + environmental. + +## Remaining work / follow-ups (not in scope) + +- `check_file_ids` (raw-vs-cellpy stat on first `cellpy.get` / batch load) + could consult journal `raw_file_size` / `raw_file_mtime` the same way — + separate issue.