Skip to content

feat(foundation): type the canonical base-artifact directory of an instance - #54

Merged
nilsonfh merged 1 commit into
mainfrom
fix/name-the-base-artifact-dir
Sep 16, 2026
Merged

nilsonfh merged 1 commit into
mainfrom
fix/name-the-base-artifact-dir

Conversation

@nilsonfh

@nilsonfh nilsonfh commented Sep 16, 2026

Copy link
Copy Markdown
Contributor

Summary

Phase (b) of bd 6ns.1: the canonical base-artifact directory of an instance gets its own type, BaseArtifactDir (a NewType over Path, identity at run time).

base_dir named two unrelated things: StudyProject.base_dir is the directory stage commands run in, and the twin's base_dir is <workspace>/instances/<instance>/digital_twin/base. 19 of the 25 functions taking a base_dir documented neither.

What changes

  • Producer: ArtifactLayout.base returns BaseArtifactDir. Named for its role in the layout, since the layout is a general mechanism over named instances and the twin is its main reader, not its owner.
  • Store: NetworkModelRepository.base_dir — the field. from_parquet keeps accepting any path-like, so the guarantee is about the field, not that constructor. Review raised this, and narrowing was measured rather than assumed: it makes 3 call sites fail, because generate_ev_scenarios, generate_flexibility_provider_artifacts and generate_semantic_graph still declare base_dir: Path. Typing those three signatures, none of which has a docstring yet, is the next phase.
  • Readers: twin/network/metadata.py (3 signatures), twin/adapters/validation.py (2), assets/modeling/artifacts.py::load_base_inputs, projects/dashboard_catalog.py::_load_network_metadata.
  • Adapters wrap at the call: cim.py (2) and network.py (4) pass BaseArtifactDir(out_dir), with the reason in a comment. The claim "this export writes the canonical base" is the caller's; export(out_dir=...) stays Path, because _validation_report_path already falls back when an export does not land there.
  • The 9 --base-dir boundaries convert at add_argument. Their defaults all come from the layout (8 DEFAULT_LAYOUT.base, 1 WORKSPACE.layout.base — measured, none built with Path(...)), so both paths into the parameter carry the type.
  • Six base_dir parameters gain their docstring line.

The limit, stated rather than implied. This types the value; it does not prove the directory is canonical (--base-dir /tmp/whatever still type-checks). And it holds where the type is required — the layout, the field, the readers, the argparse boundaries — not at NetworkModelRepository.from_parquet, which still accepts any path-like, so from_parquet(script.root) type-checks today. That gap is named rather than implied away, and closing it is the next phase.

Verification

  • Simulated first on a throwaway worktree, mypy A/B per (file, message): typing the producer and store with no call site touched surfaced exactly 6 errors, all in the two adapters. A positive control confirmed a plain Path into build_base_metadata is rejected, so mypy was reading the patched tree. Runtime imports: 0 failures.
  • Predicted before measuring: with the wraps in, the three ratchets must read 116 / 846 / 12, identical to main, not merely "no rise". Measured: 116 / 846 / 12.
  • On this branch: pinned black/isort/flake8 clean; 18/18 touched modules import at run time; targeted tests 102 passed (workspace layout, repository, base metadata, network adapters, twin workflows and verifiers, project catalog, import hygiene, layer direction, verb prefixes, workspace roots); full suite 2254 passed.

The suite's 2 failures are not from this branch

test_flagship_baselines_value_identical and test_project_workflow.py's regression CLI test fail in this checkout, and would fail without this PR:

Not in this phase

The generate_* / verify_* workflow functions keep base_dir: Path. Closing the argparse boundary was enough to type the value end to end; annotating those signatures is the next step, and several have no docstring to extend yet.

🤖 Generated with Claude Code

…stance

Phase (b) of bd 6ns.1. BaseArtifactDir is a NewType over Path in
gridalyn/foundation/platform/roots.py: identity at run time, distinct to mypy.

`base_dir` meant two unrelated things. StudyProject.base_dir is the directory
stage commands run in; the twin's base_dir is one instance's canonical
base-artifact directory. Of the 25 functions taking a base_dir parameter, 19
documented nothing about which.

- The producer is typed: ArtifactLayout.base returns BaseArtifactDir. The type
  is named for its role in the layout, not for gridalyn.twin, which is its main
  reader and not its owner: the layout is a general mechanism over named
  instances.
