[codex] Delta closure: truthful routing + portability cleanup#90
[codex] Delta closure: truthful routing + portability cleanup#90Prekzursil merged 1 commit intomainfrom
Conversation
Add SWFOC_FORCE_PROMOTED_EXTENDER override semantics in BackendRouter with default non-forced promoted routing, de-trap universal_auto backend preference, remove hardcoded user save-root metadata, and align tests/docs/TODO evidence for deterministic and tactical live validation runs. Co-authored-by: Codex <noreply@openai.com>
|
Unable to trigger custom agent "Code Reviewer". You have run out of credits 😔 |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
📝 WalkthroughWalkthroughThis pull request implements environment-variable-driven override logic for promoted extender routing, removes user-specific save root defaults from profiles for portability, adjusts the universal_auto backend preference to "auto", and updates tests and documentation to reflect the new override mechanism. Changes
Sequence DiagramsequenceDiagram
actor Env as Environment
participant Router as BackendRouter
participant Override as Override Parser
participant Candidate as Candidate Check
participant Diag as Diagnostics
Env->>Router: SWFOC_FORCE_PROMOTED_EXTENDER set/unset
Router->>Override: ResolvePromotedExtenderOverrideState()
Override->>Override: Parse env var (boolean/int/string forms)
Override-->>Router: PromotedExtenderOverrideState {Enabled, Source}
Router->>Candidate: IsPromotedExtenderActionCandidate(profile, action)
Candidate-->>Router: bool (candidate or not)
Router->>Router: Compute isPromotedExtenderAction<br/>= Candidate AND Override.Enabled
Router->>Diag: BuildDiagnostics() with override context
Diag->>Diag: Add promotedExtenderOverrideEnabled<br/>promotedExtenderOverrideSource<br/>promotedExtenderApplied
Diag-->>Router: Complete diagnostic payload
Router-->>Env: Route decision + diagnostics
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 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 |
PR Compliance Guide 🔍Below is a summary of compliance checks for this PR:
Compliance status legend🟢 - Fully Compliant🟡 - Partial Compliant 🔴 - Not Compliant ⚪ - Requires Further Human Verification 🏷️ - Compliance label |
||||||||||||||||||||||||
PR Code Suggestions ✨Explore these optional code suggestions:
|
|||||||||||||
Codacy's Analysis Summary0 new issue (≤ 0 issue) ✨ AI Reviewer: run the reviewer on demand. As new changes are pushed, run a review below. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
tests/SwfocTrainer.Tests/Profiles/ProfileMetadataPortabilityTests.cs (1)
19-19: Optional: scan subdirectories to future-proof this portability guard.Line 19 uses
SearchOption.TopDirectoryOnly; switching to recursive discovery avoids silently missing profile files if directory layout expands later.♻️ Optional refactor
- var profileFiles = Directory.GetFiles(profileDirectory, "*.json", SearchOption.TopDirectoryOnly); + var profileFiles = Directory.GetFiles(profileDirectory, "*.json", SearchOption.AllDirectories);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/SwfocTrainer.Tests/Profiles/ProfileMetadataPortabilityTests.cs` at line 19, Change the file discovery to search recursively so profile files in subfolders aren’t missed: update the Directory.GetFiles call that assigns profileFiles (currently using profileDirectory and SearchOption.TopDirectoryOnly) to use SearchOption.AllDirectories (or an equivalent recursive enumeration) so all "*.json" profile files under profileDirectory are discovered; ensure any code that expects flat results still works or is adjusted to handle nested paths.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/TEST_PLAN.md`:
- Around line 197-202: The TEST_PLAN expects summary.total=15 but the promoted
contract now records two-step set_unit_cap entries (e.g., "[1/2]" + "[2/2]")
producing an 18-row closure matrix; update the assertions/expected values that
reference summary.total=15 to summary.total=18 and adjust any companion checks
that validate matrix row counts or pass-count criteria (including the sections
that reference set_unit_cap multi-step entries) so the runbook gate counts both
steps as separate rows and will not accept incomplete promoted evidence.
In `@tests/SwfocTrainer.Tests/Runtime/BackendRouterTests.cs`:
- Around line 500-524: Tests using the PromotedExtenderOverrideScope manipulate
the process-wide env var "SWFOC_FORCE_PROMOTED_EXTENDER" and must not run in
parallel; create an xUnit collection to serialize them and apply it to the test
class: add a CollectionDefinition (e.g., name "EnvVarSerial") with
DisableParallelization = true, and annotate the test class containing
PromotedExtenderOverrideScope (or any other classes/tests that use it) with
[Collection("EnvVarSerial")], ensuring all tests that call
PromotedExtenderOverrideScope.Disable/Enable are placed in that collection to
prevent cross-test interference.
In `@TODO.md`:
- Around line 126-127: The two evidence entries that reference only
repro-bundle.json need matching markdown artifacts added: for each run ID listed
(e.g., runs/20260301-004145 and runs/20260301-004232) add a paired evidence line
pointing to the corresponding repro-bundle.md (e.g.,
TestResults/runs/<runId>/repro-bundle.md) so each JSON bundle has its paired
markdown; update the entries that currently show only `repro-bundle.json` to
include the paired `repro-bundle.md` artifact for both run IDs.
---
Nitpick comments:
In `@tests/SwfocTrainer.Tests/Profiles/ProfileMetadataPortabilityTests.cs`:
- Line 19: Change the file discovery to search recursively so profile files in
subfolders aren’t missed: update the Directory.GetFiles call that assigns
profileFiles (currently using profileDirectory and
SearchOption.TopDirectoryOnly) to use SearchOption.AllDirectories (or an
equivalent recursive enumeration) so all "*.json" profile files under
profileDirectory are discovered; ensure any code that expects flat results still
works or is adjusted to handle nested paths.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
TODO.mddocs/LIVE_VALIDATION_RUNBOOK.mddocs/TEST_PLAN.mdprofiles/default/profiles/base_sweaw.jsonprofiles/default/profiles/base_swfoc.jsonprofiles/default/profiles/universal_auto.jsonsrc/SwfocTrainer.Runtime/Services/BackendRouter.cstests/SwfocTrainer.Tests/Profiles/ProfileInheritanceTests.cstests/SwfocTrainer.Tests/Profiles/ProfileMetadataPortabilityTests.cstests/SwfocTrainer.Tests/Runtime/BackendRouterTests.cs
💤 Files with no reviewable changes (2)
- profiles/default/profiles/base_sweaw.json
- profiles/default/profiles/base_swfoc.json
| Promoted FoC actions are not forced to extender by default. | ||
| For extender-authoritative promoted matrix evidence, explicitly enable: | ||
|
|
||
| ```powershell | ||
| $env:SWFOC_FORCE_PROMOTED_EXTENDER = "1" | ||
| ``` |
There was a problem hiding this comment.
Align matrix pass-count criteria with the current promoted contract.
Line 223 still expects summary.total=15, while the companion runbook gate tracks two-step set_unit_cap entries ([1/2] + [2/2]) for an 18-row closure matrix. Please sync this section to avoid accepting incomplete promoted evidence.
Also applies to: 223-225, 236-240
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/TEST_PLAN.md` around lines 197 - 202, The TEST_PLAN expects
summary.total=15 but the promoted contract now records two-step set_unit_cap
entries (e.g., "[1/2]" + "[2/2]") producing an 18-row closure matrix; update the
assertions/expected values that reference summary.total=15 to summary.total=18
and adjust any companion checks that validate matrix row counts or pass-count
criteria (including the sections that reference set_unit_cap multi-step entries)
so the runbook gate counts both steps as separate rows and will not accept
incomplete promoted evidence.
| private sealed class PromotedExtenderOverrideScope : IDisposable | ||
| { | ||
| private const string VariableName = "SWFOC_FORCE_PROMOTED_EXTENDER"; | ||
| private readonly string? _previousValue; | ||
|
|
||
| private PromotedExtenderOverrideScope(string? value) | ||
| { | ||
| _previousValue = Environment.GetEnvironmentVariable(VariableName); | ||
| Environment.SetEnvironmentVariable(VariableName, value); | ||
| } | ||
|
|
||
| public static PromotedExtenderOverrideScope Disable() | ||
| { | ||
| return new PromotedExtenderOverrideScope(value: null); | ||
| } | ||
|
|
||
| public static PromotedExtenderOverrideScope Enable() | ||
| { | ||
| return new PromotedExtenderOverrideScope(value: "1"); | ||
| } | ||
|
|
||
| public void Dispose() | ||
| { | ||
| Environment.SetEnvironmentVariable(VariableName, _previousValue); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== xUnit parallelization configuration ==="
rg -n --type=cs 'CollectionBehavior|DisableTestParallelization|CollectionDefinition|DisableParallelization' tests
echo
echo "=== BackendRouterTests env-var mutation sites and collection attributes ==="
rg -n --type=cs -C2 'class BackendRouterTests|Collection\(|Environment\.SetEnvironmentVariable|PromotedExtenderOverrideScope' tests/SwfocTrainer.Tests/Runtime/BackendRouterTests.csRepository: Prekzursil/SWFOC-Mod-Menu
Length of output: 112
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Repository structure ==="
ls -la
echo
echo "=== Looking for BackendRouterTests.cs ==="
fd -t f "BackendRouterTests.cs" 2>/dev/null || echo "File not found with fd"
echo
echo "=== Checking tests directory ==="
ls -la tests 2>/dev/null || echo "tests directory not found"
echo
echo "=== Git files in repository ==="
git ls-files | grep -i backendrouter || echo "No BackendRouter files found"Repository: Prekzursil/SWFOC-Mod-Menu
Length of output: 3145
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== BackendRouterTests.cs - checking for Collection attributes and env-var mutations ==="
cat -n tests/SwfocTrainer.Tests/Runtime/BackendRouterTests.cs | head -100
echo
echo "=== Lines around 500-524 (PromotedExtenderOverrideScope implementation) ==="
cat -n tests/SwfocTrainer.Tests/Runtime/BackendRouterTests.cs | sed -n '495,530p'
echo
echo "=== Checking for Collection attributes anywhere in file ==="
rg -n --type=cs '\[Collection|CollectionDefinition|CollectionBehavior' tests/SwfocTrainer.Tests/Runtime/BackendRouterTests.cs || echo "No Collection attributes found"
echo
echo "=== All uses of Environment.SetEnvironmentVariable in file ==="
rg -n --type=cs 'Environment\.SetEnvironmentVariable' tests/SwfocTrainer.Tests/Runtime/BackendRouterTests.csRepository: Prekzursil/SWFOC-Mod-Menu
Length of output: 7578
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Checking for xUnit collection/parallelization settings in test project ==="
rg -n --type=cs 'CollectionBehavior|DisableParallelization|CollectionDefinition' tests/SwfocTrainer.Tests/ 2>/dev/null || echo "No parallelization settings found"
echo
echo "=== Running canonical test verification command ==="
dotnet test tests/SwfocTrainer.Tests/SwfocTrainer.Tests.csproj -c Release --no-build --filter "FullyQualifiedName!~SwfocTrainer.Tests.Profiles.Live&FullyQualifiedName!~RuntimeAttachSmokeTests" 2>&1 | tail -50Repository: Prekzursil/SWFOC-Mod-Menu
Length of output: 281
Add xUnit Collection attributes to isolate env-var tests from parallel execution.
Lines 508 and 523 mutate a process-global environment variable. Without parallelization guards, xUnit may run tests across collections concurrently, causing cross-test interference and nondeterministic outcomes.
🛠️ Required fix
+using Xunit;
+
namespace SwfocTrainer.Tests.Runtime;
+[Collection(nameof(BackendRouterEnvVarCollection))]
public sealed class BackendRouterTests
{
@@
}
+
+[CollectionDefinition(nameof(BackendRouterEnvVarCollection), DisableParallelization = true)]
+public sealed class BackendRouterEnvVarCollection
+{
+}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/SwfocTrainer.Tests/Runtime/BackendRouterTests.cs` around lines 500 -
524, Tests using the PromotedExtenderOverrideScope manipulate the process-wide
env var "SWFOC_FORCE_PROMOTED_EXTENDER" and must not run in parallel; create an
xUnit collection to serialize them and apply it to the test class: add a
CollectionDefinition (e.g., name "EnvVarSerial") with DisableParallelization =
true, and annotate the test class containing PromotedExtenderOverrideScope (or
any other classes/tests that use it) with [Collection("EnvVarSerial")], ensuring
all tests that call PromotedExtenderOverrideScope.Disable/Enable are placed in
that collection to prevent cross-test interference.
| evidence: bundle `TestResults/runs/20260301-004145/repro-bundle.json` (`classification=blocked_environment`, tactical default routing run; no swfoc process detected) | ||
| evidence: bundle `TestResults/runs/20260301-004232/repro-bundle.json` (`classification=blocked_environment`, tactical forced-override run; no swfoc process detected) |
There was a problem hiding this comment.
Add matching repro-bundle.md evidence entries.
Line 126 and Line 127 list only repro-bundle.json. For runtime/mod closure evidence in this repo, include the paired markdown artifact too for each run ID.
📎 Suggested update
evidence: bundle `TestResults/runs/20260301-004145/repro-bundle.json` (`classification=blocked_environment`, tactical default routing run; no swfoc process detected)
+ evidence: bundle `TestResults/runs/20260301-004145/repro-bundle.md` (`classification=blocked_environment`, tactical default routing run; no swfoc process detected)
evidence: bundle `TestResults/runs/20260301-004232/repro-bundle.json` (`classification=blocked_environment`, tactical forced-override run; no swfoc process detected)
+ evidence: bundle `TestResults/runs/20260301-004232/repro-bundle.md` (`classification=blocked_environment`, tactical forced-override run; no swfoc process detected)Based on learnings: Mod/runtime bugfixes must include a reproducible bundle with TestResults/runs/<runId>/repro-bundle.json and TestResults/runs/<runId>/repro-bundle.md.
📝 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.
| evidence: bundle `TestResults/runs/20260301-004145/repro-bundle.json` (`classification=blocked_environment`, tactical default routing run; no swfoc process detected) | |
| evidence: bundle `TestResults/runs/20260301-004232/repro-bundle.json` (`classification=blocked_environment`, tactical forced-override run; no swfoc process detected) | |
| evidence: bundle `TestResults/runs/20260301-004145/repro-bundle.json` (`classification=blocked_environment`, tactical default routing run; no swfoc process detected) | |
| evidence: bundle `TestResults/runs/20260301-004145/repro-bundle.md` (`classification=blocked_environment`, tactical default routing run; no swfoc process detected) | |
| evidence: bundle `TestResults/runs/20260301-004232/repro-bundle.json` (`classification=blocked_environment`, tactical forced-override run; no swfoc process detected) | |
| evidence: bundle `TestResults/runs/20260301-004232/repro-bundle.md` (`classification=blocked_environment`, tactical forced-override run; no swfoc process detected) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@TODO.md` around lines 126 - 127, The two evidence entries that reference only
repro-bundle.json need matching markdown artifacts added: for each run ID listed
(e.g., runs/20260301-004145 and runs/20260301-004232) add a paired evidence line
pointing to the corresponding repro-bundle.md (e.g.,
TestResults/runs/<runId>/repro-bundle.md) so each JSON bundle has its paired
markdown; update the entries that currently show only `repro-bundle.json` to
include the paired `repro-bundle.md` artifact for both run IDs.
|
1 participant
User description
Summary
This PR implements the delta closure wave on current
origin/mainfor truthful routing + metadata portability.Delivered:
SWFOC_FORCE_PROMOTED_EXTENDERrouting override inBackendRouter.promotedExtenderOverrideEnabledpromotedExtenderOverrideSourcepromotedExtenderApplieduniversal_autoprofile tobackendPreference: autoand cleared local promoted capability list.saveRootDefaultmetadata frombase_swfocandbase_sweaw.ProfileMetadataPortabilityTests.Why
Current main still forced promoted FoC actions into Extender through router promotion gates, and base profile metadata still contained user-specific save-root paths. This PR closes those deltas without reopening already-landed M2 scope.
Risk
risk:high(runtime routing behavior + live evidence workflow docs).Deterministic Verification
dotnet restore SwfocTrainer.sln✅dotnet build SwfocTrainer.sln -c Release --no-restore✅dotnet test tests/SwfocTrainer.Tests/SwfocTrainer.Tests.csproj -c Release --no-build --filter "FullyQualifiedName!~SwfocTrainer.Tests.Profiles.Live&FullyQualifiedName!~RuntimeAttachSmokeTests"✅ (Passed: 233, Failed: 0)Live Evidence (TACTICAL)
Default routing run:
TestResults/runs/20260301-004145/repro-bundle.jsonclassification=blocked_environment(no swfoc process detected)Forced override regression run (
SWFOC_FORCE_PROMOTED_EXTENDER=1):TestResults/runs/20260301-004232/repro-bundle.jsonclassification=blocked_environment(no swfoc process detected)Bundle schema validation:
tools/validate-repro-bundle.ps1strict validation passed for both run IDs.Reliability Evidence Contract
repro bundle json: TestResults/runs/20260301-004145/repro-bundle.json
classification: blocked_environment
PR Type
Enhancement, Tests
Description
Implement
SWFOC_FORCE_PROMOTED_EXTENDERenvironment override for promoted FoC action routingRemove hardcoded user-specific save-root paths from base profiles for portability
Update
universal_autoprofile tobackendPreference: autoand clear promoted capability listAdd comprehensive test coverage for override semantics and profile metadata portability
Diagram Walkthrough
File Walkthrough
1 files
Add promoted extender override environment variable semantics3 files
Add override scope tests and update routing behavior assertionsAdd test for universal_auto backend preference defaultAdd portability guard test for user-specific save paths3 files
Remove hardcoded user save-root default pathRemove hardcoded user save-root default pathChange backend preference to auto and clear capabilities3 files
Document delta closure wave completion with evidenceUpdate promoted action matrix evidence proceduresUpdate test expectations for promoted routing overrideSummary by CodeRabbit
Release Notes
New Features
Improvements
Documentation