feat(alignment): add cross-configuration framework#230
feat(alignment): add cross-configuration framework#230CyberSecurityErial wants to merge 8 commits into
Conversation
📝 WalkthroughWalkthroughIntroduces a versioned cross-configuration framework for strict experiment planning, semantic operator resolution, runtime materialization, paired scoring, fixed-threshold comparison, append-only artifacts, resume validation, CLI execution, and CPU smoke testing. ChangesCross-configuration alignment
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related issues
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
rl_engine/executors/stateless_executor.py (1)
259-271: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winValidate the injected operator’s exact device and dtype.
The assignment into
resultsilently casts a wrong dtype, allowing a backend to violate its declared output contract while rank validation still passes.Proposed validation
if shifted_logps.shape != shifted_labels.shape: raise ValueError( "selected_logprob_fn output shape must match selected token IDs, got " f"{tuple(shifted_logps.shape)} and {tuple(shifted_labels.shape)}" ) + if shifted_logps.device != logits.device: + raise ValueError("selected_logprob_fn output device must match logits") + if shifted_logps.dtype != output_dtype: + raise ValueError("selected_logprob_fn output dtype must match output_dtype")🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@rl_engine/executors/stateless_executor.py` around lines 259 - 271, Extend validation in the shifted_logps handling near result construction to require that the injected operator output matches the expected device and dtype exactly, in addition to the existing tensor and shape checks. Raise a clear validation error before assigning to result, preserving the existing assignment only for correctly typed and colocated tensors.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@rl_engine/alignment/cross_config/__init__.py`:
- Around line 28-40: Update the __all__ list in the cross_config package to
satisfy Ruff RUF022 by sorting the lowercase exports alphabetically: place
build_execution_plan before compare_score_artifacts, followed by load_config,
while leaving the other exports unchanged.
In `@rl_engine/alignment/cross_config/_provenance.py`:
- Around line 51-56: Update the status ordering in the precedence tuple used by
the provenance aggregation logic so MaterializationStatus.UNOBSERVABLE appears
before MaterializationStatus.FALLBACK. Preserve the remaining precedence order
and align this behavior with RuntimeTools._aggregate_status, ensuring mixed
unobservable and fallback results aggregate to UNOBSERVABLE.
In `@rl_engine/alignment/cross_config/_resume.py`:
- Around line 58-75: Update the resume validation flow to canonically validate
every required artifact before returning True, including materialized.json and
the complete comparison.json and COMPLETE contents. Compare persisted
materialization with materialization.materialized_case, reconstruct and compare
the full alignment result and completion summary, or validate hashes for all
artifacts sealed by COMPLETE. Ensure any missing, mutated, or inconsistent
artifact causes resume validation to return False.
In `@rl_engine/alignment/cross_config/comparison.py`:
- Around line 110-147: Sanitize non-finite inactive values before constructing
TokenComparisonArtifact in the comparison flow. Preserve the existing
active-value validation, then replace inactive entries in rollout_logprobs,
training_logprobs, and absolute_diff with finite zero values before persistence,
while keeping active values unchanged.
In `@rl_engine/alignment/cross_config/planner.py`:
- Around line 201-230: Add a fixed framework case-count limit to the
plan-generation logic around normalize_requested, intervention expansion, and
pairwise product generation. Check the count before appending each requested
case, stop immediately when the limit would be exceeded, and raise the planner’s
structured PLAN_TOO_LARGE error rather than allowing unbounded accumulation;
apply the same guard to both single-intervention and pairwise paths.
In `@rl_engine/alignment/cross_config/runtime.py`:
- Around line 312-333: Extend the validation around expected, descriptors, and
applications to reject every normalized path missing from descriptors,
preventing it from bypassing descriptor checks. For APPLIED applications,
require actual to equal materialized, and for non-derived knobs require
materialized to equal the normalized request. Add these failures to problems so
require_executable() cannot trust invalid adapter status.
In `@rl_engine/alignment/cross_config/schema.py`:
- Around line 525-526: Update the fixed_threshold validation in the relevant
schema class to reject all non-finite values, including NaN and positive
infinity, while retaining the existing non-negative requirement. Use an
is-finite check alongside the current comparison before raising the existing
ValueError.
In `@rl_engine/kernels/gtest/tolerance.py`:
- Around line 29-35: Update the threshold handling in the function containing
_normalize_dtype_name and load_contract so values["atol"] is converted and
validated as a real, finite, non-negative number, rejecting booleans, NaN,
infinity, negatives, and malformed entries. Convert any invalid threshold input
into ValueError while preserving the existing missing-dtype ValueError behavior.
In `@rl_engine/kernels/semantic_registry.py`:
- Around line 282-285: Update the topology matching in the strict resolution
flow around _supports so missing required topology fields do not match
accidentally. Require every explicitly declared descriptor topology key,
including world_size, tensor/context parallelism, and sharding, to be present
and matched; only allow an empty or incomplete requirement set when the
descriptor declares the wildcard "*".
In `@tests/test_cross_config_runtime.py`:
- Line 445: Update the pytest.raises match patterns at the affected tests to
escape the dots in “training.attention_backend” (and the corresponding pattern
near the second occurrence), so pytest treats the dotted path literally and
satisfies RUF043.
---
Outside diff comments:
In `@rl_engine/executors/stateless_executor.py`:
- Around line 259-271: Extend validation in the shifted_logps handling near
result construction to require that the injected operator output matches the
expected device and dtype exactly, in addition to the existing tensor and shape
checks. Raise a clear validation error before assigning to result, preserving
the existing assignment only for correctly typed and colocated tensors.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: eaaa134d-3ec1-4de6-bb80-2bb55310aa31
⛔ Files ignored due to path filters (1)
docs/assets/ws2-cross-config-before-after.pngis excluded by!**/*.png
📒 Files selected for processing (41)
.github/workflows/ci.yml.gitignoredocs/design/cross_config_implementation_report.mddocs/design/cross_config_logprob_drift_contract.mddocs/design/ws2_cross_config_logprob_drift_contract.mdexamples/cross_config_s0_cpu_smoke.jsonexamples/cross_config_s1_distributed_smoke.jsonexamples/cross_config_s2_vllm_tp_vs_fsdp.jsonexamples/cross_config_s3_qwen3_8b_tp4_cp4_bf16.jsonpyproject.tomlrl_engine/alignment/cross_config/__init__.pyrl_engine/alignment/cross_config/__main__.pyrl_engine/alignment/cross_config/_execution.pyrl_engine/alignment/cross_config/_json.pyrl_engine/alignment/cross_config/_provenance.pyrl_engine/alignment/cross_config/_resume.pyrl_engine/alignment/cross_config/artifacts.pyrl_engine/alignment/cross_config/comparison.pyrl_engine/alignment/cross_config/config.pyrl_engine/alignment/cross_config/execution_plan.pyrl_engine/alignment/cross_config/operators.pyrl_engine/alignment/cross_config/planner.pyrl_engine/alignment/cross_config/runner.pyrl_engine/alignment/cross_config/runtime.pyrl_engine/alignment/cross_config/schema.pyrl_engine/alignment/testing/__init__.pyrl_engine/alignment/testing/cpu_cross_config.pyrl_engine/alignment/testing/smoke_ops/SMOKE_OPERATORS.mdrl_engine/alignment/testing/smoke_ops/__init__.pyrl_engine/alignment/testing/smoke_ops/smoke_only_logp_offset.pyrl_engine/alignment/testing/smoke_ops/smoke_only_logp_reference.pyrl_engine/executors/stateless_executor.pyrl_engine/kernels/gtest/tolerance.pyrl_engine/kernels/registry.pyrl_engine/kernels/semantic_registry.pytests/test_cross_config_cli.pytests/test_cross_config_contract.pytests/test_cross_config_runner.pytests/test_cross_config_runtime.pytests/test_stateless_executor.pytests/test_tolerance_contract.py
💤 Files with no reviewable changes (1)
- docs/design/ws2_cross_config_logprob_drift_contract.md
35073a1 to
1c4457e
Compare
There was a problem hiding this comment.
Actionable comments posted: 9
♻️ Duplicate comments (1)
rl_engine/alignment/cross_config/_resume.py (1)
100-101: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
artifact_sha256is only checked for key coverage, never verified against artifact bytes.The marker's seal is validated for the presence of
REQUIRED_CASE_ARTIFACTSkeys, but no hash is recomputed from the files on disk. Artifacts that resume does not otherwise re-derive (notablymaterialized.json) can be mutated and still yieldTrue. Recompute SHA-256 for each sealed artifact and compare.🛡️ Proposed seal verification
+ sealed = marker["artifact_sha256"] + try: + for name, expected_digest in sealed.items(): + digest = hashlib.sha256((attempt_dir / name).read_bytes()).hexdigest() + if digest != expected_digest: + return False + except OSError: + return FalseThis is the residual part of the earlier resume-integrity discussion; the marker schema is now sealed, but the seal itself is not enforced.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@rl_engine/alignment/cross_config/_resume.py` around lines 100 - 101, Update the marker validation in the resume flow to recompute each required artifact’s SHA-256 from its on-disk bytes and compare it with marker["artifact_sha256"], rather than checking only key coverage. Ensure all REQUIRED_CASE_ARTIFACTS, including materialized.json, are verified before accepting the marker as valid.
🧹 Nitpick comments (12)
tests/test_cross_config_alignment_standard.py (1)
41-46: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valuePositional profile indexing makes these assertions fragile.
profiles[4]/profiles[-1]break silently in meaning ifRLK_ALIGNMENT_PROFILE_ORDERgains an entry. Assert againstDISTRIBUTED_ALIGNMENT_PROFILES["A4"]/["A5"](or the standard'sprofile()accessor) instead.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_cross_config_alignment_standard.py` around lines 41 - 46, Replace the positional profile lookups in the assertions with stable named access for profiles A4 and A5, using DISTRIBUTED_ALIGNMENT_PROFILES or the standard’s profile() accessor. Preserve the existing mismatched_axes assertion for A4 and production_like assertion for A5.rl_engine/alignment/cross_config/_execution.py (1)
256-268: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winMutation check raising inside
finallymasks the original scoring failure.If
scorer.scoreraises, thefinallyblock can raiseRuntimeErrorfor mutated state, discarding the real error (and the child then reports only the mutation message). Consider only enforcing the mutation check when the body completed successfully, or chaining the original exception.♻️ Suggested restructuring
try: yield evidence + except BaseException: + for module, was_training in modes: + module.training = was_training + evidence["model_modes_restored"] = True + raise + else: + for module, was_training in modes: + module.training = was_training + evidence["model_modes_restored"] = True + if snapshot is not None: + mutations = _module_state_mutations(model, snapshot) + if mutations: + raise RuntimeError( + "read-only scorer mutated model parameters/buffers: " + ", ".join(mutations) + ) + evidence["model_state_unchanged"] = True - finally: - for module, was_training in modes: - module.training = was_training - evidence["model_modes_restored"] = True - if snapshot is not None: - mutations = _module_state_mutations(model, snapshot) - if mutations: - raise RuntimeError( - "read-only scorer mutated model parameters/buffers: " + ", ".join(mutations) - ) - evidence["model_state_unchanged"] = True🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@rl_engine/alignment/cross_config/_execution.py` around lines 256 - 268, Update the cleanup logic surrounding the evidence-yielding scorer flow so the _module_state_mutations check does not replace an exception raised by scorer.score. Track whether the body completed successfully and enforce the RuntimeError only on successful completion, while preserving mode restoration and evidence updates in finally.rl_engine/alignment/cross_config/_provenance.py (1)
43-61: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low valueEmpty status list defaults to
APPLIEDhere butERRORinRuntimeTools._aggregate_status.If every application is filtered out (or the adapter returns none), this returns
APPLIED— a fail-open default that diverges from the runtime facade. Consider aligning the empty case with the facade for defense in depth.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@rl_engine/alignment/cross_config/_provenance.py` around lines 43 - 61, The status aggregation logic should default to ERROR when statuses is empty, matching RuntimeTools._aggregate_status. Update the fallback value in the next(...) expression while preserving the existing precedence ordering and filtering behavior.rl_engine/alignment/cross_config/schema.py (1)
61-62: 🗄️ Data Integrity & Integration | 🔵 Trivial | 💤 Low valueConsider
allow_nan=Falseinto_jsonfor strict-JSON parity with the artifact writer.
artifacts._canonical_jsonrefuses NaN/Infinity; this serializer silently emits non-standardNaN/Infinitytokens, so a diagnostic value that round-trips here would be rejected downstream.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@rl_engine/alignment/cross_config/schema.py` around lines 61 - 62, Update Schema.to_json to pass allow_nan=False to json.dumps, matching artifacts._canonical_json's strict JSON behavior while preserving the existing indentation, key sorting, and to_dict conversion.rl_engine/alignment/cross_config/runtime.py (1)
202-207: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low value
self.descriptors[path]can raise a rawKeyErrorfor achanged_pathsentry outside the normalized case.
_validate_application_contractonly guarantees descriptors for paths flattened fromnormalized; a straychanged_pathsvalue would surface asKeyErrorinstead ofRuntimeMaterializationError. Consider validatingcase.changed_pathsagainstself.descriptorsand raising the module error.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@rl_engine/alignment/cross_config/runtime.py` around lines 202 - 207, Update the isolation-scope preparation in _validate_application_contract to validate every case.changed_paths entry against self.descriptors before indexing it. Raise RuntimeMaterializationError for any unknown path, while preserving the existing fallback to _flatten(normalized) and lifecycle aggregation for valid paths.tests/test_tolerance_contract.py (1)
77-86: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueTest restates the implementation verbatim, so it cannot detect a canonicalization change.
Consider asserting stability against a checked-in expected digest (or at least that reordering keys yields the same fingerprint) instead of recomputing with identical
json.dumpsoptions.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_tolerance_contract.py` around lines 77 - 86, Update test_tolerance_contract_fingerprint_is_canonical_content_sha256 so it validates tolerance_contract_fingerprint() against a checked-in expected SHA-256 digest rather than recomputing the digest with the same canonicalization options. Retain the 64-character assertion and ensure the expected value reflects the current load_contract() content.rl_engine/alignment/cross_config/__main__.py (1)
39-44: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winValidate
--timeout-secondsas a positive finite float.
type=floataccepts0, negatives,nan, andinf, which silently produces a degenerate or unbounded deadline for every paired attempt.Proposed fix
+def _positive_seconds(value: str) -> float: + parsed = float(value) + if not math.isfinite(parsed) or parsed <= 0.0: + raise argparse.ArgumentTypeError("must be a positive finite number of seconds") + return parsed + + def build_parser() -> argparse.ArgumentParser: @@ run.add_argument( "--timeout-seconds", - type=float, + type=_positive_seconds, default=30.0, help="Per paired-scoring attempt deadline", )🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@rl_engine/alignment/cross_config/__main__.py` around lines 39 - 44, Update the argument definition for --timeout-seconds in the run parser to validate that the supplied value is finite and strictly greater than zero, rejecting zero, negative, NaN, and infinite values while preserving the 30.0 default.rl_engine/kernels/semantic_registry.py (1)
210-221: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
replacekeyword shadows the module-leveldataclasses.replaceimport.Harmless today, but any future use of
dataclasses.replaceinside this method would silently resolve to the bool. Considerallow_replace(oroverwrite).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@rl_engine/kernels/semantic_registry.py` around lines 210 - 221, Rename the register_backend keyword parameter replace to allow_replace (or overwrite) and update its conditional reference, avoiding shadowing the module-level dataclasses.replace import while preserving the existing replacement behavior.rl_engine/kernels/gtest/tolerance.py (1)
22-54: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winEach call re-reads and re-parses the contract from disk.
resolve_logprob_thresholdandtolerance_contract_fingerprintare invoked repeatedly per case/resume check (e.g. twice per resume validation plus the fingerprint), so this is per-attempt file I/O plus a full JSON canonicalization. Memoizing the fingerprint (and optionally the parsed contract behind a copy, sinceload_contractreturns a mutable dict) removes the hot-path cost while keeping the values fixed for a process.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@rl_engine/kernels/gtest/tolerance.py` around lines 22 - 54, The contract is reloaded and reprocessed on every call to resolve_logprob_threshold and tolerance_contract_fingerprint. Add process-local memoization for the parsed contract and deterministic fingerprint, ensuring callers cannot mutate the cached contract (use a safe copy or immutable representation) while preserving stable values throughout the process.rl_engine/alignment/cross_config/planner.py (1)
25-25: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueExport
MAX_PLAN_CASES.It is imported by tests (and is part of the observable planning contract) yet absent from
__all__.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@rl_engine/alignment/cross_config/planner.py` at line 25, Update the module’s __all__ declaration to include the existing MAX_PLAN_CASES constant, preserving its current value and behavior so tests and other consumers can import it as part of the public planning contract.tests/test_cross_config_runner.py (1)
81-84: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value2s sleep is 20x the 0.1s timeout under test.
A shorter sleep (e.g. 0.5s) still exercises the deadline while trimming suite wall time.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_cross_config_runner.py` around lines 81 - 84, Reduce the delay in SlowScorer.score from 2.0 seconds to a shorter value such as 0.5 seconds, while keeping it longer than the 0.1-second timeout so the deadline behavior remains exercised.rl_engine/alignment/cross_config/config.py (1)
194-194: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUnknown
strategysurfaces a raw enum error.
PlanningStrategy("pairwize")raisesValueError: 'pairwize' is not a valid PlanningStrategy, which is inconsistent with the explicit, path-labelled messages used everywhere else in this loader. Consider wrapping it to list the allowed values.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@rl_engine/alignment/cross_config/config.py` at line 194, Update the configuration loading path around PlanningStrategy to catch invalid strategy values and raise the loader’s established path-labelled configuration error instead of exposing the raw enum ValueError. Include the invalid value and the allowed PlanningStrategy values in the message, while preserving the default and valid-value behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@rl_engine/alignment/cross_config/__init__.py`:
- Around line 18-28: Reorder the imported names in the cross-config import block
so constants, including RLK_ALIGNMENT_PROFILE_ORDER, appear before the
AlignmentProfile and AlignmentStandard class names, while preserving isort’s
alphabetical ordering within each group.
In `@rl_engine/alignment/cross_config/_execution.py`:
- Around line 95-101: Update ChildSupervisor.__init__ to detect
torch.cuda.is_initialized() when no start_method is provided, selecting spawn or
forkserver instead of fork for CUDA-initialized runs while retaining fork as the
CPU-only default when available. Preserve explicit start_method handling and the
existing availability validation.
In `@rl_engine/alignment/cross_config/_resume.py`:
- Around line 309-315: Wrap the _comparison_diagnostics recomputation and its
surrounding tensor-validation logic in the existing fail-closed try/except
boundary so exceptions from torch.quantile or other tensor operations return
False instead of escaping. Apply the same protection to the corresponding logic
around the later range, preserving successful diagnostic results while
converting unexpected errors into a non-reusable resume decision.
- Around line 262-294: Replace the duplicated comparison and serialization logic
in the resume validation flow with the exported production helpers from
comparison.py. Reuse the shared diagnostics-key construction, masking/order and
p95/p99/KL calculations, plus _serialized_tensor, while retaining
recompute_mismatch_mask and the existing validation behavior. Remove the local
duplicate implementations in both referenced sections.
In `@rl_engine/alignment/cross_config/artifacts.py`:
- Line 508: Sort the exported names in __all__ alphabetically to satisfy Ruff
RUF022, while preserving the same three exports: ArtifactError, ArtifactStore,
and REQUIRED_CASE_ARTIFACTS.
In `@rl_engine/alignment/cross_config/planner.py`:
- Around line 565-573: Sort the __all__ entries in
rl_engine/alignment/cross_config/planner.py using isort-style ordering: place
constants before classes/functions, and order V1_KNOB_DESCRIPTORS before
V1_KNOBS.
In `@rl_engine/alignment/cross_config/standard.py`:
- Around line 8-13: Run the repository’s configured formatter and linter for
rl_engine/alignment/cross_config/standard.py, then apply and retain their
output: reorder the imports according to isort and sort the module’s __all__
entries according to Ruff. Do not make unrelated changes.
In `@rl_engine/kernels/semantic_registry.py`:
- Around line 351-352: Prevent _records from retaining every instantiated
operator indefinitely: update the _InstanceRecord storage and instantiate flow
so records do not hold strong instance references, preferably using weak
references while preserving provenance for live instances. Ensure
clear_instance_cache also releases the associated record state, including the
paths around _records initialization and the affected instantiate and
cache-clearing logic.
In `@tests/test_cross_config_alignment_standard.py`:
- Around line 6-26: Run isort on tests/test_cross_config_alignment_standard.py
and retain its import ordering changes so the pre-commit lint hook passes.
---
Duplicate comments:
In `@rl_engine/alignment/cross_config/_resume.py`:
- Around line 100-101: Update the marker validation in the resume flow to
recompute each required artifact’s SHA-256 from its on-disk bytes and compare it
with marker["artifact_sha256"], rather than checking only key coverage. Ensure
all REQUIRED_CASE_ARTIFACTS, including materialized.json, are verified before
accepting the marker as valid.
---
Nitpick comments:
In `@rl_engine/alignment/cross_config/__main__.py`:
- Around line 39-44: Update the argument definition for --timeout-seconds in the
run parser to validate that the supplied value is finite and strictly greater
than zero, rejecting zero, negative, NaN, and infinite values while preserving
the 30.0 default.
In `@rl_engine/alignment/cross_config/_execution.py`:
- Around line 256-268: Update the cleanup logic surrounding the
evidence-yielding scorer flow so the _module_state_mutations check does not
replace an exception raised by scorer.score. Track whether the body completed
successfully and enforce the RuntimeError only on successful completion, while
preserving mode restoration and evidence updates in finally.
In `@rl_engine/alignment/cross_config/_provenance.py`:
- Around line 43-61: The status aggregation logic should default to ERROR when
statuses is empty, matching RuntimeTools._aggregate_status. Update the fallback
value in the next(...) expression while preserving the existing precedence
ordering and filtering behavior.
In `@rl_engine/alignment/cross_config/config.py`:
- Line 194: Update the configuration loading path around PlanningStrategy to
catch invalid strategy values and raise the loader’s established path-labelled
configuration error instead of exposing the raw enum ValueError. Include the
invalid value and the allowed PlanningStrategy values in the message, while
preserving the default and valid-value behavior.
In `@rl_engine/alignment/cross_config/planner.py`:
- Line 25: Update the module’s __all__ declaration to include the existing
MAX_PLAN_CASES constant, preserving its current value and behavior so tests and
other consumers can import it as part of the public planning contract.
In `@rl_engine/alignment/cross_config/runtime.py`:
- Around line 202-207: Update the isolation-scope preparation in
_validate_application_contract to validate every case.changed_paths entry
against self.descriptors before indexing it. Raise RuntimeMaterializationError
for any unknown path, while preserving the existing fallback to
_flatten(normalized) and lifecycle aggregation for valid paths.
In `@rl_engine/alignment/cross_config/schema.py`:
- Around line 61-62: Update Schema.to_json to pass allow_nan=False to
json.dumps, matching artifacts._canonical_json's strict JSON behavior while
preserving the existing indentation, key sorting, and to_dict conversion.
In `@rl_engine/kernels/gtest/tolerance.py`:
- Around line 22-54: The contract is reloaded and reprocessed on every call to
resolve_logprob_threshold and tolerance_contract_fingerprint. Add process-local
memoization for the parsed contract and deterministic fingerprint, ensuring
callers cannot mutate the cached contract (use a safe copy or immutable
representation) while preserving stable values throughout the process.
In `@rl_engine/kernels/semantic_registry.py`:
- Around line 210-221: Rename the register_backend keyword parameter replace to
allow_replace (or overwrite) and update its conditional reference, avoiding
shadowing the module-level dataclasses.replace import while preserving the
existing replacement behavior.
In `@tests/test_cross_config_alignment_standard.py`:
- Around line 41-46: Replace the positional profile lookups in the assertions
with stable named access for profiles A4 and A5, using
DISTRIBUTED_ALIGNMENT_PROFILES or the standard’s profile() accessor. Preserve
the existing mismatched_axes assertion for A4 and production_like assertion for
A5.
In `@tests/test_cross_config_runner.py`:
- Around line 81-84: Reduce the delay in SlowScorer.score from 2.0 seconds to a
shorter value such as 0.5 seconds, while keeping it longer than the 0.1-second
timeout so the deadline behavior remains exercised.
In `@tests/test_tolerance_contract.py`:
- Around line 77-86: Update
test_tolerance_contract_fingerprint_is_canonical_content_sha256 so it validates
tolerance_contract_fingerprint() against a checked-in expected SHA-256 digest
rather than recomputing the digest with the same canonicalization options.
Retain the 64-character assertion and ensure the expected value reflects the
current load_contract() content.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 3864144f-a3a7-4af4-8af1-9d275c727df7
⛔ Files ignored due to path filters (1)
docs/assets/ws2-cross-config-before-after.pngis excluded by!**/*.png
📒 Files selected for processing (43)
.github/workflows/ci.yml.gitignoredocs/design/cross_config_implementation_report.mddocs/design/cross_config_logprob_drift_contract.mddocs/design/ws2_cross_config_logprob_drift_contract.mdexamples/cross_config_s0_cpu_smoke.jsonexamples/cross_config_s1_distributed_smoke.jsonexamples/cross_config_s2_vllm_tp_vs_fsdp.jsonexamples/cross_config_s3_qwen3_8b_tp4_cp4_bf16.jsonpyproject.tomlrl_engine/alignment/cross_config/__init__.pyrl_engine/alignment/cross_config/__main__.pyrl_engine/alignment/cross_config/_execution.pyrl_engine/alignment/cross_config/_json.pyrl_engine/alignment/cross_config/_provenance.pyrl_engine/alignment/cross_config/_resume.pyrl_engine/alignment/cross_config/artifacts.pyrl_engine/alignment/cross_config/comparison.pyrl_engine/alignment/cross_config/config.pyrl_engine/alignment/cross_config/execution_plan.pyrl_engine/alignment/cross_config/operators.pyrl_engine/alignment/cross_config/planner.pyrl_engine/alignment/cross_config/runner.pyrl_engine/alignment/cross_config/runtime.pyrl_engine/alignment/cross_config/schema.pyrl_engine/alignment/cross_config/standard.pyrl_engine/alignment/testing/__init__.pyrl_engine/alignment/testing/cpu_cross_config.pyrl_engine/alignment/testing/smoke_ops/SMOKE_OPERATORS.mdrl_engine/alignment/testing/smoke_ops/__init__.pyrl_engine/alignment/testing/smoke_ops/smoke_only_logp_offset.pyrl_engine/alignment/testing/smoke_ops/smoke_only_logp_reference.pyrl_engine/executors/stateless_executor.pyrl_engine/kernels/gtest/tolerance.pyrl_engine/kernels/registry.pyrl_engine/kernels/semantic_registry.pytests/test_cross_config_alignment_standard.pytests/test_cross_config_cli.pytests/test_cross_config_contract.pytests/test_cross_config_runner.pytests/test_cross_config_runtime.pytests/test_stateless_executor.pytests/test_tolerance_contract.py
💤 Files with no reviewable changes (1)
- docs/design/ws2_cross_config_logprob_drift_contract.md
🚧 Files skipped from review as they are similar to previous changes (23)
- .gitignore
- rl_engine/alignment/cross_config/_json.py
- rl_engine/alignment/testing/init.py
- examples/cross_config_s2_vllm_tp_vs_fsdp.json
- .github/workflows/ci.yml
- examples/cross_config_s3_qwen3_8b_tp4_cp4_bf16.json
- examples/cross_config_s0_cpu_smoke.json
- rl_engine/alignment/testing/smoke_ops/SMOKE_OPERATORS.md
- pyproject.toml
- tests/test_stateless_executor.py
- rl_engine/alignment/testing/smoke_ops/smoke_only_logp_reference.py
- rl_engine/alignment/testing/smoke_ops/init.py
- examples/cross_config_s1_distributed_smoke.json
- docs/design/cross_config_implementation_report.md
- rl_engine/alignment/cross_config/execution_plan.py
- rl_engine/alignment/testing/smoke_ops/smoke_only_logp_offset.py
- tests/test_cross_config_cli.py
- rl_engine/alignment/cross_config/comparison.py
- rl_engine/alignment/cross_config/operators.py
- rl_engine/executors/stateless_executor.py
- rl_engine/alignment/testing/cpu_cross_config.py
- rl_engine/alignment/cross_config/runner.py
- tests/test_cross_config_runtime.py
| from rl_engine.alignment.cross_config.standard import ( | ||
| ALIGNMENT_PROFILE_VERSION, | ||
| ALIGNMENT_STANDARD_ID, | ||
| DISTRIBUTED_ALIGNMENT_PROFILES, | ||
| AlignmentProfile, | ||
| AlignmentStandard, | ||
| RLK_ALIGNMENT_PROFILE_ORDER, | ||
| alignment_profile_fingerprint, | ||
| get_alignment_standard, | ||
| iter_alignment_profiles, | ||
| ) |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Import names inside this block aren't isort-ordered — likely the failing isort hook.
With isort's default order_by_type=True, RLK_ALIGNMENT_PROFILE_ORDER (a CONSTANT) must be grouped with the other constants before the class names.
Proposed fix
from rl_engine.alignment.cross_config.standard import (
ALIGNMENT_PROFILE_VERSION,
ALIGNMENT_STANDARD_ID,
DISTRIBUTED_ALIGNMENT_PROFILES,
+ RLK_ALIGNMENT_PROFILE_ORDER,
AlignmentProfile,
AlignmentStandard,
- RLK_ALIGNMENT_PROFILE_ORDER,
alignment_profile_fingerprint,
get_alignment_standard,
iter_alignment_profiles,
)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| from rl_engine.alignment.cross_config.standard import ( | |
| ALIGNMENT_PROFILE_VERSION, | |
| ALIGNMENT_STANDARD_ID, | |
| DISTRIBUTED_ALIGNMENT_PROFILES, | |
| AlignmentProfile, | |
| AlignmentStandard, | |
| RLK_ALIGNMENT_PROFILE_ORDER, | |
| alignment_profile_fingerprint, | |
| get_alignment_standard, | |
| iter_alignment_profiles, | |
| ) | |
| from rl_engine.alignment.cross_config.standard import ( | |
| ALIGNMENT_PROFILE_VERSION, | |
| ALIGNMENT_STANDARD_ID, | |
| DISTRIBUTED_ALIGNMENT_PROFILES, | |
| RLK_ALIGNMENT_PROFILE_ORDER, | |
| AlignmentProfile, | |
| AlignmentStandard, | |
| alignment_profile_fingerprint, | |
| get_alignment_standard, | |
| iter_alignment_profiles, | |
| ) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@rl_engine/alignment/cross_config/__init__.py` around lines 18 - 28, Reorder
the imported names in the cross-config import block so constants, including
RLK_ALIGNMENT_PROFILE_ORDER, appear before the AlignmentProfile and
AlignmentStandard class names, while preserving isort’s alphabetical ordering
within each group.
Source: Pipeline failures
| def __init__(self, start_method: Optional[str] = None): | ||
| available = mp.get_all_start_methods() | ||
| resolved = start_method or ("fork" if "fork" in available else "spawn") | ||
| if resolved not in available: | ||
| raise ValueError(f"multiprocessing start method is unavailable: {resolved}") | ||
| self.start_method = resolved | ||
| self._active_processes: list[mp.Process] = [] |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate file =="
git ls-files | rg '(^|/)rl_engine/alignment/cross_config/_execution\.py$|(^|/)ChildSupervisor|scorers|instances' || true
echo "== file outline/size =="
if git ls-files | rq -q 'rl_engine/alignment/cross_config/_execution.py' >/dev/null 2>&1; then :; else true; fi
wc -l rl_engine/alignment/cross_config/_execution.py 2>/dev/null || true
ast-grep outline rl_engine/alignment/cross_config/_execution.py 2>/dev/null || true
echo "== relevant lines =="
sed -n '1,180p' rl_engine/alignment/cross_config/_execution.py 2>/dev/null || true
echo "== references to ChildSupervisor and start_method =="
rg -n "ChildSupervisor|start_method|multiprocessing|cuda\.is_initialized|cuda.init|cuda" rl_engine -g '*.py' || trueRepository: RL-Align/RL-Kernel
Length of output: 29088
🌐 Web query:
PyTorch fork multiprocessing CUDA context child process initialization documentation
💡 Result:
PyTorch does not support the use of the fork start method when working with CUDA [1][2][3]. Attempting to initialize CUDA in a process created via fork often leads to a RuntimeError or other undefined behavior because the CUDA runtime is not fork-safe [1][2][4]. To use CUDA in subprocesses, you must use either the spawn or forkserver start methods [1][2][3]. These methods ensure that child processes start with a clean state, avoiding issues where a parent process's initialized CUDA context is inherited incorrectly by child processes (a problem often referred to as "poison fork") [1][2]. Key recommendations and best practices include: 1. Set the Start Method: You should configure the multiprocessing start method as early as possible in your script, specifically inside the if name == 'main': block to avoid configuration errors [5][4]. import torch.multiprocessing as mp mp.set_start_method('spawn', force=True) 2. Avoid Early Initialization: Do not initialize CUDA (e.g., calling torch.cuda.is_available or creating a device object) in the main process before spawning subprocesses [1][2][4]. If CUDA is initialized too early, child processes may inherit an invalid context, even if you subsequently use spawn [4]. 3. Use Proper Contexts: Instead of setting a global start method, you can also create a multiprocessing context explicitly when initializing tools like DataLoaders, which allows you to specify the start method on a per-instance basis [5][1][4]: loader = torch.utils.data.DataLoader(..., multiprocessing_context='spawn') 4. Resource Management: For sharing tensors across processes, PyTorch recommends using multiprocessing.Queue for safety [2][3]. While inheriting tensors through memory mapping can work with fork, it is complex, error-prone, and generally discouraged compared to established queue-based patterns [2][3].
Citations:
- 1: https://docs.pytorch.org/docs/main/notes/multiprocessing.html
- 2: https://docs.pytorch.org/docs/stable/notes/multiprocessing.md
- 3: https://docs.pytorch.org/docs/2.3/notes/multiprocessing.html
- 4: Cannot re-initialize CUDA in forked subprocess pytorch/pytorch#40403
- 5: Cannot re-initialize CUDA in forked subprocess pytorch/pytorch#40403
Default to a non-fork process start method when CUDA is initialized.
ChildSupervisor forks child processes that inherit the parent’s model objects; if the parent has already initialized the CUDA runtime, forked children can fail in the CUDA backend. Use spawn/forkserver by default only when torch.cuda.is_initialized(), and keep fork for CPU-only runs.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@rl_engine/alignment/cross_config/_execution.py` around lines 95 - 101, Update
ChildSupervisor.__init__ to detect torch.cuda.is_initialized() when no
start_method is provided, selecting spawn or forkserver instead of fork for
CUDA-initialized runs while retaining fork as the CPU-only default when
available. Preserve explicit start_method handling and the existing availability
validation.
| active_mask = batch.active_mask.to(device="cpu", dtype=torch.bool) | ||
| if not bool(torch.isfinite(rollout[active_mask]).all().item()) or not bool( | ||
| torch.isfinite(training[active_mask]).all().item() | ||
| ): | ||
| return False | ||
| rollout_threshold = resolve_logprob_threshold(specs["rollout"].dtype) | ||
| training_threshold = resolve_logprob_threshold(specs["training"].dtype) | ||
| if rollout_threshold != training_threshold: | ||
| return False | ||
| fixed_threshold = rollout_threshold | ||
| rollout = rollout.masked_fill(~active_mask, 0.0) | ||
| training = training.masked_fill(~active_mask, 0.0) | ||
| absolute_diff = torch.abs(training - rollout) | ||
| mismatch_mask = recompute_mismatch_mask( | ||
| rollout, | ||
| training, | ||
| active_mask, | ||
| fixed_threshold, | ||
| ) | ||
| expected_tensors = { | ||
| "rollout_logprobs": rollout, | ||
| "training_logprobs": training, | ||
| "active_mask": active_mask, | ||
| "absolute_diff": absolute_diff, | ||
| "mismatch_mask": mismatch_mask, | ||
| } | ||
| if any( | ||
| token_tensors[name].dtype != expected.dtype | ||
| or token_tensors[name].shape != expected.shape | ||
| or not torch.equal(token_tensors[name], expected) | ||
| for name, expected in expected_tensors.items() | ||
| ): | ||
| return False |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy lift
Reuse the production comparison/serialization code instead of reimplementing it here.
Diagnostics keys, masking order, p95/p99/KL formulas, and _serialized_tensor are duplicated from comparison.py (only recompute_mismatch_mask is shared). Any future change there silently makes every resume attempt fail validation. Export the diagnostics/serialization helpers from comparison.py and call them.
Also applies to: 353-390
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@rl_engine/alignment/cross_config/_resume.py` around lines 262 - 294, Replace
the duplicated comparison and serialization logic in the resume validation flow
with the exported production helpers from comparison.py. Reuse the shared
diagnostics-key construction, masking/order and p95/p99/KL calculations, plus
_serialized_tensor, while retaining recompute_mismatch_mask and the existing
validation behavior. Remove the local duplicate implementations in both
referenced sections.
| diagnostics = _comparison_diagnostics( | ||
| rollout, | ||
| training, | ||
| active_mask, | ||
| absolute_diff, | ||
| mismatch_count, | ||
| ) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Tensor math can raise out of the fail-closed boundary.
_comparison_diagnostics runs outside any try, and torch.quantile raises for inputs above its element limit (~16M), so a large completed attempt turns a "resume not reusable" decision into a hard crash in the runner. Wrap the recomputation so unexpected tensor errors return False.
Also applies to: 353-377
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@rl_engine/alignment/cross_config/_resume.py` around lines 309 - 315, Wrap the
_comparison_diagnostics recomputation and its surrounding tensor-validation
logic in the existing fail-closed try/except boundary so exceptions from
torch.quantile or other tensor operations return False instead of escaping.
Apply the same protection to the corresponding logic around the later range,
preserving successful diagnostic results while converting unexpected errors into
a non-reusable resume decision.
| os.close(descriptor) | ||
|
|
||
|
|
||
| __all__ = ["ArtifactError", "ArtifactStore", "REQUIRED_CASE_ARTIFACTS"] |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Sort __all__ to satisfy Ruff RUF022.
Proposed fix
-__all__ = ["ArtifactError", "ArtifactStore", "REQUIRED_CASE_ARTIFACTS"]
+__all__ = ["REQUIRED_CASE_ARTIFACTS", "ArtifactError", "ArtifactStore"]🧰 Tools
🪛 Ruff (0.15.21)
[warning] 508-508: __all__ is not sorted
Apply an isort-style sorting to __all__
(RUF022)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@rl_engine/alignment/cross_config/artifacts.py` at line 508, Sort the exported
names in __all__ alphabetically to satisfy Ruff RUF022, while preserving the
same three exports: ArtifactError, ArtifactStore, and REQUIRED_CASE_ARTIFACTS.
Source: Linters/SAST tools
| __all__ = [ | ||
| "ExperimentPlan", | ||
| "Planner", | ||
| "PlanningError", | ||
| "PlanningIssue", | ||
| "V1_KNOBS", | ||
| "V1_KNOB_DESCRIPTORS", | ||
| "normalize_backend_id", | ||
| ] |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Sort __all__ (RUF022) to keep lint clean.
Constants come first under isort-style ordering, and V1_KNOB_DESCRIPTORS precedes V1_KNOBS.
Proposed fix
__all__ = [
+ "V1_KNOBS",
+ "V1_KNOB_DESCRIPTORS",
"ExperimentPlan",
"Planner",
"PlanningError",
"PlanningIssue",
- "V1_KNOBS",
- "V1_KNOB_DESCRIPTORS",
"normalize_backend_id",
]🧰 Tools
🪛 Ruff (0.15.21)
[warning] 565-573: __all__ is not sorted
Apply an isort-style sorting to __all__
(RUF022)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@rl_engine/alignment/cross_config/planner.py` around lines 565 - 573, Sort the
__all__ entries in rl_engine/alignment/cross_config/planner.py using isort-style
ordering: place constants before classes/functions, and order
V1_KNOB_DESCRIPTORS before V1_KNOBS.
Source: Linters/SAST tools
| import hashlib | ||
| import json | ||
| from collections.abc import Callable, Mapping | ||
| from dataclasses import dataclass, field | ||
| from types import MappingProxyType | ||
| from typing import Any |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Resolve the reported lint failures.
CI reports that isort rewrites the imports, and Ruff reports that __all__ is unsorted. Run the repository formatter/linter and commit its output so the lint job can pass.
Also applies to: 198-208
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@rl_engine/alignment/cross_config/standard.py` around lines 8 - 13, Run the
repository’s configured formatter and linter for
rl_engine/alignment/cross_config/standard.py, then apply and retain their
output: reorder the imports according to isort and sort the module’s __all__
entries according to Ruff. Do not make unrelated changes.
Sources: Linters/SAST tools, Pipeline failures
| self._cache: dict[str, Any] = {} | ||
| self._records: dict[int, _InstanceRecord] = {} |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
_records grows unboundedly and clear_instance_cache doesn't release it.
Every instantiate call stores a strong reference to the instance in self._records, even when cache=False, and clear_instance_cache only clears self._cache. A long-lived session (e.g. many cases per process) retains every operator instance it ever built.
Proposed fix
def clear_instance_cache(self) -> None:
self._cache.clear()
+ self._records.clear()If provenance must stay queryable after a clear, consider keying _records weakly (weakref.WeakValueDictionary-style) instead of holding instance in the record.
Also applies to: 405-416, 463-464
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@rl_engine/kernels/semantic_registry.py` around lines 351 - 352, Prevent
_records from retaining every instantiated operator indefinitely: update the
_InstanceRecord storage and instantiate flow so records do not hold strong
instance references, preferably using weak references while preserving
provenance for live instances. Ensure clear_instance_cache also releases the
associated record state, including the paths around _records initialization and
the affected instantiate and cache-clearing logic.
| import hashlib | ||
| import json | ||
|
|
||
| from rl_engine.alignment.cross_config import ( | ||
| ALIGNMENT_PROFILE_VERSION, | ||
| ALIGNMENT_STANDARD_ID, | ||
| DISTRIBUTED_ALIGNMENT_PROFILES, | ||
| RLK_ALIGNMENT_PROFILE_ORDER, | ||
| alignment_profile_fingerprint, | ||
| compare_score_artifacts, | ||
| get_alignment_standard, | ||
| iter_alignment_profiles, | ||
| ) | ||
| from rl_engine.alignment.cross_config.schema import ( | ||
| RuntimeProvenance, | ||
| ScoreArtifact, | ||
| ScoreSide, | ||
| ScorerSpec, | ||
| SemanticIdentitySpec, | ||
| ) | ||
| from rl_engine.kernels.gtest.tolerance import tolerance_contract_fingerprint |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Run isort on this file to clear the linting job.
The pre-commit isort hook modified this file, failing CI.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/test_cross_config_alignment_standard.py` around lines 6 - 26, Run isort
on tests/test_cross_config_alignment_standard.py and retain its import ordering
changes so the pre-commit lint hook passes.
Source: Pipeline failures
031232d to
1c4457e
Compare
Summary
Add a compact cross-configuration alignment framework with a CPU-only executable smoke path and explicit extension boundaries for future production adapters.
Functional modules
Scope and claim boundary
The executable path validates framework plumbing with a synthetic CPU model. It does not claim production vLLM, FSDP, tensor/context-parallel, distributed, or accelerator numerical alignment. The larger named scenarios are reproducible planning inputs until production adapters expose verified injection and read-back hooks.
Planned cases:
Validation
60 passed.418 passed, 242 skipped.git diff --checkpass.The CPU suite excludes the two Triton-required collection modules; Gloo and POSIX shared-memory tests were run with host resources available.
Review order
feat(kernels): add semantic operator catalogfeat(alignment): define cross-configuration contracts and plansfeat(alignment): add runtime materialization and scoring bridgefeat(alignment): add execution and artifact primitivesfeat(alignment): add provenance-aware paired executionfeat(alignment): add CPU smoke workflow and CLItest(alignment): add focused cross-configuration coveragedocs(alignment): document cross-configuration workflowRelated: #83, #108, #111, #222.
Summary by CodeRabbit