Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,120 @@
---
date: 2026-07-25
topic: prove-roundtrip-c2-invariant
strategy_track: guide-import-fidelity
related:
- docs/brainstorms/2026-07-25-lossless-roundtrip-universal-pipeline-requirements.md
- docs/knowledgebase/product-vision.md
- STRATEGY.md
---

# Prove roundtrip C2 invariant

## Summary

Enable the excluded roundtrip test suites, fix any issues they surface, and establish a regression guard for the C2 invariant (IF → MD → IF preserves all components, instructions, and Choose trees). This proves the core product claim that instruction files are round-trippable and unblocks multi-author share/publish trust.

---

## Problem Frame

ModSync's product vision hinges on instruction files being the source of truth, with markdown guides as derived views. The C2 invariant — that an instruction file can be emitted as markdown and re-ingested without losing components, instructions, or option trees — is the technical foundation for this claim.

Today, the three strongest roundtrip test suites (`DocumentationRoundTripTests`, `MarkdownImportTests`, `MarkdownFileTests`) are excluded from compilation in `src/ModSync.Tests/ModSync.Tests.csproj:88-95`. The product documentation claims guides are "round-trippable" but no test enforces this in normal or agent runs. The 186-vs-189 component divergence between markdown-derived and TOML instruction files (documented in `src/ModSync.Tests/MarkdownTomlParityTests.cs:72-90`) is a known exception, but without active roundtrip tests, there's no way to distinguish "known exception" from "regression."

This gap blocks the strategy's multi-author share/publish track: a share link or `modsync://` handoff is worthless if the shared instruction file is lossy.

---

## Actors

- A1. Developer or CI agent running the test suite — needs roundtrip tests to pass in default flows
- A2. Mod-build author using guide ingestion/emission — needs confidence that round-tripping preserves their work

---

## Requirements

**Test re-enablement**

- R1. The `DocumentationRoundTripTests`, `MarkdownImportTests`, and `MarkdownFileTests` suites are re-included in the default test compilation (remove the `Condition` that excludes them from `ModSync.Tests.csproj`).
- R2. All re-enabled tests pass in the default test configuration. Tests that cannot pass due to known corpus divergences (e.g., the three KOTOR1 romance components) are marked with explicit skip reasons referencing the documented exception list, not blanket exclusions.

**C2 invariant regression guard**

- R3. A regression test enforces the C2 invariant: given any instruction file in the test corpus, emitting it to markdown and re-ingesting produces identical components, instructions, and Choose trees. The test runs in the default agent flow (not a scheduled or excluded set).
- R4. The C2 test uses the pinned `mod-builds` corpus (or a committed subset) so results are reproducible across agents and CI. When the corpus is absent, the test skips cleanly rather than hard-failing.

**Known divergence handling**

- R5. The three KOTOR1 romance components missing from the markdown source are carried as a documented exception list, not as test failures. The exception list is explicit, versioned, and referenced by the regression test.
- R6. Any new divergence discovered during test re-enablement is either fixed (if a parser bug) or added to the exception list with a rationale (if a genuine source divergence).

---

## Acceptance Examples

- AE1. **Covers R1, R2.** Given the default test configuration, when `dotnet test` runs, then the roundtrip test suites execute and pass (or skip with explicit exception-list reasons).
- AE2. **Covers R3.** Given the `KOTOR1_Full` instruction file, when it is emitted to markdown and re-ingested, then the resulting components and instructions are identical to the original.
- AE3. **Covers R5.** Given the KOTOR1 roundtrip test, when it encounters the three romance components, then it skips with a reason referencing the documented exception list, not a blanket "not sure if I want to support" message.

---

## Success Criteria

- The roundtrip test suites are part of the default test flow and run on every CI build.
- The C2 invariant is explicitly tested and regression-guarded against the pinned corpus.
- A new divergence is immediately visible as a test failure or a documented exception, never as silent data loss.
- The product can truthfully claim "instruction files are round-trippable" with test evidence to back it.

---

## Scope Boundaries

**In scope**

- Re-enabling the three excluded roundtrip test suites
- Fixing parser or emitter issues that prevent tests from passing
- Establishing a C2 regression guard test
- Documenting known divergences as explicit exceptions

**Deferred for later**

