fix: thread analysis-level Inputs into manifest input_versions (#90)#91
Conversation
The Snakefile generator split declared inputs into two paths: sibling outputs reached run_rule's inputs dict (and so were fingerprinted by write_manifest), while analysis-level Inputs (with source:) went into a separate external_inputs dict used only for recipe-template substitution. Result: lc verify reported broken_chain for every Output declaring a source-backed Input, since the manifest's input_versions never recorded them. Collapse the two paths: every declared input becomes a Snakemake input: slot — universe-templated for sibling outputs, literal for source paths. write_manifest falls through to fingerprint_external for plain files, so the manifest layer needs no change. As a side benefit, Snakemake now enforces external-source existence at DAG time and reruns on mtime drift. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
@claude can you review this PR? |
|
Claude finished @EiffL's task in 2m 36s —— View job PR Review
Verdict: LGTM — clean fix, well-tested, ready to merge. Root Cause DiagnosisThe diagnosis is correct. Fix Correctness
union21_table="data/table.txt",...and threads it into the
Test Coverage
Together the two tests give good coverage: one exercises generation, the other exercises the verify chain. Side BenefitHaving external files as Snakemake No Issues FoundNothing blocking. The change is minimal, targeted, and doesn't introduce any new abstractions beyond what's needed to fix the bug. |
✅ Eval Results
Graders✅ spec_valid (1.00) Full output |
Summary
Fixes #90. Outputs that declared an analysis-level Input (one with
source:) failedlc verifywithbroken_chainbecause the Snakefile generator never threaded those inputs into the rule'srun_rule(inputs=...)dict — sowrite_manifesthad nothing to fingerprint, and the manifest'sinput_versionscame back empty for them.The two pre-existing input paths (sibling outputs vs. analysis-level Inputs) had grown apart unnecessarily. Collapse them: every declared input becomes a Snakemake
input:slot — universe-templated for sibling outputs, literal forsource:paths.write_manifestalready falls through tofingerprint_externalfor plain files, so no change is needed in the manifest layer.Side benefit: Snakemake now enforces existence of external sources at DAG time and reruns rules when their mtime drifts (previously silent staleness).
src/lightcone/engine/snakefile.py: drop theexternal_inputsdict andresolved_inputsrebuild loop; one unifiedrule_inputslist.tests/test_snakefile.py: regression test that the externalsource:path appears as a Snakemake input slot AND in therun_rule(inputs=...)dict literal.tests/test_verify.py: end-to-end regression matching the issue scenario —lc verifynow passes for an Output backed by an analysis-level Input.Test plan
uv run pytest— 287 passed (existing 285 + 2 new regression tests)uv run ruff check— cleanokinstead ofbroken_chain🤖 Generated with Claude Code