- The store's field is typed: NetworkModelRepository.base_dir. from_parquet
  keeps accepting any path-like, so the guarantee below is about the field, not
  about that constructor. Narrowing it was measured and deferred: it makes 3
  call sites fail, because generate_ev_scenarios,
  generate_flexibility_provider_artifacts and generate_semantic_graph still
  declare base_dir: Path, and typing those three signatures (none of which has
  a docstring yet) is the next phase, not this one.
- Readers typed: twin/network/metadata.py (3), twin/adapters/validation.py (2),
  assets/modeling/artifacts.py load_base_inputs, projects/dashboard_catalog.py
  _load_network_metadata.
- The two adapters wrap at the call: cim.py (2) and network.py (4) pass
  BaseArtifactDir(out_dir) into write_base_metadata and
  write_network_adapter_validation_report, with the reason as a comment. The
  claim "this export writes the canonical base" belongs to the caller;
  export(out_dir=...) stays Path, because _validation_report_path already falls
  back when an export does not land there.
- The 9 `--base-dir` boundaries convert at add_argument, so the value carries
  the type from where argparse produces it. Their defaults all come from the
  layout (8 DEFAULT_LAYOUT.base, 1 WORKSPACE.layout.base, measured, none built
  with Path(...)), so both paths into the parameter are typed.
- Six base_dir parameters gain their docstring line.

The limit, stated rather than implied. This types the value, it does not prove
the directory is canonical: `--base-dir /tmp/whatever` still type-checks. And
it holds where the type is required -- the layout, the field, the readers, the
argparse boundaries -- not at NetworkModelRepository.from_parquet, which still
accepts any path-like, so `from_parquet(script.root)` type-checks today. That
gap is named here rather than implied away, and closing it is the next phase.

Predicted before measuring: with the wraps in, the three ratchets must read
116 / 846 / 12, identical to main, not merely "no rise". Measured: 116 / 846 /
12.

Verified: simulated first on a throwaway worktree (mypy A/B per file and
message; the 6 surfaced errors were exactly the two adapters; a positive
control confirmed a plain Path is rejected). On this branch: pinned
black/isort/flake8 clean, 18/18 touched modules import at run time, targeted
tests 102 passed, full suite 2254 passed. Its 2 failures are pre-existing and
not from this branch: the flagship outputs in this checkout are from a
2026-09-07 run (git_commit f6a35e4) that predates the topology re-base, and
with those same outputs a pristine worktree of main fails the same two tests
with 42 of 94 pins mismatched.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@nilsonfh
nilsonfh force-pushed the fix/name-the-base-artifact-dir branch from bb78b59 to 328667f Compare September 16, 2026 12:47
@nilsonfh
nilsonfh merged commit 39df523 into main Sep 16, 2026
12 of 14 checks passed
nilsonfh added a commit that referenced this pull request Sep 17, 2026
On 2026-09-16 six CI jobs across five pull requests and main died in the
dependency-install step, none of them on a red test:

  ERROR: Could not install packages due to an OSError:
  ('Connection broken: IncompleteRead(214027 bytes read, 3458 more expected)')

Every one went green on a rerun of the same commit. The byte counts say what
it is: identical across the jobs of a single run (#57: both 214027/3458; #54:
both 230547/273), different between runs. One upstream response is cut and
whichever jobs are installing at that moment die together.

pip's own retries do not cover it. Every site already runs with pip's defaults
and setup-python's pip cache; those retries cover a connection that fails to
open, while a body cut mid-download surfaces from the install phase as OSError
and takes the process down. The unit to retry is the install.

tools/pip_install_retry.py retries ONLY the measured transport signatures. A
resolution error, a missing extra or a build failure still fails on the first
attempt, in seconds -- a real break must not hide behind three attempts and a
timeout. It lives in tools/ rather than as five copies of a shell loop because
inline workflow logic cannot be tested, the same reason ci_main_status.mjs is
there.

Wired into all five install steps (four in ci.yml, one in pages.yml).
Upgrading pip itself is left alone, so the retry cannot mask a broken pip.

tests/test_pip_install_retry.py drives the tool against a real child process,
including the branch that matters most -- a genuine failure is NOT retried --
and gates the wiring in both directions, so CI cannot silently stop calling it.

bd ahx

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
@nilsonfh
nilsonfh deleted the fix/name-the-base-artifact-dir branch September 17, 2026 14:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant