feat(cli,data,engine)!: align projects on the manifest layout with canonical parameters - #252
Draft
Emin017 wants to merge 93 commits into
Draft
feat(cli,data,engine)!: align projects on the manifest layout with canonical parameters#252Emin017 wants to merge 93 commits into
Emin017 wants to merge 93 commits into
Conversation
Emin017
changed the title
…as TOML Canonical parameter vocabulary is now flat snake_case across the Python layer: PDK templates, in-memory Parameters.data, all read sites, and the workspace's persisted configuration. Workspace configuration moves from home/parameters.json to home/ecc.toml ([design]/[pdk]/[flow]/[params]), written atomically via tomli-w and read via tomllib; [pdk].config is stored workspace-relative. Legacy workspaces migrate on open at load_workspace: the long-key parameters.json is normalized, rewritten to home/ecc.toml, verified, and deleted; when both files exist the TOML wins with a shadowing warning; a failed rewrite falls back to the normalized in-memory copy and retries on the next open. The RPC creation path converts GUI flat payloads (including the positional geometry aliases) through the parameter_keys module, so creation-time GUI parameters are effective instead of landing as inert duplicates. Layout-edit publish normalizes the native long-key parametersPatch at the merge boundary and writes the TOML store. Run start re-establishes home.json pointers (flow/checklist/parameters) after home.reset(). Signoff packaging reads [params] from home/ecc.toml and bundles it as initial/ecc.toml.
- dreamplace builder reads the TOML workspace config via load_parameter
instead of json_read (which silently returned {} on the new store)
- GUI scalar margin broadcasts to both core axes so it survives the
floorplan margin shape check
- die.size now drives the floorplan die_builder (mode die_size +
width/height) instead of being ignored
- a malformed home/ecc.toml never falls back to a stale legacy
parameters.json; TOML precedence is unconditional
- save_workspace_config validates [flow] before writing and converts
tomli_w serialization failures (e.g. legacy null payloads) into a
False return so migration fallback paths stay reachable
- [flow] start/end accept canonical step names only; display aliases
translate at the manifest/RPC boundary
…ayouts Projects carrying a project.json manifest (GUI projects) now resolve run directories through the manifest workspaces table: a single non-archived workspace auto-selects, --run-id matches workspace_id or the declared path tail, and unknown or ambiguous ids fail with workspace_not_declared listing the declared ids. Broken manifests surface as manifest_invalid. ecc check validates the manifest instead of demanding ecc.toml. Legacy runs/ projects (no project.json) keep their existing behavior and now carry a non-fatal legacy_layout_detected warning record on run/check/status disclosing ecc migrate.
ecc param subcommands fail with an explicit param_requires_ecc_toml error in project.json projects (the manifest param backend is a designed follow-up), and ecc init no longer pre-creates the empty runs/ container so init'd projects classify as virgin.
…tatus A first run in a virgin project creates the workspace at the project root (<root>/<run_id>) and generates project.json (schema v1, GUI draft objectives, preset-derived start/end display names, initial running status, default qor_baseline) via write-if-absent; --set values stay run-scoped and never enter the manifest. Losing the generation race discards ours and continues read-only. Runs in manifest projects layer base_design + parameter_patch beneath --set, seed the flow range from the workspace entry's start/end display names, and finalize the entry's status (success/failed) through the shared read-modify-write helper; undeclared single-segment run ids create <root>/<id> with a workspace_not_registered warning and no manifest entry, while nested or absolute ids fail invalid_run_id.
Flow targets are now ranges over the canonical harden chain, persisted per workspace in home/ecc.toml [flow] (preset form for CLI-born workspaces, start/end canonical names for GUI-born ones, seeded at creation; non-contiguous selections degrade to first..last with a log note). Workspace files carry canonical names only. engine/reconcile.py compares the target with flow.json pairwise under home/workspace.lock: a proper-prefix target appends the missing suffix as Unstart and adopts the new target; equal or target-prefix shapes no-op or resume; a stale [flow] is repaired in place (range-compared, so preset and start/end forms of the same range are not churned); divergent flows fail with flow_mismatch before any write.
ecc run against an existing workspace no longer fails run_exists; it reconciles the flow target (project ecc.toml [flow] over home/ecc.toml [flow]; manifest projects use the workspace's own section) with the persisted flow.json and: no-ops when everything succeeded, resumes from the first non-Success step, extends a proper-prefix target by appending the suffix as Unstart (prefix outputs untouched, tool deps checked only for executed steps), and fails divergent flows with flow_mismatch suggesting --overwrite or a new --run-id before any mutation. --set on an existing run is rejected with set_requires_fresh_run and [params] in ecc.toml degrades to a params_ignored_on_existing_run warning. Workspace mode (--workspace) reconciles against the workspace's own [flow] before rerun selection, so extension works there too.
ecc migrate upgrades a legacy project to the manifest layout: it prints the plan and asks for confirmation on a TTY (--yes otherwise), moves each runs/<id> workspace to the project root, rebases home.json pointers, regenerates tool configs via refresh_workspace_config, and registers every migrated workspace in a generated project.json with start/end and status derived from the persisted flow.json. A rebase failure rolls that workspace back (rename back, all-or-nothing per workspace); a run id colliding with a root entry is skipped with migration_collision while others proceed; runs/ is removed once empty. The command is idempotent: an already migrated project reports already_migrated, and a partially migrated one resumes by appending only the missing manifest entries.
Review follow-ups: - Hybrid projects (both ecc.toml and project.json) now layer the manifest base beneath the ecc.toml values instead of silently discarding it, and a declared entry still seeds the creation-time flow range. - Manifest-assembled configs carry origin_verilog (as the rtl fallback) and origin_def into workspace creation. - An existing run directory whose flow ledger has no steps fails with invalid_flow_json instead of reporting a no-op success. - Migration rollback restores workspace content, not just the directory location: home.json pointers are reverse-rebased and tool configs regenerated at the source. Manifest registration failures now fail the command (exit 1) instead of a warning, and losing the manifest creation race falls back to appending entries. - Losing the virgin manifest generation race reloads the winning manifest and writes back status only when the workspace is declared. - load_manifest validates mpc entries with the GUI parser's rules. - The run-target resolution, manifest config assembly, and existing-run reconcile wiring move to cli/project/run_prepare.py, and the migrate handler to cli/project/migrate.py, returning command_handlers/ project.py under the module-size budget.
- The CommandInput protocol members become read-only properties, which is what the frozen input dataclasses actually provide; this removes the pre-existing execute_command variance error class rather than adding a new instance for migrate. - normalize_parameter_dict exposes the dict-shaped normalize path with a plain dict return so neither ty's isinstance narrowing (dict[Never] keys) nor a narrowed/delared union poisons downstream .get calls. - ProjectConfig gains manifest_parameters/manifest_origin_def fields instead of dynamically assigned attributes. - Manifest objectives normalization mirrors the GUI parser exactly: primary defaults to timing, directions keep only maximize/minimize entries from the source.
code-simplifier pass over the new modules (mapping-table payload split/merge, _flow_step_template reuse, _find_workspace_entry extraction, stdlib import hoisting) plus the final codex review fixes: - target-prefix reconciles are unconditional no-ops even when steps beyond the target are unfinished; a real crash window (suffix Unstart + stale [flow]) repairs then resumes - hybrid projects (ecc.toml + project.json) use the project [flow] as the existing-run target and read the manifest base layer only after the real run name resolves - ecc migrate rejects a broken/identity-less ecc.toml before moving anything, and failed workspaces register as failed (the persisted state string is "Incomplete", not "Imcomplete") - virgin manifest writes are fully atomic (temp file + link create-if-absent); generated base_design.parameters includes the ecc.toml [params] resolution on both the virgin and migrate paths - legacy workspace migration seeds [flow] from the persisted flow.json first/last steps (invalid derivations degrade to absent) - layout-edit publish renders home/ecc.toml through the shared render_workspace_config, restoring workspace-relative pdk.config and dropping the stray [params._flow] - ecc config surfaces manifest selection errors; ecc check in a manifest project with an ambiguous selector fails with workspace_not_declared instead of a misleading checked record - [flow] preset values are validated against the known builders
- target-prefix reconciles no longer rewrite the workspace [flow] at all: extra persisted steps are kept untouched and reconcile never widens intent, so back-to-back runs both no-op instead of the second one resuming the suffix - ecc migrate rejects semantic config errors before moving anything (bad params, missing design/pdk identity, unknown pdk/preset), while filesystem existence checks stay out of the migration path - ecc check surfaces manifest selection errors for hybrid projects too (ecc.toml present), not only manifest-only ones - legacy flow seeding tolerates a non-object flow.json instead of crashing the workspace open
The integration conftest and the filelist/stdio/requests fixtures still wrote legacy long keys, which landed as inert duplicates after the canonical flip and broke the full uv run pytest gate.
A non-contiguous flowConfig.steps selection previously wrote the explicit steps into flow.json while [flow] widened to first..last, so the two stores contradicted each other and the next reconcile saw a divergence. One shared resolver now widens both stores to the same contiguous range with the log note.
A target-prefix workspace (persisted flow covers more than the target) now adopts the effective target section into home/ecc.toml [flow] when its persisted section is stale, per AC-13. Adoption always writes the effective target itself, never a range derived from the ledger, so extra persisted steps are kept but never become the target; execution remains an unconditional no-op.
resolved_base_parameters now projects the ecc.toml-resolved canonical payload through parameters_to_geometry, so positional values surface as the wizard's aliases (utilitization, margin, die_width) instead of backend nested subtrees.
…y commands Unknown top-level keys in a GUI flat payload are now logged before persistence (kept for forward compatibility, never silently shadowing canonical ones). The single-segment run-id validator is shared with the read-only manifest discovery path, so nested or absolute --run-id values fail with invalid_run_id instead of workspace_not_declared.
Hybrid projects now fill missing identity/pdk/rtl fields from the manifest base before validation (explicit ecc.toml values always win), layer the workspace entry's parameter_patch after run-name resolution, seed the entry flow range only when the project ecc.toml has no flow target (project [flow] outranks the manifest entry), and emit a config_layer_diverged warning when the two layers disagree.
- Registration failure now rolls the whole unregistered batch back to runs/ (previously moved workspaces were stranded at the root and a retry reported "already migrated") - An existing project.json is validated semantically before the first rename, so a malformed manifest fails before any move - Workspace-local pdk config paths are rebased before the moved workspace loads (both legacy parameters.json and home/ecc.toml), so they never point back at the old source - D7 status precedence: any Incomplete marks the workspace failed, outranking Ongoing/Pending
update_manifest now re-reads before replacing and re-applies the mutator to the freshest document when an unrelated change lands mid-update, so concurrent GUI edits are never silently overwritten. Project-level updated_at is owned by each mutator: status write-back now touches only the target entry's status and updated_at, and failed write-backs surface as manifest_write_back_failed warning records without failing the run.
…config load_workspace_config now derives an absent [flow] from the persisted flow.json first/last steps (D5a fallback), and TOML parse/flow validation failures propagate instead of collapsing into an empty payload, so runs report workspace_config_invalid rather than invalid_workspace (and never fall back to stale legacy JSON).
- Fresh-run preparation and execution moves from cli/command_handlers/project.py to cli/project/run_prepare.py (execute_fresh_run), along with the manifest-mode check handler; the handler is back to input/result mapping at 547 lines - The legacy parameters migration and the flow selection resolver move from data/workspace/__init__.py to data/workspace_config.py, next to the store they operate on - The RPC creation converter and the layout-edit TOML staging helpers move to the new runtime/workspace_config_io.py New modules stay under the size budget; no CLI startup import cost (verified: no chipcompiler.data modules on the app import path).
Two round-12 review fixes: - find_config_path now detects lexical presence (lexists) instead of readability: a symlink loop, directory, or stat-failing ecc.toml is PRESENT, so the read that follows raises ConfigUnreadableError and the project fails loud instead of being silently demoted to the manifest layer; - manifest workspace selection matches the DECLARED path tail from the raw document again: canonicalizing workspace_path for execution must not rename the selector — a symlinked declared path is found by its declared name, and the canonical target name is not accepted as an alias the document never spelled.
…ases Two round-13 review fixes, same lexical-presence/declared-spelling class as the previous round: - find_manifest now uses lexists like find_config_path: a directory, symlink loop, or dangling symlink at project.json classifies the project as manifest and fails load_manifest with manifest_invalid, instead of silently demoting to the virgin/legacy layout; - an undeclared run id that canonically lands on a DECLARED workspace's path is refused with workspace_not_declared naming the declared selector — resuming or overwriting that workspace under an alias the document never spelled would bypass its registration and status write-back.
…classification gaps
… drop unsavable nulls
…harden config boundaries
… disambiguate include anchors
…ration destination, validate effective manifest parameters
…eflight - Take the workspace lock before create_workspace makes the run target discoverable (held through seeding and engine execution), so a concurrent run can never classify the fresh target as existing and win the race to execute it; the lock file moves to a sibling path (<workspace>.lock) so it predates and survives the creation. - Split the existing-workspace run path out of run_prepare (700-line budget) into run_existing. - Validate manifest-layer parameter values in the shared validate_effective preflight so ecc check reports them too; values overridden by explicit ecc.toml/--set keys stay inert. - Treat --overwrite as a fresh run during preflight (the target is wiped and recreated) and reject symlinked home/home.json in the existing-target guard.
…st validation - run_steps' full-ledger completeness check no longer fails target-bound resumes (the persisted ledger may legitimately be wider than the reconciled target; fresh runs keep the strict default). - Validate and present manifest parameters on the canonical projection so GUI-flat geometry aliases (utilitization, margin, aspect_ratio) face registry range checks and show up in config --resolved; raw non-numeric values now fail instead of silently skipping range checks. - Reject candidate patches against a missing workspace parameters config instead of recreating it from the patch alone. - Avoid double-closing the project fd when the migration destination identity check fails.
A present-but-undecodable or wrong-shaped home/flow.json previously degraded to the empty-ledger defaults, so the migration registered a fabricated Synth..Harden range and not_started status that contradict the persisted ledger — the next run would fail flow_mismatch against its own manifest. The ledger read now distinguishes absent (migrates with not_started defaults, a workspace that never ran) from malformed (blocked with migration_unsupported and left under runs/, including a step record without a name).
A resume plan containing only blocked workspaces reported already_migrated with exit 0 while those workspaces were still under runs/ — the early return now requires no blocked entries too. A present flow.json without a valid steps field is likewise malformed (the engine always writes one), not an empty ledger, so it is blocked instead of fabricating Synth..Harden defaults.
…xecution Round-14 targeted review fixes: - the explicit --workspace path moved out of the command handler into run_workspace.py: everything from the locked re-probe through execution holds the sibling workspace lock, and the default resume is bounded to the reconciled target — a wider persisted ledger is never re-executed or invalidated past the target end (rerun gains a `through` bound on run_resume/run_from and bounded_resume_names); - ecc migrate serializes each move with an active execution of that workspace via its existing sibling lock (opened only when present, so discovery skips *.lock artifacts and a clean migration still removes an empty runs/); - a virgin run that loses the manifest generation race binds to the winner only when the winning entry declares this id at THIS path — a same-id winner elsewhere receives neither the run nor its status; - rollback reports truthfully: a workspace moved back with unrestored pointers/config is migration_rollback_incomplete, not rolled_back; - --set values that override a different lower-layer value (manifest base or ecc.toml [params]) now emit config_layer_diverged on the same canonical projection as the other layer pairs.
An --workspace run against a nonexistent path took the sibling lock before failing, leaving the parent directories and a stray <name>.lock behind. The existence check now precedes lock acquisition — a command with nothing to run never mutates the tree. rerun.py's new through/last bounds use explicit Optional annotations, and the workspace-run wiring tests materialize their virtual workspace on disk to match the locked execution design.
Every migration-only function now carries a static typing_extensions.deprecated marker (category=None: pyright/IDE-visible, zero runtime warnings — no DeprecationWarning spam on every workspace open). Two families: the legacy runs/ -> manifest layout machinery (migrate.py, migrate_plan.py, migrate_fs.py helpers, project_migrate_lock, existing_workspace_execution_lock, the migrate command handler) and the legacy parameters.json -> ecc.toml config migration (migrate_legacy_parameters, legacy_parameters_fallback, legacy_parameters_path, workspace config-filename migration). Deliberately unmarked: flock_file (shared with the permanent .manifest.lock writer coordination), _derive_flow_from_ledger (also the permanent hand-broken-config fallback), is_workspace_config (recognizes both formats, not migration machinery).
…t remnants Rebase follow-up onto main 1f910c3 (#111 Yosys LEC, #250 obsolete flow config removal, #251 Sizer legalization): the canonical chains now carry Timing optimization and postRouteLec and no longer contain fixFanout. - MANIFEST_FLOW_STEPS, PRESET_MANIFEST_RANGE (incl. the new synthesis_lec preset), the migration display map, and the workspace step-name alias table all cover the new steps; - the removed netlist-opt step's leftover mapping table entry, fixfanout config write, checklist class, runner function, and metrics builder are deleted (main removed the feature); yosys_lec reads the canonical lec parameter key; - ecc-tools submodule follows main's pin (f6ee10aa, a descendant of the branch's logger-throw pin); - tests updated to the new chains; the two ics55 integration tests fail locally only because the Sizer binary is absent — main fails them identically with the PDK linked (verified on an origin/main worktree).
Emin017
force-pushed
the
emin/project-alignment
branch
from
September 3, 2026 01:42
4de21cf to
fcb93fb
Compare
…et path On filesystems that recycle inodes immediately (overlayfs on CI), a fresh replacement directory can land on the moved workspace's freed inode, so the anchored child_stat identity proof wrongly counted it as recovered — the rollback then reported a clean failure without the incomplete marker, and the unconfirmed object was treated as restored. A missing target is now always an incomplete rollback: no object is touched, and the user inspects manually. The proof had no realistic trigger and no test coverage.
The workspace's persisted configuration moved from home/ecc.toml to home/params.toml, so a project has exactly one human-editable TOML (the project-root ecc.toml) and the machine-managed snapshot no longer shares its name. Content, sections, and the canonical vocabulary are unchanged; the parameters.json migration path now targets params.toml directly (the ecc.toml name never shipped — no compatibility layer for it). The three sites that hardcoded home/ecc.toml now go through workspace_config_path so the name is single-sourced. When both params.toml and a legacy parameters.json exist in a workspace, the TOML still wins — and run/check/status now append a workspace_config_shadowed warning record at the command boundary, telling the user the JSON is inert and safe to delete.
Codex review of 8620894 found four gaps: - the agent foundation extractor still preferred home/ecc.toml, so a params.toml-only workspace extracted with unknown design/PDK context — it now prefers params.toml through workspace_config_path, falling back to the legacy JSON; - the shadow warning probed ctx.run_dir even for run --workspace — it now probes the explicit workspace target, and uses lexists so a dangling legacy link still counts as shadowing (the spec'd semantics); - the canonical filename was not fully single-sourced: signoff packaging, the runtime snapshot's pointer fallback, and reconcile's adopt-failure message all built the path by hand — all now go through workspace_config_path.
…ale names Round-2 review of the params.toml rename found two leftovers: the signoff collector's [params] read still built the path by hand (now through workspace_config_path like the packaging point), and the deprecated markers plus merge docstring in workspace_config.py still named ecc.toml as the migration target.
The round-3 review's P3 sweep: the deprecated marker in workspace/__init__.py, candidate_materialization.py's docstring, the gcd example trees (README.md, README.cn.md, both ics55flow scripts), and a legacy-readonly test's comment and variable still named the workspace config file ecc.toml. All now say params.toml. The project-level ecc.toml references are intentionally untouched.
Round-4 P3 sweep: two test docstrings, one comment, and one test name still said ecc.toml where the workspace config (home/params.toml) is meant; all renamed. No behavior change.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What Changed
chipcompiler/data/parameter_keys.py): parameter keys are now flat snake_case (design,top_module,frequency_max, ...) across PDK templates, in-memoryParameters.data, every read site, and the persisted workspace config. GUI/RPC payloads — including the positional geometry aliases — are converted at the boundary so creation-time GUI parameters are effective instead of landing as inert duplicates. Breaking: the long keys ("Design", "Top module", ...) are no longer produced or consumed.home/parameters.json→home/ecc.toml([design]/[pdk]/[flow]/[params],[pdk].configstored workspace-relative), written atomically viatomli-wand read viatomllib. Legacy workspaces migrate on open atload_workspace: normalize → rewrite → verify → delete the JSON, falling back to the in-memory copy and retrying on the next open if the rewrite fails; when both files exist the TOML wins with a shadowing warning. Signoff packaging bundles it asinitial/ecc.toml.project.jsonis generated on virgin first run and keeps run status write-back; runs resolve from the manifest with hints for legacyruns/layouts;ecc paramis rejected in manifest projects; manifest presence is detected lexically and manifest flow ranges are validated against the canonical chain. Effective-config layering (GUI-flat / hybrid / base) is unified betweenecc checkandecc runand validated before execution.engine/reconcile.py,cli/project/run_*): persisted flows are reconciled against flow targets; running against an existing workspace serializes on the project lock, revalidates run state inside the lock, bounds resume to the target, locks fresh runs before publish, and commits terminal step state after fallible post-success work. Failed fresh runs stay artifact-free.ecc migrate: upgrades legacyruns/projects to the manifest layout — exact preview with confirmation, per-workspace transactional move anchored to confirmed filesystem identities viarenameat2(all-or-nothing per workspace, symlink sources refused), idempotent resume with collision reporting, serialized on a project lock. All migration-only functions are markedtyping_extensions.deprecated(static-only,category=None: pyright/IDE-visible, zero runtime warnings) to mark them as sunset machinery.ecc-toolsharden QoR area gates read the canonical mpc key.test_overwrite_guard.pysplit along ownership boundaries.Scope
Select the areas touched by this PR:
uv.lock, or release artifacts.Runtime And Packaging Impact
ecc-toolsorecc-dreamplacedependency changedNotes:
tomli-w>=1.0(pure-Python TOML writer) for atomichome/ecc.tomlwrites;uv.lockupdated. Noecc-tools/ecc-dreamplaceversion change.ecc migratecommand (JSON statuses such asalready_migrated,migration_collision);project.jsonmanifest;workspace.createexample payloads now use canonical flat keys.runs/<id>;home/ecc.tomlreplaceshome/parameters.json(legacy files migrate on open); signoff bundlesinitial/ecc.tomlinstead ofparameters.json.Validation
List the commands you ran. Mark checks that are not applicable as N/A.
uv run pytest test/— run aspytest test/ --ignore=test/examples/test_soc.py(CI parity, ics55 PDK cloned per CI setup): 1789 passed, 3 skipped, 4 xfaileduv run ruff check chipcompiler test— all checks passeduv run ruff format --check chipcompiler test— 261 files already formattedecc --help,ecc --version,ecc version --jsonnix run .#cli -- --helpSkipped checks and reason:
test/integration/test_harden_flow.py::test_ics55_gcdin the suite above.Checklist