- The broader universal pipeline unification (CLI and GUI sharing one ingest/emit path) — covered by `2026-07-25-lossless-roundtrip-universal-pipeline-requirements.md`
- C1 (ingest completeness) and C3 (content parity) measurement — separate tracks
- Natural-language Choose/CleanList parsing — only structural constructs are required for C2
- Lossless prose reproduction (byte-identical markdown) — semantic parity is the bar

**Outside this product's identity**

- Making `mod-builds` a writable store or authoring target
- A mandatory bespoke guide markup language

---

## Key Decisions

- **Semantic C2 parity, not byte-identical.** The instruction file is ground truth; prose wording and whitespace may normalize. This matches the existing test expectations.
- **Exception list over corpus edits.** The three missing romance components are a source divergence (markdown omits them), not a loader bug. The test skips them explicitly rather than editing the read-only corpus.
- **Re-enable then fix, not rewrite.** The test suites already exist and were passing before exclusion. The priority is re-enabling them and fixing any rot, not building new test infrastructure.

---

## Dependencies / Assumptions

- The test suites were excluded for reasons that may include now-stale test failures or build configuration changes. Re-enabling may surface issues that need fixing.
- A `./mod-builds` clone or committed test subset is needed for the C2 regression test. The existing parity test already uses a committed subset, so this pattern is established.
- The `MarkdownParser`, `ModComponentSerializationService`, and `DraftInstructionService` are the code paths under test. Any fixes needed are in these services, not in new infrastructure.

---

## Outstanding Questions

### Resolve Before Planning

- None. The scope is well-defined by the existing test suites and the documented C2 invariant.

### Deferred to Planning

- [Needs research] Which specific assertions in the excluded test suites are likely to fail given current parser state? A quick scan of the test code would surface this before planning begins.
- [Needs research] Whether the excluded tests use the same `mod-builds` commit as the parity test, or if they need corpus pinning.
181 changes: 181 additions & 0 deletions docs/plans/2026-07-25-001-feat-prove-roundtrip-c2-invariant-plan.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,181 @@
---
date: 2026-07-25
type: feat
status: active
origin: docs/brainstorms/2026-07-25-prove-roundtrip-c2-invariant-requirements.md
---

# Prove roundtrip C2 invariant

## Summary

Enable the three excluded roundtrip test suites (`DocumentationRoundTripTests`, `MarkdownFileTests`, `MarkdownImportTests`), fix any issues they surface, and establish a regression guard for the C2 invariant (instruction file → markdown → instruction file preserves all components, instructions, and Choose trees).

---

## Problem Roundtrip C2 invariant

ModSync's product vision hinges on instruction files being the source of truth, with markdown guides as derived views. The C2 invariant — that an instruction file can be emitted as markdown and re-ingested without losing components, instructions, or option trees — is the technical foundation for this claim.

Today, the three strongest roundtrip test suites are excluded from compilation in `src/ModSync.Tests/ModSync.Tests.csproj:91-93`. The product documentation claims guides are "round-trippable" but no test enforces this in normal or agent runs. This gap blocks the strategy's multi-author share/publish track.

---

## Implementation Units

### U1. Re-enable excluded test suites

**Goal:** Remove the `<Compile Remove="...">` entries for the three roundtrip test suites so they compile and run.

**Requirements:** R1

**Dependencies:** None

**Files:**
- `src/ModSync.Tests/ModSync.Tests.csproj`

**Approach:**
Remove the three `<Compile Remove="...">` lines for `DocumentationRoundTripTests.cs`, `MarkdownFileTests.cs`, and `MarkdownImportTests.cs`. Keep the other exclusions (`RegexPreviewHighlightingTests.cs`, `MainWindowUITests.cs`, `CheckpointSystemIntegrationTests.cs`, `ModSyncMetadataTests.cs`, `ResourceMetadataSerializationTests.cs`) unchanged — those are excluded for different reasons.

**Test scenarios:**
- After removal, `dotnet build src/ModSync.Tests/ModSync.Tests.csproj` succeeds with no compilation errors in the re-enabled test files
- The re-enabled test classes are discoverable by the test runner

**Verification:** Build succeeds; `dotnet test --filter "FullyQualifiedName~MarkdownFileTests|FullyQualifiedName~MarkdownImportTests|FullyQualifiedName~DocumentationRoundTripTests" --list-tests` shows the test methods.

---

### U2. Fix test infrastructure dependencies

**Goal:** Ensure the re-enabled tests can run without external dependencies that may not be present (test fixture files, `mod-builds` corpus).

**Requirements:** R2, R4

**Dependencies:** U1

**Files:**
- `src/ModSync.Tests/DocumentationRoundTripTests.cs`
- `src/ModSync.Tests/MarkdownImportTests.cs`

**Approach:**
The `DocumentationRoundTripTests.Setup()` method looks for `test_modbuild_k1.md` in the test directory. If this file doesn't exist, the test fails with `Assert.Fail`. Two options:
1. Commit a small test fixture markdown file that the tests can use
2. Change the tests to skip gracefully when the fixture is missing

The `MarkdownImportTests.FullMarkdownFile_ParsesAllMods` test depends on `mod-builds/content/k1/full.md` which requires a `mod-builds` clone. This test should skip cleanly when the corpus is absent (use `[Ignore]` with a clear reason, or check for file existence and `Assert.Inconclusive`).

**Test scenarios:**
- `DocumentationRoundTripTests` passes when the test fixture file exists
- `DocumentationRoundTripTests` skips gracefully when the fixture is missing (not a hard failure)
- `MarkdownImportTests.FullMarkdownFile_ParsesAllMods` skips when `mod-builds` is absent
- All other `MarkdownImportTests` and `MarkdownFileTests` pass (they use inline test data, no external deps)

**Verification:** Run the re-enabled tests both with and without the test fixtures; confirm skips are clean, not failures.

---

### U3. Add C2 regression guard test

**Goal:** Add a dedicated test that enforces the C2 invariant: IF → MD → IF preserves components, instructions, and Choose trees.

**Requirements:** R3

**Dependencies:** U2

**Files:**
- `src/ModSync.Tests/C2RoundtripInvariantTests.cs` (new file)

**Approach:**
Create a new test class that:
1. Takes an instruction file (TOML) as input
2. Emits it to markdown using `ModComponentSerializationService.GenerateModDocumentation`
3. Re-ingests the markdown using `MarkdownParser`
4. Asserts that the re-ingested components are identical to the original: same count, same names, same GUIDs, same instructions (action type, source, destination), same Choose trees (options, branches, per-branch instructions)

The test should use inline test data (a small instruction file with a Choose component) rather than depending on external corpus. This keeps it self-contained and always runnable.

**Test scenarios:**
- Round-trip with a simple component (no options) preserves all fields
- Round-trip with a Choose component preserves the option tree structure
- Round-trip with multiple components preserves component count and order
- Round-trip with instructions containing `<<modDirectory>>` and `<<kotorDirectory>>` placeholders preserves placeholders
- Component metadata (tier, category, author, installation method) survives round-trip

**Verification:** `dotnet test --filter "FullyQualifiedName~C2RoundtripInvariantTests"` passes.

---

### U4. Document known divergences

**Goal:** Ensure the three KOTOR1 romance components missing from the markdown source are documented as explicit exceptions, not test failures.

**Requirements:** R5

**Dependencies:** U2

**Files:**
- `src/ModSync.Tests/MarkdownTomlParityTests.cs` (verify existing documentation)

**Approach:**
The existing `MarkdownTomlParityTests.Kotor1Full_SourceFiles_CurrentlyContainKnownSemanticDivergences` test already documents the 186-vs-189 divergence. Verify this test is not excluded from compilation and that its skip reason or assertion message references the exception list clearly. No new code needed — just verification.

**Test scenarios:**
- The parity test is included in the default test suite
- The parity test's skip/assertion message is clear and references the exception list

**Verification:** `dotnet test --filter "FullyQualifiedName~Kotor1Full_SourceFiles_CurrentlyContainKnownSemanticDivergences"` runs and passes/skips with a clear reason.

---

## Scope Boundaries

**In scope**
- Re-enabling the three excluded roundtrip test suites
- Fixing test infrastructure issues (missing fixtures, corpus dependencies)
- Establishing a C2 regression guard test
- Documenting known divergences as explicit exceptions

**Deferred for later**
- Broader universal pipeline unification (CLI and GUI sharing one ingest/emit path)
- C1 (ingest completeness) and C3 (content parity) measurement
- Natural-language Choose/CleanList parsing
- Lossless prose reproduction (byte-identical markdown)

**Outside this product's identity**
- Making `mod-builds` a writable store or authoring target
- A mandatory bespoke guide markup language

---

## Key Technical Decisions

- **Semantic C2 parity, not byte-identical.** The instruction file is ground truth; prose wording and whitespace may normalize. This matches existing test expectations.
- **Exception list over corpus edits.** The three missing romance components are a source divergence (markdown omits them), not a loader bug. Tests skip them explicitly rather than editing the read-only corpus.
- **Self-contained C2 test.** The new regression guard uses inline test data, not the external `mod-builds` corpus, so it always runs.

---

## Dependencies / Assumptions

- The test suites were excluded for reasons that may include now-stale test failures or build configuration changes. Re-enabling may surface issues that need fixing.
- A committed test fixture file (`test_modbuild_k1.md`) may need to be created or the tests modified to skip gracefully.
- The `MarkdownParser`, `ModComponentSerializationService`, and `DraftInstructionService` are the code paths under test.

---

## Risk Analysis

| Risk | Likelihood | Impact | Mitigation |
|------|-----------|--------|------------|
| Re-enabled tests fail due to parser regressions | Medium | Low | Fix parser issues or document as known limitations |
| Test fixture file missing | High | Low | Create fixture or modify tests to skip gracefully |
| `mod-builds` corpus not present in CI | High | Low | Tests skip cleanly when absent |
| Other excluded tests have same issues | Low | Low | Out of scope — handle separately |

---

## Verification

- `dotnet build src/ModSync.Tests/ModSync.Tests.csproj` succeeds
- `dotnet test --filter "FullyQualifiedName~MarkdownFileTests|FullyQualifiedName~MarkdownImportTests|FullyQualifiedName~DocumentationRoundTripTests|FullyQualifiedName~C2RoundtripInvariantTests"` passes
- The C2 invariant is explicitly tested and regression-guarded
6 changes: 4 additions & 2 deletions src/ModSync.Core/ModComponent.cs
Original file line number Diff line number Diff line change
Expand Up @@ -807,16 +807,18 @@ public string SerializeComponent()
return Services.ModComponentSerializationService.SerializeSingleComponentAsTomlString(this);
}

/// <param name="tomlString">The raw TOML content to deserialize.</param>
/// <param name="requireName">See <see cref="Services.ModComponentSerializationService.DeserializeComponent"/>.</param>
[CanBeNull]
public static ModComponent DeserializeTomlComponent([NotNull] string tomlString)
public static ModComponent DeserializeTomlComponent([NotNull] string tomlString, bool requireName = true)
{
if (tomlString is null)
{
throw new ArgumentNullException(nameof(tomlString));
}

// Use the unified deserialization service
IReadOnlyList<ModComponent> components = Services.ModComponentSerializationService.DeserializeModComponentFromTomlString(tomlString);
IReadOnlyList<ModComponent> components = Services.ModComponentSerializationService.DeserializeModComponentFromTomlString(tomlString, requireName);
return components?.FirstOrDefault();
}
public async Task<InstallExitCode> InstallAsync(
Expand Down
4 changes: 2 additions & 2 deletions src/ModSync.Core/Parsing/MarkdownParser.cs
Original file line number Diff line number Diff line change
Expand Up @@ -822,7 +822,7 @@ private void ParseModSyncMetadata([NotNull] ModComponent component, [NotNull] st
_logVerbose($" Detected YAML format, attempting to deserialize...");
try
{
ModComponent yamlComponent = Services.ModComponentSerializationService.DeserializeYamlComponent(metadataText);
ModComponent yamlComponent = Services.ModComponentSerializationService.DeserializeYamlComponent(metadataText, requireName: false);
if (yamlComponent != null)
{

Expand All @@ -848,7 +848,7 @@ private void ParseModSyncMetadata([NotNull] ModComponent component, [NotNull] st
tomlString = "[[thisMod]]\n" + metadataText;
}

var tomlComponent = ModComponent.DeserializeTomlComponent(tomlString);
var tomlComponent = ModComponent.DeserializeTomlComponent(tomlString, requireName: false);
if (tomlComponent != null)
{

Expand Down
Loading
Loading