Skip to content

source-mode preceding: preceding_instance_identity can name a csvpath that never ran #223

Description

@dk107dk

Summary

preceding_instance_identity (Results Instance Manifest field, written only
when source_mode_preceding is true) is computed by declared-position math,
but the actual source-mode: preceding data lookup, for the ordinary,
non-reference invocation path, uses a different, execution-order mechanism.
When any earlier csvpath in the group is skipped (will_run == False, for
example run-mode: no-run), the two mechanisms disagree, and the recorded
field can name a csvpath that never actually ran and never produced a
data.csv.

Where the field comes from

csvpath/managers/paths/paths_manager.py, get_preceeding_instance_identity:

def get_preceeding_instance_identity(self, name, index: int) -> str:
    if index <= 0:
        raise ValueError("0 is the first csvpath in named-paths group")
    paths = self.get_named_paths(name)
    paths = self.get_identified_paths_in(name, paths)
    return paths[index - 1][0]

Called from csvpath/managers/results/result_registrar.py
(register_start and register_complete), using
ri = int(self.result.run_index). run_index is the plain enumerate
index i from the named-paths group loop in csvpath/csvpaths.py (around
line 875), so this is pure declared-position math: whichever csvpath is one
slot earlier in the declared list.

Where the actual data source comes from

For the ordinary (non-reference) invocation path,
csvpath/csvpaths.py, _load_csvpath (around line 706), resolves the real
input file with:

result = self.results_manager.get_last_named_result(name=pathsname, before=csvpath.identity)
...
file = result.data_file_path

And ResultsManager.get_last_named_result:

def get_last_named_result(self, *, name: str, before: str = None) -> Result:
    results = self._get_results_list(name)
    if results and len(results) > 0:
        return results[len(results) - 1]
    return None

The before parameter is accepted but never used. The real source is simply
whichever Result was most recently appended to named_results[pathsname]
in the current run, i.e. execution order, not declared position.

(The separate $-reference invocation path in _load_csvpath, around lines
689 to 701, does use position math consistent with the metadata field,
idnames[i - 1]. That path is fine and not part of this bug.)

Where they diverge

In csvpath/csvpaths.py (around lines 875 to 901), the enumerate loop
builds a Result and calls _load_csvpath for every csvpath in the group,
but skips creating a Result entirely when csvpath.will_run is False,
for example run-mode: no-run:

for i, path in enumerate(paths):
    csvpath = self.csvpath()
    if not csvpath.will_run:
        continue
    result = Result(..., run_index=i, ...)
    ...

i still advances across the skip, so preceding_instance_identity for the
csvpath after the skip reports the identity of the skipped csvpath, one
declared position back, even though that csvpath never ran and never
produced a data.csv. Meanwhile get_last_named_result reaches back to
whichever csvpath actually completed and has a Result in
named_results[pathsname], which may be two or more declared positions
further back.

Net effect: with a skip in the group, source-mode: preceding still
resolves and pulls the correct predecessor data by execution order, but the
manifest field describing that predecessor names the wrong csvpath, one that
did not run.

Suggested fix direction

Make preceding_instance_identity reflect the same execution-order
mechanism that source-mode: preceding actually uses at runtime, instead of
declared position. Two options:

  • Implement the currently unused before parameter on
    get_last_named_result so it filters to results before a given identity,
    and derive preceding_instance_identity from that same lookup, or
  • Thread the identity that _load_csvpath actually resolved
    (result.identity_or_index of the Result returned by
    get_last_named_result) through to ResultRegistrar, instead of
    recomputing it separately from index - 1.

Either way, the metadata field and the runtime data source should not be
able to disagree.

Repro sketch

A named-paths group of three csvpaths, where the middle one is marked
run-mode: no-run, and the third is source-mode: preceding. The manifest
for the third csvpath will report preceding_instance_identity as the
skipped second csvpath, while its actual data.csv input will have come
from the first.

Found while reviewing manifest fields for the references v3 work.

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions