Repository navigation
refactor(writeback): give each promotion and recovery job its own module - #2132
Conversation
Code metrics gate: PASSED with waived findings (
|
| Dimension | Limit | New | Worse | Baseline out of date | Baseline entries | vs base |
|---|---|---|---|---|---|---|
| Cyclomatic complexity | fails > 20 | 0 | 0 | 0 | 6 | +0 |
| Cognitive complexity | fails > 30 | 0 | 0 | 0 | 54 | +0 |
| Function length (lines) | fails > 50 | 2 | 0 | 0 | 550 | -2 |
| Nested block depth | fails > 4 | 0 | 0 | 0 | 45 | +0 |
| Parameter count | fails > 5 | 1 | 0 | 0 | 81 | -1 |
| Module length (lines) | warns > 800, fails > 1200 | 0 | 0 | 0 | 44 | -1 |
| Duplicated lines per file | fails > 0 | 0 | 0 | 0 | 95 | +0 |
| Dead code (vulture) | fails > 0 | 0 | 0 | 0 | 2 | +0 |
| Check | Findings | Effect |
|---|---|---|
| Comments | 0 | |
| Broad catch without a reason | 0 | |
| English only | 0 | |
| Production code imports test code | 0 | |
| Repeated literals | 0 | |
| A1 Private name of another package | 0 | |
| A8 Flag argument | 0 | |
| A9 Tuple return | 0 | |
| A10 Raw vocabulary comparison | 0 | |
| A11 Hidden global write | 0 | |
| A12 Pure forwarder | 0 | |
| A12 Single-implementation abstraction | 0 |
New violations (not in the baseline) (waived by the baseline-raise label): 3
| Unit | Dimension | Value | Limit |
|---|---|---|---|
src/hyperloom/orchestrator/loop/_keep_patch_column.py:233 AgentKeepStagingMixin._stage_agent_keep |
Parameter count | 6 | fails > 5 |
src/hyperloom/orchestrator/loop/_baseline_failures.py:277 BaselineFailureMixin.handle_unpromotable_result |
Function length (lines) | 55 | fails > 50 |
src/hyperloom/orchestrator/loop/_resume_recovery.py:233 ResumeRecoveryMixin._resume_recover_pending_warm_replay |
Function length (lines) | 75 | fails > 50 |
How to fix
- New or worse: split the unit (extract a function, a class, a module) until it is back under the limit, or no worse than its recorded value. The baseline cannot absorb a new violation: CI refuses any baseline entry added or raised relative to the base branch.
- Baseline out of date: something improved. Run
python scripts/code_metrics.py --update-baselineand commitscripts/code_metrics_baseline.txt; the command only lowers or removes entries. - Override: the
baseline-raisePR label waives new, worse and baseline-growth findings and the failing design checks (they stay listed here). Use it only with the reason in the PR description; it waives nothing else. - Tools for a local run:
pip install ruff==0.16.2 complexipy==8.0.1 vulture==2.16andnpm install -g jscpd@5.4.1(or pointCODE_METRICS_JSCPDat another jscpd command, such asnpx --yes jscpd@<version>).
26 findings in files this change does not touch
Outside this change (informational): 26
| Unit | Dimension | Baseline | Now |
|---|---|---|---|
src/hyperloom/orchestrator/actions/executors/_integrate_apply.py ApplyStageMixin._stage_apply |
Cognitive complexity | absent | 32 |
src/hyperloom_kb/collect/mapping.py _experience |
Cognitive complexity | absent | 31 |
src/kernelforge/cli/forge_loop_command.py forge_loop |
Cognitive complexity | absent | 31 |
src/kernelforge/loop/runner.py IterationLoop._collect_session_candidate |
Cognitive complexity | absent | 44 |
src/kernelforge/loop/runner.py IterationLoop._rule_on_attempt |
Cognitive complexity | absent | 38 |
src/hyperloom/orchestrator/phases/kernel.py KernelPhase._record_geak_delegation_timeline |
Parameter count | absent | 7 |
src/hyperloom_kb/http_service.py |
Module length (lines) | absent | 1525 |
src/kernelforge/cli/forge_loop_command.py |
Module length (lines) | absent | 2003 |
src/hyperloom/orchestrator/specialists/dispatch.py SpecialistDispatchCollaborator.warm_specialist_params |
Cognitive complexity | 93 | 94 |
src/hyperloom/inference_optimizer/cli/preflight.py _probe_rocm_build |
Function length (lines) | 54 | 57 |
src/hyperloom/inference_optimizer/breakdown/schema.py |
Module length (lines) | 2681 | 2686 |
src/hyperloom/inference_optimizer/cli/__init__.py |
Module length (lines) | 2569 | 2739 |
src/hyperloom/orchestrator/actions/executors/_kv_metrics.py |
Module length (lines) | 1578 | 1579 |
src/hyperloom/orchestrator/actions/executors/roofline.py |
Module length (lines) | 1303 | 1305 |
src/hyperloom/orchestrator/kernel/conc_sweep.py |
Module length (lines) | 1429 | 1449 |
src/hyperloom/orchestrator/phases/kernel.py |
Module length (lines) | 4102 | 4199 |
src/hyperloom/orchestrator/phases/prelude.py |
Module length (lines) | 2504 | 2506 |
src/hyperloom/orchestrator/prompts/prompt_builder.py |
Module length (lines) | 1205 | 1212 |
src/hyperloom/orchestrator/prompts/specialist_prompt_builder.py |
Module length (lines) | 2556 | 2567 |
src/kernelforge/fusion/command.py |
Module length (lines) | 1584 | 1708 |
src/kernelforge/fusion/validate.py |
Module length (lines) | 1513 | 1514 |
src/kernelforge/gemm_tune/tuners/_aiter_dense_common.py |
Module length (lines) | 1399 | 1463 |
src/kernelforge/loop/insession_gate.py |
Module length (lines) | 1218 | 1260 |
src/kernelforge/loop/knowledge_integration.py |
Module length (lines) | 1457 | 1459 |
src/kernelforge/loop/runner.py |
Module length (lines) | 6051 | 6510 |
...and 1 more; the job log lists every row.
Verdict computed by the gate scripts in
/home/runner/work/_temp/code-metrics-gate, not by this tree's copy.
Thresholds and sources: [tool.hyperloom.code_metrics] in pyproject.toml. Tools: complexipy 8.0.1, jscpd 5.4.1, ruff 0.16.2, vulture 2.16.
| from ..state.shared_state import SharedState | ||
| from ._integrate_promotion import _patch_recipe_delta | ||
| from ._keep_patch_column import _source_layer_handles | ||
| from .proposals import PendingProposal |
54435f5 to
5026b99
Compare
`orchestrator/loop/writeback.py` had grown to 4156 lines: a hundred-method `WritebackCollaborator` plus the ~750 lines of helpers it read. The jobs it had collected -- staging a KEEP into the patch column, validating cumulative gain, charging a baseline failure, lifting a winner into current_best, promoting each of the five result kinds, pinning the launch a measurement ran under, replaying a resumed log, recovering the windows a crash left open, and auditing a resumed run -- each get a module, and the five-result dispatcher stays where it is. Every unit moves verbatim; the twelve stage modules are mixins the collaborator composes, so `coordinator.py` still constructs the same class. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
5026b99 to
0f4519d
Compare
chaojhou
left a comment
There was a problem hiding this comment.
PR #2132 -- refactor(writeback): give each promotion and recovery job its own module
What it does: splits src/hyperloom/orchestrator/loop/writeback.py (4156 lines after #1956) into one module per promotion and recovery job, as mixins on WritebackCollaborator, leaving writeback.py at 239 lines. Call sites and method names are unchanged, and the code-metrics baseline is regenerated for the new modules.
Blocking issues: none
Checked:
- Equivalence. An AST comparison of every definition in the merge-base
writeback.pyagainst its new location found no body changes beyond the move itself, and no module-level state duplicated across modules. - Wiring. Every import of, attribute access on, string path to and test patch of a moved name was checked at head. Each patch lands on the module that reads the name, and no cross-package
_-name import is added. - Baseline. Three moved methods lose their old baseline entries and get no new ones. This does not fail a later PR:
code_metrics.py'sexcuse_base_backlogtreats a unit that is no worse than at the merge base as backlog. - Main. The PR merges cleanly with current main 7e98e1f. The directory gate from #2136 (50 entries) holds:
orchestrator/loop/has 31 entries after the merge.writeback.pyhas no dedicated CODEOWNERS rule, so ownership is unchanged. - Description. Every per-module line count matches the head.
Ran:
- the AST equivalence script over the merge base and the head modules;
git merge-tree --write-tree origin/main 0f4519dc7(clean), and the directory-entry count of the merged tree;- CI at head: all required checks are green. The two cancelled jobs were superseded by successful reruns.
Head: 0f4519d
LGTM
writeback.py was split into per-job modules on main (#2132); re-apply the winner_vetoed warning and best_updated emit in _current_best_lift.py. Co-Authored-By: Claude <noreply@anthropic.com>
…2149) #2132 moved IntegrateRecoveryIncomplete from loop.writeback into loop._promote_outcome while #2130 added a test importing it from the old path, so collection fails on main and every test shard goes red. Signed-off-by: zoroyihan7 <Yihan.Wang@amd.com> Co-authored-by: Claude <noreply@anthropic.com>
* Move profiler setup abd patching to common path * Fix shared profiler preparation for native AgentX * Simplify shared profiling setup and refresh patch status * Add default values for num_steps * Set detailed annotations to always true as Magpie check compatibility * chore(metrics): lower the _workload_envs module-lines baseline to 1664 * refactor(writeback): bring every function in the writeback module under the complexity ceiling (#1956) * refactor(writeback): lift the GEAK revalidation ruling out of _promote_explore _promote_explore carried the whole GEAK stack-revalidation path inline, so one method held the rebench reading, the grading that decides it, the overlay survival check, the material gate, and each of the four ways a ruling can be carried out. Give each of those its own name. _promote_explore: 493 -> 185 lines, cognitive 237 -> 79, cyclomatic 48 -> 24, nested depth 7 -> within the limit. Observable effect: none -- this is a pure refactor. No behaviour, interface, default, flag, or artifact changes. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * refactor(writeback): thread the rebench through one record, not eight locals The GEAK ruling's steps each took the measurement, the hash, the current best and the candidate as separate arguments, which both repeated the derivation and pushed three signatures past the parameter limit. Read the rebench once into _GeakRebench and pass that: current_best and candidate are then captured before a promotion can move them, so every arm judges the state the rebench was measured against. _rule_on_geak_rebench: 109 -> 78 lines, 7 -> 3 parameters. Observable effect: none -- pure refactor. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * chore(metrics): lower the baseline the merge left out of date Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * refactor(writeback): drop the geak_result assignment nothing reads CodeQL flagged it on this PR because the extraction moved the line, but it is the same dead assignment main carries: the except branch binds geak_result and the refusal branch below rebinds it before any read, on every path that reaches either. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * chore(metrics): lower the module-length entry the dead-line removal left stale Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * refactor(writeback): give the GEAK revalidation, harness replay and launch identity their own modules The writeback module carried four jobs at once: settling a task result into SharedState, ruling on a GEAK revalidation rebench, replaying a GEAK win through GEAK's own harness, and describing the launch `current_best` was measured on. The last three now live beside it, each as one module reading as one job, and the settlement handlers it keeps name the steps they take rather than inlining them. No observable change: internal structure only. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * refactor(writeback): name the steps of the GEAK revalidation and the launch descriptor Each of the three moved modules now states its work as a sequence of named steps: what an accepted GEAK result offers to deploy, the params that re-measure it, how a 2a replay is run and what its accuracy and overlay earn it, and which layers reconstruct the launch `current_best` was measured on. The rebench's engagement evidence travels as one value instead of four parallel keyword arguments. The "did not validate" summary drops the accuracy fields it could never reach: an accuracy failure returns before it. No observable change: internal structure only. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * refactor(writeback): settle each promote path through steps that name what they settle The per-kind promote handlers, the current_best lift and the unpromotable-result path each read as the sequence of decisions they make: what grading refuses, what a stack row is attributed to, which recipe delta a KEEP re-applies, what a failed baseline is charged to, and what the anchor run's facts are. The values that travel together — a lift request, the launch it resolves to, a crashed integrate window's marker, a scan of the event log — are named types rather than parallel locals and keyword arguments. Two duplicated decisions go with it: the profile and roofline handlers cleared the pending auto-roofline pointer twice under the same condition, and the GEAK 2a summary carried accuracy fields that an accuracy failure returns before it can ever reach. No observable change: internal structure only. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * refactor(writeback): name the two rulings a GEAK revalidation can reach `validated` and `fallback` are the 2b ruling's own vocabulary, read by the harness replay as well, so they are named once where the ruling lives. No observable change: internal structure only. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * chore(writeback): store the writeback modules with LF endings A scripted edit on Windows rewrote them with CRLF, so every line of all four read as changed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(writeback): report an orphaned framework KEEP under the kind that produced it The resume alert for a KEEP the stack lost reported the stack family label, ``framework``, where it had always reported the producing kind, ``framework_agent``. An operator filtering the resume report on the kind no longer saw them. The eval-failure trigger helpers lost their dependency on the collaborator with it: the enablement contract tests bind `_persist_eval_failure` onto a stand-in that carries SharedState alone. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * refactor(writeback): give the promote outcome its own module so the writeback modules do not import each other in a cycle ``_PromoteOutcome`` is the vocabulary the promote handlers and the GEAK revalidation share, so it sits beside both rather than in one of them. The GEAK modules now name `Task` through a plain import as well, which is what it always was. No observable change: internal structure only. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * chore(metrics): drop the function-length entry _resume_consistency_pass no longer earns Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(writeback): confirm integrate recovery before registering its levers An unconfirmed recovery sets the stop reason, saves the session and raises, so the levers a patch authored must not already be in that saved state: on resume the recovery can roll the patch back while explore keeps benching switches whose code is no longer on disk. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * chore(metrics): record the line writeback.py lost with the eval opt-out Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * refactor(writeback): rule on a GEAK win in one module instead of two that import each other The 2a harness replay is the 2b rebench's own fallback: it is reached from the rebench ruling, hands its verdict back as a _ReplayRuling, and shares that ruling's decision vocabulary. Splitting it off left the replay importing the revalidation at module level while the revalidation called into the replay through the mixin, which is a cycle with the evidence removed. One module, one mixin and the cross-module import go. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * refactor(loop): reduce the helpers' docstrings to their summary line The restructuring these files needed took writeback's function count from 76 to 176, and the per-function docstring convention took the docstring budget with it. The helpers added here keep a docstring, reduced to its summary line; the Args:/Returns: blocks go, and so does elaboration the signature already carries. The one sentence that states an invariant, a constraint from outside the file, or why a slower path is the correct one is kept where it exists. No behaviour change. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * chore(code-metrics): lower the module-length baseline the merge made stale Merging origin/main shortened the module, so its recorded module-lines entry no longer matches the tree and the gate reports the baseline as out of date. Lowered to the value CI measured; nothing else changed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * refactor(orchestrator): let the collaborator base declare what it borrows `collaborator.py` is imported by every collaborator in the orchestrator, and it named `Coordinator` and `CoordinatorState` to annotate the object it borrows from. That pointed the package root back up into `loop.coordinator`, and it was the return leg of every import cycle CodeQL reports across the `loop` package: 321 cyclic import edges repo-wide collapse to 95 once it is gone. The borrowed surface is declared structurally instead, inside the `TYPE_CHECKING` block the file already had, so the contract is stated by the layer that consumes it and points down. `Coordinator` satisfies it unchanged. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * refactor(orchestrator): drop the protocol, widen the borrowed annotation The protocol restated nineteen members of the Coordinator in the module that borrows them -- a second declaration of a surface the Coordinator already owns, and it narrowed `_coord` for every subclass that reaches past those nineteen (245 new mypy findings). Widening the parameter instead deletes the same import with no second declaration: `cast` keeps the borrowed types the properties hand to callers, and mypy is back at its pre-change count. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Lower the writeback module-lines baseline the merge made stale The merge hoisted the perf_metric imports to module scope, removing three lines from writeback.py. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * refactor(writeback): keep the launch descriptor and the promote outcome in writeback Fold `_launch_identity.py` and `_promote_outcome.py` back into `writeback.py` and restore every function they held to its merge-base body, except `build_env_spec`: its two near-identical `source_patch` appends become one comprehension, the recipe path is read once instead of three times, and the promoted-measurement fallback is read in place. `_geak_revalidation.py` no longer imports `_PromoteOutcome`: the GEAK ruling returns the verdict and `_promote_explore` records it, so the loop package's import graph points one way. `_integrate_keep_recipe_delta` and `_replayed_recipe_delta` built the same recipe delta from two different fallbacks; they are now one function that takes the fallback. `_server_arg_workload_fields` is back inside its one caller. No observable change: internal structure only. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * refactor(writeback): drop the helper docstrings that restate their own names The one-line docstrings on the promote, keep-origin and GEAK-revalidation helpers repeat the name above them and nothing else; the GEAK module's own header restated the method list below it. Delete them, and fold the two branch-free record builders back into their single callers. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs(writeback): keep the revalidation-conclusion invariant with the method ``_record_geak_rebench_conclusion`` only moved module; its docstring states that the recorder declines silently once the kernel event has closed, which is an invariant the name does not carry. Restore it verbatim from the merge base. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * chore(metrics): record the writeback module length the merges left out of date The entry read 3972 while the file is 4156; the gate excused it as base backlog because the merge base is 4717, so nothing reported the drift. --------- Co-authored-by: chennyiiis <chenyi@amd.com> Co-authored-by: Claude Opus 5 <noreply@anthropic.com> Co-authored-by: Test <test@test.com> Co-authored-by: xiaofei.zheng <xiaofei.zheng@amd.com> * feat(plugs): add the plug kernel, built-in profiles and the events socket (#2110) * feat(plugs): add the plug kernel, built-in profiles and the events socket Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: Claude <noreply@anthropic.com> * refactor(plugs): read the entry-point name once Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: Claude <noreply@anthropic.com> * Fix CodeQL findings in test_host activated test Co-Authored-By: Claude <noreply@anthropic.com> * fix(plugs): record a plugin name two distributions declare, and drop the test-only legacy groups iter_entry_point_objects skipped every name it had already yielded, so a second distribution declaring the same implementation name for an open socket never reached the registry: the first one won by metadata order and plugin_errors stayed empty. That dedup existed only for legacy_groups, which no caller passes. Both are gone; the registry's own duplicate check now records the conflict. load_manifest_file's docstring says JSON, which is all it reads. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Signed-off-by: Claude <noreply@anthropic.com> Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: Hou <chaojhou@amd.com> * feat(hosted): read and check the host-injected inputs in one place (#2114) * feat(hosted): read and check the host-injected inputs in one place Add hosted/host_info.py: read_host_info(env, require=...) checks the inputs a host injects into a hosted run, names every missing or malformed variable in a single error, and returns a frozen HostInfo. Credentials are held in a Secret that never prints. Document the variables. Co-Authored-By: Claude <noreply@anthropic.com> * fix(hosted): keep a URL query and a template key out of host-info errors and reprs A credential in a URL query or fragment would have been stored and printed in the HostInfo repr, and a malformed template entry's key was echoed in the error. Refuse query and fragment in the URL inputs and name no key in the template error. Co-Authored-By: Claude <noreply@anthropic.com> * test(hosted): derive the credential sentinels instead of spelling key-shaped literals The secret scanner flagged the fixture values. Sentinels derived from the variable's position are as distinctive for a substring search and carry no key-shaped literal. Co-Authored-By: Claude <noreply@anthropic.com> * feat(hosted): take_host_info removes every host-only input from the environment A scheduler that starts children without env= cannot strip inputs per launch, so the inputs must not stay in os.environ. NODE_NAME and CLAW_SESSION_ID stay because other code reads them. Co-Authored-By: Claude <noreply@anthropic.com> * feat(hosted): hold the archive region in HostInfo Co-Authored-By: Claude <noreply@anthropic.com> * feat(hosted): make the result archive optional; shared storage when unset HYPERLOOM_RESULT_ARCHIVE is unset (results stay on the shared storage) or s3://<bucket>/<prefix>. The AWS key pair is required only for s3://. Co-Authored-By: Claude <noreply@anthropic.com> * fix(hosted): refuse an archive prefix with an empty or '.' segment instead of rewriting it _archive_location split the prefix with PurePosixPath, which drops '.' segments and collapses repeated slashes before the check runs, so s3://b/a/./b, s3://b//a, s3://b/a// and s3://b/a/. were accepted and rewritten to another prefix. S3 keys are literal, so the results would land under a key the host did not name. The raw path is now split as written; one trailing slash still names the same prefix. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: Hou <chaojhou@amd.com> * [RSI] Block preflight unless the framework probe exits clean (#2099) * [RSI] Block preflight when the framework probe finds no usable GPU * Move preflight device-init detection to cli/device_init * Block preflight on any probe without a ROCm verdict * ci(code-metrics): tighten function length and nesting rules, require a reason on broad noqa (#2127) * ci(code-metrics): tighten function length and nesting, require a reason on broad noqa - Function length counts the lines of code in the body (no def lines, decorators, blank, comment or docstring lines); the limit drops from 80 to 50. - Nested block depth limit drops from 5 to 4. - Enable ruff B006 (mutable default argument) and B012 (jump in finally). - An added '# noqa: BLE001' must give a reason on its line. - Teach the gate a directory-size metric (entries directly in a directory); it stays pending until its threshold is added. - Re-seed the function-lines and nested-blocks baseline entries. * refactor(code-metrics): keep the A10 comparison walk within four nested blocks The gate scripts carry no baseline entries; split the match-case and compare branches of _comparisons into their own generators. * docs(agents): two review triggers remain once function length is gated --------- Co-authored-by: chaojhou <chaojhou@amd.com> * fix(prelude): commit a promoted warm replay's overlays on the framework tree (#2122) * fix(prelude): commit a promoted warm replay's overlays on the framework tree The overlays were left as uncommitted worktree changes, so every integrate_patch candidate stashed them before it applied and was benched on the stock tree. Checkout promotion now commits them on each git root; a commit that cannot be made fails the promotion, and a lift that refuses the replay moves HEAD back before the rollback restores the tree. Refs #2120 * fix(prelude): undo the promotion commit in every warm replay rollback The commit was undone only when the lift refused the replay. A commit failing on a later tree left the earlier trees committed, and a session killed after the commit resumed with a pending record still naming the pre-patch sha; both rollbacks then refused on head_mismatch. The pending record now says the overlays are being committed, persisted before the commit, and rollback_combined_warm moves a tree back to its pre_sha before it restores the files. * fix(prelude): undo the promotion commit on a tree the replay found already holding the overlays rollback_combined_warm skips a tree the replay did not write to, so a present_in_dirty_worktree overlay stayed committed after a refused replay. The undo now runs before that skip and resets the index too, leaving the overlays uncommitted as they were before the promotion. --------- Co-authored-by: BaoYunkai <269084791+BaoYunkai@users.noreply.github.com> * refactor(preflight): delete the framework-deps installer and preflight's unreachable branches (#2128) * refactor(framework-agent): drop unused __version__ constant Nothing reads the package-level version string. Co-Authored-By: Claude <noreply@anthropic.com> * refactor(preflight): remove the per-framework dependency-manifest installer assets/framework_deps/ no longer exists, so the installer never had a manifest to act on. Drops framework_deps.py, the preflight framework_deps step, the install.sh pass and its tests and doc entries. Co-Authored-By: Claude <noreply@anthropic.com> * refactor(preflight): drop the unreachable no-LLM-endpoint branch of _export_llm_endpoints _validate_credentials exits before endpoint export unless a base URL resolves, so resolved_urls is never None. Narrow it to tuple[str, str] across preflight and the model-resolution helpers and remove the dead None guards. Co-Authored-By: Claude <noreply@anthropic.com> * refactor(preflight): make args required in _preflight and drop the recipe_kb_enabled copy _preflight and its helpers took an optional args only for tests; the sole production caller passes a Namespace, so the args-is-None branches and the IR-3 args_unavailable skip are gone. recipe_kb_enabled duplicated not args.degraded_kb and is derived at its one reader instead. Co-Authored-By: Claude <noreply@anthropic.com> * refactor(preflight): share the rocm-smi GPU probe with detect_gpu_count device_init._rocm_smi_visible_gpus carried a line-for-line copy of detect_gpu_count's rocm-smi call and GPU[ dedup parse. visible_devices now owns the probe, keeping "could not probe" apart from "zero GPUs", and both use it. Co-Authored-By: Claude <noreply@anthropic.com> * chore(metrics): ratchet the code-metrics baseline for the preflight cleanup Lower the preflight.py `_preflight` function-length and module-length entries to match the tree after the cleanup. Co-Authored-By: Claude <noreply@anthropic.com> * refactor(preflight): derive the diagnostics KB status from kb_degraded_reason The IR-3 step sets kb_degraded_reason before the diagnostics step runs, so the diagnostics no longer re-read degraded_kb. Also pin the skipped install-step contract of _check_gpu_visibility when rocm-smi cannot be probed, and restore a docstring in the auth-override fixture. Co-Authored-By: Claude <noreply@anthropic.com> * refactor(preflight): replace the RocmSmiProbe result type with a raising rocm_smi_gpu_count The probe now returns a plain count and raises the standard subprocess/OS errors; device_init and detect_gpu_count map them themselves, so the dataclass, its Literal and their exports go. Drop the probe tests that duplicate detect_gpu_count and preflight coverage, the tautological degraded_kb asserts, the inaccurate "falls back to the other side" docstring, and the now-false "no LLM base URL resolved" diagnostics text. Co-Authored-By: Claude <noreply@anthropic.com> --------- Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: Hou <chaojhou@amd.com> * Delete .github/workflows/auto-label.yml (#2137) * fix(grid): a profiled AgentX round that Magpie completed is a succeeded round of run_grid (#2139) run_grid counted a round a success only when its measurement was valid. Magpie marks a profiled AgentX replay diagnostic_only and not publishable, so such a round was a failed variant (benchmark_report_invalid_metric) even though Magpie reported the profile capture complete. run_grid now asks round_succeeded: for a native AgentX round it is Magpie's own verdict (success, Magpie-owned, valid or diagnostic), for any other round it is valid_measurement. ProfileExecutor already used that predicate inline; it now shares the one in benchmark_result. A diagnostic round stays diagnostic_only and not benchmark_valid, so it is no KEEP evidence. Co-authored-by: Claude Sonnet 5.5 <noreply@anthropic.com> Co-authored-by: chaojhou <chaojhou@amd.com> * ci(code-metrics): gate directory size at 50 entries (#2136) * ci(code-metrics): gate directory size at 50 entries Directory size counts the production modules directly in a directory and its subdirectories that hold one. Baseline the two directories already over. * chore(code-metrics): drop function-args from PENDING Its threshold has been in pyproject.toml since the parameter-count gate landed, so the name should have left PENDING then; a config without it is now refused. * fix: parse every diff header shape, reject malformed HYPERLOOM_MN_* settings, restore kept patches on resume (#2130) * fix(common): decode quoted diff paths as UTF-8 and parse every header shape parse_patch_manifest read git's octal path escapes as Latin-1 characters ("b/caf\303\251.py" became "b/café.py"), rejected the header-only deletion of a binary or empty file (it has no --- line), rejected header-only binary and mode-only sections whose names are quoted or carry no a/ b/ prefix, and split non-git diffs at hunk lines such as "--- old comment", adding a write target the patch never makes. The no-git applier, the kernel snapshot apply and snapshot materialization all take their targets from this manifest, so each of these refused or misdirected a valid patch. Quoted names are now C-unquoted to bytes and read as strict UTF-8, the two diff --git names are split the way git apply splits them, a header-only deletion takes its path from that line, and non-git sections are walked by their hunk line counts. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(multi-node): read HYPERLOOM_MN_POLL_TIMEOUT_S through one strict resolver Three readers applied three rules to the same variable: the multi-node CLI warned and fell back to 110s on a malformed value, optimize's rayjob adoption crashed on it (and on a whitespace-only value) with a bare int() ValueError, and the restart path read it with env_int and then took max() with the CLI's lenient read, which was never decisive. resolve_poll_timeout_s(default) is now the only reader: blank means unset, the value is floored at one second, and a malformed value raises EnvValueError naming the variable. The multi-node CLI reports that as a config error, not as a transient failure a re-run could fix. Observable effect: valid values behave as before. A malformed HYPERLOOM_MN_POLL_TIMEOUT_S now fails a multi-node CLI subcommand with exit 3 and optimize's cluster adoption with an EnvValueError naming the variable, instead of a warning plus a 110s budget or an unexplained int() error; a whitespace-only value counts as unset. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(executors): validate restart settings before the multi-node restart starts Every numeric HYPERLOOM_MN_* setting a per-round restart reads is now resolved once, up front, into RestartSettings. A malformed one used to surface wherever it was first read: the poll timeout and the health waits raised a bare EnvValueError past the restart's callers, which handle only ServerRestartFailed, and the health-poll, worker and probe settings were read inside the restart attempt's broad except, so each attempt killed and relaunched the server, swept its logs and reclaimed VRAM before failing with "post-launch /health wait raised". The restart now raises ServerRestartFailed("invalid multi-node restart setting: ...") before the TraceLens fan-out or any kill, which every caller already handles. A blank HYPERLOOM_MN_HEALTH_WAIT_S now counts as unset for the aiter wait widening, as it already did for the wait itself. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(multi-node): read the serving-probe settings like the restart path The resume probe in restart-server and the post-restart wait read HYPERLOOM_MN_COMPLETION_PROBE_TOKENS and _MIN_TOKENS so that they agree on what "serving" means, but the probe used its own reader, which turned a malformed value into the default. It now reads both with env_int before its first request, outside the except that maps a ValueError to "not serving", so a malformed value raises EnvValueError naming the variable: the multi-node CLI exits 3, and a per-round restart has already rejected it up front. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(kernel-agent): back every kernel-patch apply up under the session patches dir The apply backup root came from the payload when one was given, so a kernel patch's manifest could land anywhere. Every apply now backs up under <session>/patches/<kernel_id>/backup/, the one place the multi-node adoption looks for kept patches, and the policy gate no longer lists backup_root as a payload path. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(multi-node): restore kept kernel patches on the pods when adopting a cluster A pod created after a KEEP starts from the image's sources. The old replay scanned kernel-agent-workspace for manifests with status=applied, a directory and a status no kept patch ever has, so it never ran. Adopting a cluster now sends every file a finalized patch changed, as the sandbox holds it, to every pod, carrying the AITER JIT invalidation once per build dir, and stops the run when a file cannot be restored. A cluster without server control (benchmark-only) is left as it is, since no restart would load the files. Co-authored-by: Cursor <cursoragent@cursor.com> * docs(kernel): describe the kept-patch restore on cluster adoption Co-authored-by: Cursor <cursoragent@cursor.com> * chore(code-metrics): lower the duplicated-lines entries the removed replay test leaves behind test_multi_node_patch_replay_skip_and_failure_paths was a copy shared by test_common_utils.py and test_coverage_gap_units.py; deleting it drops their duplicated lines to 785 and 581, the values CI measured. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(multi-node): restore kept kernel patches in the resume pass, after the kept source layers The restore ran when the optimize CLI adopted the cluster, which made hyperloom.inference_optimizer.cli import the orchestrator: an upward import the layer contract forbids. It now runs in the Coordinator's resume pass, right after the kept source layers are back on this host's tree, so the pods get the kept files from a tree that holds them. A kept file that cannot be read or sent halts the resume with patch_recovery_incomplete, as a source layer that cannot be reinstalled does. A run that is not multi-node, or a cluster without server control, sends nothing. external_has_server_control is exported from hyperloom.inference_optimizer.multi_node so the orchestrator reaches it without the package's _internal modules. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(kernel): let a failed pod backup cleanup leave the resume running The restore's cleanup dispatch raises TimeoutExpired when one pod stalls, and nothing caught it, so a resume whose kept kernel files had all been sent died while deleting their pod-side backups. A raised cleanup is now treated like a reported one: a warning, and the resume continues. The kernel tool's docstring now names the resume pass, which replaced the adoption replay, as the reader of the backups. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(kernel): skip a kept kernel file a later KEEP deleted on resume A file a later snapshot KEEP deleted is absent from this host, and the restore stopped the resume because a kept file was missing, so every resume of such a session ended as patch_recovery_incomplete. This host's tree is the reference and the pods keep their copy after that KEEP itself, so the file is now not sent. A JIT build dir is now carried only by a file that is sent, so another KEEP's file carries it when the first carrier was deleted. Co-authored-by: Cursor <cursoragent@cursor.com> * refactor(multi-node): read the published-service gate's settings in place The gate bound published_ready_s and health_poll_s to local aliases that are never rebound. Reading them from the settings, as the worker and /health waits do, keeps the function's body at its length on main under the body-line count the code-metrics gate now uses. Co-authored-by: Cursor <cursoragent@cursor.com> --------- Co-authored-by: Cursor <cursoragent@cursor.com> * chore(code-metrics): record the executors directory at 84 entries (#2148) #2130 added _multi_node_restart_settings.py to src/hyperloom/orchestrator/actions/executors and was merged right after #2136 set the directory-size threshold, with CI that predated the gate. The directory now has 84 entries against a recorded 83, and directory-entries has no merge-base exemption, so every change touching a file under it fails code-metrics on a count it did not change. Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> * fix(grid): a round reaped before its server was ready names the server log its workspace holds (#2140) The server log of a round that the session or boot limit reaped sits in the round's benchmark_* directory under the variant slot. The lookup that fills server_log_path searched the slot's warmup round only, so such a round was recorded skipped with no server_log_path, and nothing could quote the line its log ends on. The lookup now searches the benchmark_* directories of the slot as well as those of its warmup round. Co-authored-by: Claude Sonnet 5.5 <noreply@anthropic.com> Co-authored-by: chaojhou <chaojhou@amd.com> * refactor(critic, kb): remove dead Critic request kinds and unused KB plumbing (#2129) * refactor(critic): accept only coordinator_inbox requests and drop dead runtime paths CriticAgentBackend only ever writes coordinator_inbox requests. The four dialogue request kinds (critic_decision_request, kb_draft_request, kb_hint_request, objection_signal) have no producer anywhere in the repo, so remove them together with everything that existed only for them, plus unreachable branches and unread state in the runtime. Runtime changes (src/hyperloom/agents/critic/runtime/): request_models.py - Remove DECISION_REQUEST, KB_DRAFT_REQUEST, KB_HINT_REQUEST, OBJECTION_REQUEST and REQUEST_KINDS; parse_request accepts only coordinator_inbox and always requires a non-empty raw_prompt. - Remove the decision_id auto-generation for decision/objection kinds. - Remove CriticRequest.messages, .decision, .raw and CriticRequest.to_dict. - Add Proposal.from_payload, the single owner of the action_name and predicted_gain_pct extraction (previously copied in inbox_parser). decision_reviewer.py - Remove _commit_decision_request, _commit_kb_draft and _topic_for_decision; commit_review always commits a coordinator inbox. - _fetch_kb_priors looks up priors per proposal only; the trace mode is always per_proposal (an idle round records requests=[]). - Remove JudgeBundle.messages, .decision, .kb_priors_for_decision and CommitOutcome.decision_review, .notes. - DecisionReviewer.__init__ takes a required kb_writer and reuses its session_memory; the kb_client argument and in-memory fallback go. - _populate_inbox drops the kind/raw_prompt guards and the InboxParseError catch; _maybe_write_kb_for_verdict drops the unreachable RuntimeAdapterError catch. - ceiling_importance reads CRITIC_IMPORTANCE_CEILING instead of a literal 0.84. - decisions.jsonl inbox records no longer carry a duplicate inner ts. kb_writer.py - Remove write_kb_drafts and slug_for_kind. - Remove WriteContext.source_role, source_type, topic and extra_metadata; metadata writes "source_role": "critic" directly. - KBWriter requires session_memory. - Fold the three dead-letter except branches of _upsert_with_dead_letter into one: reason is validation_error for KBValidationError and transport_error otherwise; only KBTransportError trips the breaker. kb_client.py, in_memory_kb_client.py - Remove batch_insert and add_edges (Protocol, HTTP and in-memory). cli.py - replay-dead-letter replays only upsert records; batch_insert, edges/add and list branches go. - Reuse DEFAULT_TIMEOUT_MS/DEFAULT_RETRY_MAX; drop the duplicate KB_SERVICE_TOKEN and KB_DEAD_LETTER_DIR reads (owners read them). - _resolve_reviewer builds DecisionReviewer(kb_writer=...). inbox_parser.py - ParsedPrompt keeps only shared_state and proposals; drop agent_name, inbox, kb_hints_text, extras, raw_payload, both to_dict methods, _agent_from_inbox_title and the "Knowledge base hints" branch. - InboxRow becomes the private _InboxRow. intent_envelope.py - build_envelope drops its inline idle fallback (decision_reviewer owns the idle intent; an empty list now fails validation). - build_review_verdict_intent drops the verdict and target checks that _commit_coordinator_inbox already enforces; the source check stays. - DEFAULT_IDLE_BODY leaves __all__. session_memory.py - MergeResult keeps merged and missing_keys; merge_context always persists and drops its no-op second loop. - get_cached_priors drops the now= parameter. slugify.py - Remove the no-op repeated-dash fold and slugify_safe's fallback_prefix. errors.py, importance_mapping.py, category_mapping.py - Remove InboxParseError, importance_for_kb_draft and the whole category_mapping module. Callers and docs: - orchestrator/roles/critic_agent.py: drop kb_priors_for_decision from _build_kb_priors_trace and from the _reason bundle view. - agents/critic/SKILL.md: describe only the Coordinator inbox caller; drop the dialogue Mission item, the request-type table, KB drafts and needs_info mentions. - agents/critic/README.md: replace the A2A hosting guide with the CriticAgentBackend flow; drop the Bash allowlist; keep Exit codes. - Delete actions/review_decision.md, actions/draft_kb.md, actions/objection_signal.md and references/decision_review_schema.md; drop the KB draft sections of references/verdict_schema.md and references/risk_rules.md; update references/coordinator_protocol.md. - pyproject.toml: fix the critic package-data comment. - scripts/code_metrics_baseline.txt: drop the write_kb_drafts entry. Tests: delete the tests of removed behaviour; move the empty-target check to test_commit_review_empty_target_raises; pin the idle-round trace (configured=True, requests=[]); pin that a bare KBError dead-letters as transport_error without opening the breaker. Observable changes: - prepare-review/commit-review reject the four dialogue kinds (rc=2). - An idle round no longer looks up KB priors by decision topic; its kb_priors trace keeps configured=True with requests=[] and prior_count=0. - judge_bundle.json loses messages, decision and kb_priors_for_decision; the Critic prompt's bundle view loses kb_priors_for_decision and SKILL.md changes, so the prompt cache prefix is invalidated once. - emit.json loses notes; decisions.jsonl inbox records lose their duplicate inner ts. - The dead-letter queue no longer produces batch_insert.jsonl; a leftover one now fails replay as an unknown endpoint and stays queued. Co-authored-by: Cursor <cursoragent@cursor.com> * refactor(kb): drop dead framework-KB start-up code and unused KB client, Experience KB and recipe-identity surface Framework KB (src/hyperloom/agents/framework/kb.py): - Remove prepare_kb_environment, check_kb_configuration, migrate_legacy_partition_once, _copy_partition_atomically, _LEGACY_WORKSPACE_KB_DIRNAME and _REMOVED_KB_ROOT_ENV, with the logging/shutil/uuid imports they needed. The legacy <workspace>/kb migration only served pre-release alphas, and FRAMEWORK_AGENT_KB_DIR has not been read for several releases. - read_pr_ledger() takes no argument and resolves through framework_optimization_root(), the same owner the writer uses. Preflight (src/hyperloom/inference_optimizer/cli/preflight.py): - Remove the prepare_kb_environment install step and its kb_withdrawn_override/kb_enabled locals; IR-3 still owns recipe_kb_enabled and kb_degraded_reason. - .importlinter: drop the two preflight -> agents.framework.kb ignores that no longer match. Critic KB clients (src/hyperloom/agents/critic/runtime/): - in_memory_kb_client.py: remove the test-only surface (simulate_failure, _maybe_fail, _failure_queue, all_rows, reset), the never-firing scope_value_normalized warning, the metadata filter, the deleted flag on rows, and the local copies of the scope normaliser and scope key. - kb_client.py: remove KBListQuery.metadata_filter, sort_by and include_deleted; HTTPKBClient.list sends sort_by="updated_at_desc" and include_deleted=False as literals, so the request body is unchanged. - scope_builder.py: expose normalise_scope_value, scope_key and UNKNOWN_SCOPE_VALUES as the single owners; scope_cache_key builds on scope_key (keys are byte-identical). - session_memory.py: _is_missing reads UNKNOWN_SCOPE_VALUES instead of its own copy. Experience KB (src/hyperloom/inference_optimizer/experience_kb.py): - Remove _manifest_context and every manifest/architecture fallback in _context: manifest.json never carries task_config or metadata, and SharedState has no architecture, compute_partition_mode, partitions, current_best_tput or best_tput attribute. The read payload sent in production is unchanged; _current_best_throughput stays local (importing orchestrator from inference_optimizer would break the hl-top-layers contract). Remove the now-unused _first_value. - from_env relies on RemoteConfig.from_env returning None for a blank HYPERLOOM_KB_URL instead of repeating the check. - Remove ExperienceKBEvidence.warnings, which nothing reads; the reads-off state is still logged. Recipe identity: - recipe_snapshot_constants.py: remove 15 unreferenced wire constants (F_AUTHORITY, F_CONFIDENCE, F_EVIDENCE_REFS, F_PROVENANCE, F_CANONICAL_ID, F_VERSION, F_METRIC_MIN, F_METRIC_MAX, F_PV_DETAILS, AUTHORITY_EXPERIENTIAL, ORDER_BY_*, FOREGROUND_HTTP_TIMEOUT_SEC, DEFAULT_CONFIDENCE), the test-only canonical_labels with its seven F_LABEL_* keys, and the stale "single source of truth" docstring. - orchestrator/knowledge/recipe_kb/__init__.py and canonical_id.py: stop re-exporting names nobody imports through them (CANONICAL_ID_*, DEFAULT_*_SLUG, LOCK_FILENAME, canonical_labels, detect_framework_version). recipe_canonical_id stays exported, so recipe_journal and recipe_kb_t0 keep their imports. Docs and baselines: - .env.template and docs/reference/environment-variables.md: drop the FRAMEWORK_AGENT_KB_DIR and legacy-partition migration sentences. - scripts/code_metrics_baseline.txt: drop canonical_labels; lower preflight.py::_preflight to 193 and preflight.py module lines to 2637. Tests: delete the tests of removed behaviour (legacy migration, withdrawn-variable warning, start-up hook, canonical_labels, metadata filter, deleted rows, fault injection); observe the in-memory KB through list(); carry the validation-error dead-letter contract to a stub upsert; pin the HTTP list body including the sort_by and include_deleted literals; read the PR ledger through the env override; build the Experience KB read context from state fields only. Observable changes: - The SBD V6 install event no longer has a prepare_kb_environment step. Setting FRAMEWORK_AGENT_KB_DIR no longer logs a warning or marks the install event degraded; --degraded-kb is still recorded by IR-3, the manifest's kb_degraded_reason and the diagnostics kb_status. - A workspace that only ever ran pre-release alphas no longer has its <workspace>/kb/framework_optimization ledger copied to framework-kb; the FRAMEWORK phase cold-starts that ledger instead. - In-memory critic KB rows no longer carry "deleted": false, and ExperienceKBEvidence no longer carries warnings. Co-authored-by: Cursor <cursoragent@cursor.com> * refactor(experience-kb): drop the session_dir the integration no longer reads _manifest_context was the only reader of ExperienceKBIntegration.session_dir, and it went with the dead manifest fallbacks. Remove the parameter from ExperienceKBIntegration.__init__, ExperienceKBIntegration.from_env and integration_for, and from Coordinator.experience_kb and the tests that threaded it through. Nothing observable changes. Co-authored-by: Cursor <cursoragent@cursor.com> * refactor(critic): drop the request kind now that the Critic accepts one With only coordinator_inbox left, the request's "kind" carried no information but was still validated, stored and echoed. Remove it end to end: - orchestrator/roles/critic_agent.py: the request no longer carries "kind" and the judge-bundle view sent to the LLM drops it. - runtime/request_models.py: parse_request no longer reads or validates "kind" (a stray "kind" key is ignored like any other unknown key); drop CriticRequest.kind and COORDINATOR_INBOX. - runtime/decision_reviewer.py: drop JudgeBundle.kind and CommitOutcome.kind, so judge_bundle.json and emit.json no longer carry "kind". - SKILL.md: replace the "Request Type" section with a pointer to the inbox action; actions/review_coordinator_inbox.md and README.md stop describing a request kind. - scripts/code_metrics_baseline.txt: lower CriticAgentBackend._reason to 73. - Tests: drop "kind" from every request, bundle and emit fixture, the JudgeBundle construction and the view-key table; delete test_parse_non_inbox_kind_rejected. Observable changes: request.json, judge_bundle.json and emit.json no longer have a "kind" key, and the Critic prompt's judge-bundle view loses its "kind" entry, so the prompt cache prefix changes once more. Co-authored-by: Cursor <cursoragent@cursor.com> * docs(recipe-identity): drop the stale remote-URL comment recipe_snapshot_constants no longer holds any remote wire constants, so the note about there being no default KB_STORE_URL no longer describes anything in the module. Comment-only change. Co-authored-by: Cursor <cursoragent@cursor.com> --------- Co-authored-by: Cursor <cursoragent@cursor.com> Co-authored-by: Hou <chaojhou@amd.com> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> * feat(cli): select plug implementations from optimize, share entry-point discovery (#2111) * feat(plugs): add the plug kernel, built-in profiles and the events socket Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: Claude <noreply@anthropic.com> * feat(cli): select plug implementations from optimize, share entry-point discovery Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: Claude <noreply@anthropic.com> * refactor(plugs): read the entry-point name once Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: Claude <noreply@anthropic.com> * refactor(cli): keep optimize's growth-gated units no larger than main's Move the no-target-GPU warning into bootstrap and the resume flags into their own parser helper, and add the plug flags in one loop, so _run_optimize, _build_parser and the cli module are not larger than at main. Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: Claude <noreply@anthropic.com> * chore(metrics): tighten the baseline for two units this change shrank Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: Claude <noreply@anthropic.com> * Fix CodeQL findings in test_host activated test Co-Authored-By: Claude <noreply@anthropic.com> * Rename the optimize plug flag --profile to --plug-profile The flag no longer shadows the argparse abbreviation --profile -> --profile-osl, so existing invocations keep parsing as before. Adds regression tests for both. Co-Authored-By: Claude <noreply@anthropic.com> Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> * fix(cli): resolve and plan the plug manifest before any session state is written A refused plug flag, manifest variable or damaged plugs.resolved.json used to exit 2 only after the session was opened or seeded: each refused --resume-from ... --extend-hours grew the budget again, --force-resume cleared a terminal target_reached, and a fresh run left a half-seeded session. The manifest is now resolved and planned first (resolve_plug_manifest, no writes) and only bound to the session, recorded and announced afterwards (open_plug_host). Co-Authored-By: Claude <noreply@anthropic.com> * test(cli): assert the plug refusal itself, not only exit 2 Co-Authored-By: Claude <noreply@anthropic.com> * fix(kernelforge): log a failed provider load once The shared helper already logs it; the registry logged it again. Also pins that two entry points with one name are recorded as a plugin error. Co-Authored-By: Claude <noreply@anthropic.com> * docs: document the plug flags, the two manifest variables and the resume behaviour Co-Authored-By: Claude <noreply@anthropic.com> * refactor(cli): resolve_plug_manifest reads --resume-from itself; keep cli/__init__.py no longer than main Co-Authored-By: Claude <noreply@anthropic.com> * Lower code-metrics baseline entries that main improved Co-Authored-By: Claude <noreply@anthropic.com> --------- Signed-off-by: Claude <noreply@anthropic.com> Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: chaojhou <208445444+chaojhou@users.noreply.github.com> * refactor(writeback): give each promotion and recovery job its own module (#2132) `orchestrator/loop/writeback.py` had grown to 4156 lines: a hundred-method `WritebackCollaborator` plus the ~750 lines of helpers it read. The jobs it had collected -- staging a KEEP into the patch column, validating cumulative gain, charging a baseline failure, lifting a winner into current_best, promoting each of the five result kinds, pinning the launch a measurement ran under, replaying a resumed log, recovering the windows a crash left open, and auditing a resumed run -- each get a module, and the five-result dispatcher stays where it is. Every unit moves verbatim; the twelve stage modules are mixins the collaborator composes, so `coordinator.py` still constructs the same class. Co-authored-by: chenyi <test@test.com> Co-authored-by: Claude Opus 5 <noreply@anthropic.com> * set profiler config flags after probing the container. * Revert "set profiler config flags after probing the container." This reverts commit 3205af7. Magpie already probes the container for profiler support before it launches, so a second probe here is redundant, and it is the weaker of the two: it returns False for a server that lacks the fields, for a non-zero exit and for a probe that could not run at all, writes the same detailed_annotations: false in every case, records no degraded reason, and caches the verdict for the process. A probe that times out on a loaded node then costs a full profile round whose trace TraceLens cannot split, with nothing marking the round degraded. Back to the agreed shape: default true, explicit false preserved, Magpie checks runtime support. * fix(agentx): follow the patch outcome for the native annotation default Magpie defaults detailed_annotations to false and probes the server only when it is true, and that probe raises rather than degrading when the server lacks the fields. enable_profile claimed the field unconditionally, so a native profile round on a runtime the TraceLens patch did not reach ended before the server started, where the fixed-sequence path degrades instead. Both paths now read one predicate over the degraded reason patch_for_profile already writes; an explicit operator value is still preserved either way. --------- Signed-off-by: Claude <noreply@anthropic.com> Co-authored-by: Deval Shah <devashah@amd.com> Co-authored-by: Hao Li <18546749+haofrank@users.noreply.github.com> Co-authored-by: Tharun Adithya Srikrishnan <tsrikris@amd.com> Co-authored-by: Yang, Chenyi <chenyyan@amd.com> Co-authored-by: chennyiiis <chenyi@amd.com> Co-authored-by: Claude Opus 5 <noreply@anthropic.com> Co-authored-by: Test <test@test.com> Co-authored-by: xiaofei.zheng <xiaofei.zheng@amd.com> Co-authored-by: zoroyihan7 <Yihan.Wang@amd.com> Co-authored-by: Hou <chaojhou@amd.com> Co-authored-by: luochen <chenluo@amd.com> Co-authored-by: lishuoshuo-amd <shuoli@amd.com> Co-authored-by: BaoYunkai <ybao@amd.com> Co-authored-by: BaoYunkai <269084791+BaoYunkai@users.noreply.github.com> Co-authored-by: Zheng Gong <zgong@amd.com> Co-authored-by: fengshaoyi-amd <shafeng@amd.com> Co-authored-by: Cursor <cursoragent@cursor.com> Co-authored-by: meinali-566 <meili@amd.com> Co-authored-by: chaojhou <208445444+chaojhou@users.noreply.github.com>
… user (#2115) * feat(specialists): run legacy-flow specialists as a separate non-root user HYPERLOOM_SPECIALIST_USER names an account the legacy specialist launcher starts the agent CLI as, via Popen(user=, group=, extra_groups=). Unset keeps today's launch unchanged; an unusable user fails the task instead of falling back to the app user. Co-Authored-By: Claude <noreply@anthropic.com> * fix(specialists): refuse a symlink as the shared tree root and leave .git alone The group-share walk followed a symlink given as its top-level path, so a linked worktree root could change modes on its target. Refuse it, and skip .git entirely (the specialist does no git writes). Test the demoted child's umask 002 and that a plain launch keeps the app's umask. Co-Authored-By: Claude <noreply@anthropic.com> * docs(specialists): the specialist user must traverse the session directory's parents Co-Authored-By: Claude <noreply@anthropic.com> * refactor(specialists): keep the dispatcher's run() no larger than before Move the GPU/cache environment block and the specialist-user launch steps out of SpecialistSubprocessDispatcher.run into launch_env, and tighten the code-metrics baseline entries for run() and the module to the new values. Co-Authored-By: Claude <noreply@anthropic.com> * test(specialists): neutral fixture names and closed files in the demote tests Co-Authored-By: Claude <noreply@anthropic.com> * feat(specialists): app umask 002 when a specialist user is set; read-only share mode With HYPERLOOM_SPECIALIST_USER set the app's own umask becomes 0o002, so worktree files the app rebuilds on a sync stay writable by the specialist's group. share_tree(write=False) grants group read only. Co-Authored-By: Claude <noreply@anthropic.com> * docs(specialists): the app umask under HYPERLOOM_SPECIALIST_USER Co-Authored-By: Claude <noreply@anthropic.com> * fix(specialists): umask 007 instead of 002, so others get no access A restrictive host umask (e.g. 077) would otherwise have become world-readable for everything the app creates once a specialist user is set. Co-Authored-By: Claude <noreply@anthropic.com> * fix(specialists): set the app umask at launch, not in the user lookup; restore it in tests specialist_run_as() looked like a query but set the process umask, and a test that reached it leaked 0o007 into later tests of the same process. set_group_shared_umask() is now called by resolve_launch_user only once a user is named and the launch is not refused. Co-Authored-By: Claude <noreply@anthropic.com> * fix(specialists): never follow a child-planted symlink; keep git and the MCP config reachable for the demoted account - app-side reads and writes of child-writable paths (done files, patches/, the harvested patch, the Codex config, the system prompt, the archive copies) open without following symlinks (common/nofollow) - git worktree add runs under umask 022 and the worktree's gitfile and admin dir are shared read-only on every launch, so a second task's git works - the --mcp-config file is staged in the task workspace for the demoted claude child - a demote needs CAP_SETUID, CAP_SETGID and CAP_KILL (or root); documented and checked Co-Authored-By: Claude <noreply@anthropic.com> * refactor(specialists): stage the MCP config on the argv, name repeated literals, tighten the baseline Co-Authored-By: Claude <noreply@anthropic.com> * test(specialists): pin the shared git metadata, the integrator's done read and the archive copy Co-Authored-By: Claude <noreply@anthropic.com> * fix(specialists): a read-only share removes group write, so the child cannot write the git admin dir Co-Authored-By: Claude <noreply@anthropic.com> * test(run_as): explain the ignored refusal with a suppress instead of an empty except Co-Authored-By: Claude <noreply@anthropic.com> * fix(specialists): codeql alerts (suppress, owner-only default) and a non-root-safe capability test Co-Authored-By: Claude <noreply@anthropic.com> * fix(specialists): codex round r4 (claimed patch path through a link, applied-patch archive copy, writable worktrees dir) - _patch_path_within_bases refuses a claimed patch whose path passes through a symlink - the archive copy of an applied patch no longer follows its source (bounded read) - share_worktree_git removes group write from the shared <repo>/.git/worktrees directory Co-Authored-By: Claude <noreply@anthropic.com> * test(specialists): make the archive-link oracle see the copy, not the link Co-Authored-By: Claude <noreply@anthropic.com> * chore: tighten the runner.py module-lines baseline Co-Authored-By: Claude <noreply@anthropic.com> * Lower code-metrics baseline entries that main improved Co-Authored-By: Claude <noreply@anthropic.com> * fix(specialists): no app write follows a child link, a demoted child holds no capability, snapshot objects stay readable - redact_file_in_place stages its copy without following links (create fresh with O_EXCL after unlinking), as do the prompt, process.log, the runner prompt and the dsh home writes; dsh home/profile removal and the dsh session trace refuse a linked home or log. - the demoted command runs through setpriv (clear inheritable and ambient sets, no_new_privs), so a capability-only app does not hand CAP_SETUID to the child. - drop the process-wide umask 007; the snapshot repo's git runs under an explicit umask 022. Co-Authored-By: Claude <noreply@anthropic.com> * refactor: keep the code-metrics gate green for the nofollow rewrite Co-Authored-By: Claude <noreply@anthropic.com> * docs: specialist user keeps the app umask, drops the child's capabilities through setpriv Co-Authored-By: Claude <noreply@anthropic.com> * fix(specialists): the app's git never runs a program planted in the worktree; no append, chmod or log read follows a swapped link - app_git runs against the app-owned admin dir (--git-dir, ownership and back-pointer proven), with config scopes, hooks, fsmonitor, external diff and textconv switched off; harvest and the base-commit read use it. - transcript.jsonl append, share_tree chmod/chown (O_PATH fd) and the dsh session log read open without following a link. Co-Authored-By: Claude <noreply@anthropic.com> * fix(specialists): the app's git runs against the app-owned admin dir only; no append, chmod or session-log read follows a swapped link - common/git_safety.app_git / worktree_head: --git-dir is the admin dir proven to be the app's (ownership and gitdir back pointer), command-line config switches off fsmonitor, hooks, external diff and attributes files, no system or global config; the harvest and the base-commit read use it, with --no-ext-diff --no-textconv. - transcript.jsonl append, share_tree chmod/chown (O_PATH descriptor) and the dsh session-log read open without following a link. - the helpers live in fs_utils, git_safety and env_safety (the common directory may not grow). Co-Authored-By: Claude <noreply@anthropic.com> * test: import IntegrateRecoveryIncomplete from its new module (main's #2130 and #2132 collide) Co-Authored-By: Claude <noreply@anthropic.com> --------- Co-authored-by: Claude <noreply@anthropic.com>
Description: what and why
src/hyperloom/orchestrator/loop/writeback.pywas 4156 lines (itsmodule-linesbaseline entry): a hundred-method
WritebackCollaborator, plus the ~750 lines ofmodule-level helpers it read. It is the one place a settled result turns into persistent
state, and twelve separate jobs had collected inside it.
Each of those jobs now has its own module in the package:
_baseline_promotion.py_resume_recovery.py_baseline_failures.py_current_best_lift.pycurrent_best, and the stack entry it leaves_integrate_promotion.py_keep_patch_column.py_launch_identity.pycurrent_bestwas measured on, and how well it is evidenced_resume_consistency.pycurrent_best, and what it re-enqueues_resume_replay.py_profile_roofline_promotion.py_explore_promotion.py_validation_watermark.py_promote_outcome.pyThe twelve stage modules are mixins
WritebackCollaboratorcomposes, so the classcoordinator.pyconstructs keeps its one identity — andcoordinator.py, the onlyproduction importer of this module anywhere in the tree, is not touched.
writeback.pykeeps the collaborator's own shape — construction,emit_lifecycle,is_promotable_result,record_intervention_for_task, and thepromote_to_shared_statethat routes a result to its promoter — and drops from 4156 to 239 lines.
Every unit moves verbatim. An AST comparison of the 159 units (module-level functions,
classes and assignments, plus every method of
WritebackCollaborator) before and aftershows all 159 present and 157 byte-identical. The two exceptions are both forced by the
split and nothing else:
_identity_verification_statuscalled the sibling staticmethod asWritebackCollaborator._measurement_observed_server_identity(...); both methods are nowon
LaunchIdentityMixin, so the class name in that one line changes with them.logging as _loggingto keep the name out of its own way. Each newmodule takes the package's plain
log = logging.getLogger(__name__);writeback.pykeeps the old form, unedited.
One unit moves for a reason beyond size:
_lever_for_keepnames the lever a settled KEEPmoved, and
_is_patch_column_keepis written in terms of it, so it sits with the patchcolumn rather than with integrate promotion — otherwise those two modules would import each
other.
Linked issue(s): close/fix refs — none; this pays down the second-largest
module-linesbaseline entry (afterkernelforge/loop/runner.py).Tests: added/updated? commands run?
No test was added or removed, and no assertion changed. Almost nothing had to move: the
mixins keep every method on the same instance, so each
coord.writeback.<method>()andmonkeypatch.setattr(coord.writeback, …)in the suite works unchanged. What moved is theeight test files that import a module-level name —
IntegrateRecoveryIncomplete,_PromoteOutcome,_BASELINE_FINGERPRINT_KEYS,_authorable_launch_log,_is_patch_column_keep,_source_layer_handles,_parse_baseline_workload_extra— whichnow import it from the module that owns it.
One patch named
writeback.enablement_event. That is the recorder module object — thealias never mattered, the attribute is global — so the test imports the recorder directly
instead of reaching through the writeback module for it.
Size/complexity triggers crossed: split, or why the shape is right
This is the split. Every new module is under the 800-line review trigger; the largest is
_baseline_promotionat 575. Nothing is near the 1200 ceiling.The baseline loses all four of this file's entries and gains none.
python scripts/code_metrics.py --update-baselineis committed here.baseline-raiseis on this PR for exactly three units, all for the one reason the gatedocuments:
match_movesdoes not match a method moved to another class, so a re-qualifiedmethod is reported as new even at an unchanged value.
_keep_patch_column.pyAgentKeepStagingMixin._stage_agent_keep_baseline_failures.pyBaselineFailureMixin.handle_unpromotable_result_resume_recovery.pyResumeRecoveryMixin._resume_recover_pending_warm_replayEach is the same method at the same number, unedited; the entry the PR deletes and the
finding it raises are the same value. (The two function-length units are the ones ci(code-metrics): tighten function length and nesting rules, require a reason on broad noqa #2127's
tightened 50-line limit catches; on the 80-line limit this PR was opened against, only the
parameter-count unit needed a waiver.) Bringing them under the limits is a separate change.
Coverage.
writeback.pyis not in[tool.coverage.run].omitand neither is any newmodule: this code was measured before the split and still is. On
mainthe file stands at92.73% (1912 statements, 139 missed), well above the 80% diff-coverage floor a move
inherits, so no test is added to clear a gate.
Import cycles. The
looppackage carries one pre-existing cycle —conversation → coordinator → intent_router → proposals → writeback. It is unchanged inkind:
replay_for_resumeconstructs aPendingProposal, so thefrom .proposals import PendingProposaledge thatwriteback.pyheld onmainnow sits on_resume_replay.pyand that module joins the same SCC. Nothing new is introduced, andimport-linterreports 10 contracts kept, 0 broken.Diff budget:
size-exception. 8228 production Python lines, against a 1000-linebudget. The number is the size of the module being split: the diff deletes 3969 lines from
writeback.pyand the same bodies arrive in thirteen new files, verbatim. Thirteen PRswould move the same lines thirteen times and leave the file over the ceiling at every step.
What there is to review is the boundary table above and the mixin composition; the bodies
are covered by the AST comparison.
If this simplifies or refactors: which mechanism was removed, which contract was
preserved, and the tests that show it
The mechanism removed is a 4156-line module that answered to twelve owners at once, and one
cycle-in-waiting between the patch column and integrate promotion. The contract preserved is
WritebackCollaboratoritself: the same class, the same methods, the same bodies,constructed and driven exactly as
coordinator.pyalready did — that file is not in thisdiff at all.
Observable effect — none. No behaviour, interface, default, flag or artifact changes;
this is a pure move, so the release notes have nothing to carry.
Breaking changes: no.
writebackstill exportsWritebackCollaborator. The moved namesare package-internal; their in-tree users are migrated in this PR.
PR addresses single concern: yes — splitting one module along the boundaries it already
had.
Root cause is upstream: n/a.
🤖 Generated with Claude Code