diff --git a/AGENTS.md b/AGENTS.md index 5fcad0a1..1585e921 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -48,7 +48,7 @@ scripts/ agents/ # Helper scripts for agent workflows docs/ # Runbooks and documentation vendor/ # Third-party binaries -mod-builds/ # Clone here: github.com/th3w1zard1/mod-builds +mod-builds/ # Clone here: github.com/KOTOR-Community-Portal/mod-builds (dev branch) ``` ## Build @@ -176,9 +176,9 @@ Example launch (after `mod-builds` exists at repo root and template dirs are cre `./scripts/agents/launch_gui_desktop.sh --instruction-file ./mod-builds/TOMLs/KOTOR1_Full.toml --kotor-dir ./tmp/kotor_template --mod-dir ./tmp/mod_downloads` -Clone `mod-builds` at the repo root if missing: +Clone `mod-builds` at the repo root if missing (the canonical guide content lives on the `dev` branch; there is no `TOMLs/` directory in this repo - vendor a TOML instruction file separately, e.g. from the frozen `oldrepublicwizard/mod-builds` snapshot, if a launch command needs one): -`git clone https://github.com/th3w1zard1/mod-builds ./mod-builds` +`git clone -b dev https://github.com/KOTOR-Community-Portal/mod-builds ./mod-builds` Typical local desktop flow: diff --git a/docs/brainstorms/2026-07-25-lossless-roundtrip-universal-pipeline-requirements.md b/docs/brainstorms/2026-07-25-lossless-roundtrip-universal-pipeline-requirements.md new file mode 100644 index 00000000..5294cbe0 --- /dev/null +++ b/docs/brainstorms/2026-07-25-lossless-roundtrip-universal-pipeline-requirements.md @@ -0,0 +1,136 @@ +--- +date: 2026-07-25 +updated: 2026-07-30 +topic: lossless-roundtrip-universal-pipeline +--- + +# Lossless guide roundtrip + one universal pipeline — requirements + +## Summary + +Prove that ModSync can carry a `mod-builds` guide through its own instruction file and back without losing components, instructions, or option trees, using natural-language parsing as the primary (not fallback) mechanism, and route both the CLI and GUI through one shared ingest/emit path with a single options contract. Fidelity and pipeline-unity are two independent tracks that may ship separately. The `mod-builds` corpus stays strictly read-only. + +**Updated 2026-07-30** with findings from the guide repo's full commit history and prior author correspondence (see Sources / Research). These findings correct two load-bearing assumptions in the original brainstorm — see Key Decisions. + +## Problem Frame + +Guide ingestion and guide emission both ship, but neither is proven lossless and the two entry surfaces don't share one path. The markdown loader produces 186 components for KOTOR1 while a hand-built TOML translation has 189; a committed test names this a source divergence — the markdown source omits three romance components (`src/ModSync.Tests/MarkdownTomlParityTests.cs:72-90`). The strongest markdown roundtrip suites — `DocumentationRoundTripTests`, `MarkdownImportTests`, `MarkdownFileTests` — were compiled out of the build (`src/ModSync.Tests/ModSync.Tests.csproj:88-95`) as of this brainstorm's original writing; they have since been re-enabled on a separate branch, but the broader unification and fidelity work below was not part of that fix. CLI `convert`/`merge` and the GUI load/paste flows each call `MarkdownParser` and the draft service directly with different draft-on/off behavior, while the `GuideIngestService`/`GuideEmitService` ports meant to be the shared path exist only in tests. The result is two subtly divergent pipelines and a fidelity claim no test guards. + +This is the precondition for the strategy's next moves: a share link or a `modsync://` handoff is worthless if the shared instruction file is lossy. Lossless round-trip has to hold before publish/share and consume paths carry other authors' builds. + +**What the guide actually looks like, confirmed by reading the corpus's full history and the guide author's own correspondence:** the guide is, and has always been, pure human-authored prose. There is no structured `#### Instructions` / `#### Options` / `Guid:` block anywhere in its ~850-commit history — that is ModSync's own native markdown convention, never the guide's. Every install step, including Choose-style "pick one of the following" branches, is written as free-text sentences inside a single Directions/Installation-Instructions field. The guide author (in direct correspondence, see Sources) explicitly and repeatedly declined to standardize that prose beyond consistent word-choice per instruction type, calling out specific mods (HQ Blasters, redrob's texture cleanlist, a K2CP+HD-Visas nested conditional) as genuinely irreducible to a fixed formula. She was willing to standardize syntax/structure (heading levels, field order, field names) but never instruction wording. A prior effort to maintain a hand-built TOML translation alongside the guide (in a separate repository) was abandoned for exactly this reason and is now frozen. + +## Key Decisions + +- **Three named fidelity checks, and only one is "lossless."** `C1` ingest completeness: every component and instruction the guide markdown *expresses* appears in the resulting components, and unparsed prose is flagged rather than dropped. `C2` emit round-trip — the lossless loop: `IF → emitted guide → IF` preserves every component, instruction, and Choose tree, with ModSync's own instruction file as ground truth. `C3` content parity: the markdown-derived component set versus a paired full-build instruction file, reported as a diff. Because the instruction file is the source of truth and emit regenerates markdown, `MD→IF→MD` cannot measure fidelity to the source of truth — `C2` is the loop that can, and `C1`/`C3` bracket what markdown ingest and corpus authorship contribute. +- **Correction: the paired TOML is not an upstream artifact and is stale.** The original brainstorm assumed a maintained TOML lived alongside the canonical guide. It does not, and never has — `KOTOR-Community-Portal/mod-builds` (the canonical repo) has zero `.toml` files across its entire history. The `TOMLs/` directory referenced by `C3`/R4 existed only in a separate, non-canonical repository (`oldrepublicwizard/mod-builds`) that hand-translated guide content into ModSync's format, then was abandoned and has had no commits since 2025-10-31 while the canonical guide kept changing. **Any `C3` comparison against that TOML is only valid for guide content as of that date; it is not ground truth for current or future guide revisions.** `C2` (self-consistency against ModSync's own emitted instruction file) is therefore the only fidelity check with a durable source of truth — `C3` is now explicitly a historical/best-effort cross-check, not a release gate. +- **Correction: natural-language parsing is the primary lever, not a secondary one.** R5 previously assumed structured markdown (`#### Options`, Choose-as-structure) was the main round-trip mechanism and prose was the exception. Confirmed false: 0 occurrences of ModSync's structured conventions anywhere in the guide's history; Choose-style branches are always prose. The natural-language parser (`NaturalLanguageInstructionParser.cs`) is the primary mechanism for the overwhelming majority of real content, not a fallback for edge cases. +- **The guide author will not standardize instruction prose — accepted as a permanent constraint, not a temporary gap.** Confirmed directly: syntax/structure (headings, field names, field order) can be kept consistent on request; free-text instruction wording cannot and will not be, because many steps are genuinely conditional and defy a fixed formula. Any future plan must design for permanent prose variance, not assume a standardization effort will eventually close the gap. +- **Target the current guide format only.** The guide converted to Docusaurus-style admonition fences (`:::note` / `:::warning` wrapping a `: ` definition-list prose block) in commits on 2025-10-20 (K1) and 2025-10-21 (K2), replacing the older flat `**Installation Instructions:** ` inline convention. Confirmed: the current parser has no handling for the fence/definition-list wrapper. This brainstorm scopes to the current (post-Oct-2025) format; the older inline convention is out of scope unless a real guide snapshot using it is reported in the wild. +- **Named irreducible cases become explicit acceptance examples, not just documented exceptions.** HQ Blasters (an undocumented TSLPatcher-quirk workaround), redrob's cleanlist-driven conditional deletion, and the K2CP+HD-Visas nested conditional are real, well-documented, recurring corpus cases the author herself named as maximally hard. They're strong fixtures precisely because they're real, not synthetic edge cases — see AE6-AE8. +- **Reuse and extend the guide ports.** The universal path is the existing `GuideIngestService`/`GuideEmitService`, extended to carry content sections (preamble/widescreen/Aspyr), parser options/profile, parse traces, and the draft flag — a façade to widen, not a wire-only swap. +- **Draft-on-ingest is one shared option, not two hidden behaviors.** File-open defaults to no prose drafting; paste and `--parse-directions` opt in. Unification preserves this as a single explicit flag in the shared contract, resolving today's per-surface divergence. +- **The corpus source is pinned.** `KOTOR-Community-Portal/mod-builds`, `dev` branch (the active/staging branch the author edits directly), at a specific commit recorded alongside any measured parity numbers — not the frozen `oldrepublicwizard/mod-builds` TOML repo, which is not authoritative for guide content. + +## Requirements + +Priority markers: `[M]` must-have for the v1 bar, `[E]` enabler whose user-visible outcome is another requirement. + +### Roundtrip fidelity + +- R1. `[M]` The `C2` emit round-trip is lossless: `IF → emitted guide → IF` preserves every component, every instruction (action type, relative order, `<>`/`<>` placeholders), and every Choose tree with its branches and per-branch instructions. +- R2. `[M]` Semantic metadata survives `C2`: tiers, installation method, and the presence and meaning of the preamble/widescreen/Aspyr sections. Prose wording and whitespace may normalize. +- R3. `[M]` `C1` ingest completeness: every component and instruction the guide markdown expresses appears in the resulting components; nothing expressed is silently dropped. +- R4. `[E]` `C3` content parity between the markdown-derived component set and a paired full-build instruction file is reported with an explicit exception list (today: three KOTOR1 romance components absent from markdown as of the last cross-check). This check is historical/best-effort only — the paired TOML is frozen as of 2025-10-31 and is not ground truth for guide content after that date. No corpus edits, no invented components. + +### Parser coverage + +- R5. `[M]` **Natural-language prose parsing is the primary round-trip lever.** Nearly all real install constructs — including Choose-style branches — are expressed as prose, not structure, in the actual corpus. Structured markdown (`#### Options` etc.) is a ModSync-native convention the guide has never used; treat it as a lower-priority secondary path, not the primary one. +- R6. `[M]` The current admonition-fence format (`:::note` / `:::warning` wrapping a `: ` definition-list prose block, live since 2025-10-20/21) is parsed as a first-class format, not an incidental one — confirmed unhandled today. Wording and phrasing inside the fence follow the same prose parsing rules as any other Directions text. +- R7. `[M]` The draft parser recognizes the phrasing patterns the corpus actually uses, catalogued from real guide text: simple moves ("Move everything from X to your Override"), multi-folder moves, exception/exclusion clauses ("EXCEPT the files for..."), delete-before-move, range notation ("file01 through file04"), conditional clauses ("if using X, ..."), copy/rename, and folder-navigation phrasing. Overwrite-handling and file-list conjunctions ("X, Y, and Z" vs "X and Y") vary between guide sections and must both parse. +- R8. `[M]` Draft coverage is quantified against the corpus as the fraction of prose Directions that produce a correct draft instruction, with a recorded floor. Prose carried through as metadata does not count as a surviving instruction. +- R9. `[M]` Prose the parser cannot interpret surfaces as an explicit, reviewable gap on both surfaces (CLI output and GUI), where the author can see and act on it; it is never silently dropped. This is a change to the draft/ingest result contract, not a regex-only change. Per Key Decisions, this is a permanent design requirement, not a temporary gap expected to close as the guide standardizes. + +### Unified pipeline + +- R10. `[E]` CLI `convert`/`merge` and GUI file-open/paste ingest and emit guides through one shared path that fixes a single options/behavior contract (including the draft-on/off flag); neither surface calls the markdown parser or draft service directly. +- R11. `[M]` Given the same guide and the same options, CLI and GUI produce the same components and instructions. Draft-on-ingest is part of the shared contract, not a per-surface default. + +### Measurement and regression safety + +- R12. `[M]` `C1`, `C2`, and the R8 draft-rate floor are proven by tests that run in the default and agent flows — re-enabling or replacing any currently excluded suites. +- R13. `[M]` A regression guards the `C2` invariants (zero dropped or invented across `IF→MD→IF`) and the drafted-instruction-rate floor against the pinned full builds. Percentages are diagnostics; release gates on the absolute `C2` invariants, not on a non-decreasing ratchet. + +### Corpus handling + +- R14. `[M]` `mod-builds` is read-only: ingest, emit, and all tests read the corpus and write exclusively to temp or output directories. +- R15. `[E]` The corpus is pinned to `KOTOR-Community-Portal/mod-builds`, `dev` branch, at a recorded commit. Tests that require it skip cleanly when it is absent rather than hard-failing, and the flows define what "green" means without a local clone. + +## Acceptance Examples + +- AE1. **Covers R1.** Given an instruction file with a Choose component, when it is emitted and re-ingested, then the Choose tree and each branch's instructions are identical to the original. +- AE2. **Covers R3, R9.** Given a Directions line the parser cannot map to an action, when the guide is ingested, then the line is reported as an unparsed gap on both CLI and GUI, not omitted. +- AE3. **Covers R8.** Given `KOTOR1_Full`, when it is ingested, then the drafted-instruction rate meets or exceeds the recorded floor, and a component whose prose did not draft counts as zero drafted even though its prose survives as metadata. +- AE4. **Covers R11.** Given one guide loaded via CLI and GUI with drafting off, both produce identical components and instructions; with drafting on, both produce identical drafts. +- AE5. **Covers R4.** Given `KOTOR1_Full`, when the markdown-derived component set is compared to the frozen instruction-file set, then the diff equals exactly the documented exception list, with no other missing or extra components. +- AE6. **Covers R7.** Given the HQ Blasters entry's Directions (delete `keblastore.utm` from TSLPatchdata before running the patcher to force an intentional single error, then rename `w_ionrfl_04.*` files to `w_ionrfl_004.*` post-install, then delete several more files), when the guide is ingested, then either the full instruction sequence drafts correctly, or the undraftable portion surfaces as an explicit reviewable gap per R9 — it is never silently dropped or partially applied without signal. +- AE7. **Covers R7.** Given a Directions entry conditioned on redrob's cleanlist (per-mod file deletion driven by an externally maintained `cleanlist_k1.txt`, not deducible from the guide text alone), when the guide is ingested, then the conditional nature of the deletion is preserved as metadata or surfaces as an explicit gap, never silently resolved to a wrong fixed file list. +- AE8. **Covers R7.** Given the K2CP mod's Directions with a nested conditional ("delete these files before moving to override; if also using HD Visas, additionally delete these three more"), when the guide is ingested, then both the unconditional and the HD-Visas-conditional deletions are captured as distinct instructions, not merged into one unconditional step. + +## Success Criteria + +- A baseline draft-rate floor and the `C3` exception list are measured from a live corpus read (pinned commit, `KOTOR-Community-Portal/mod-builds` `dev`) and recorded in the repository before any regression ratchet takes effect. +- The `C2` lossless invariant runs green in the default and agent test flows, not only in a scheduled or excluded set. +- AE6-AE8 pass or produce an explicit, reviewable gap report — never a silent partial application. +- A reviewer can confirm CLI and GUI share the ingest/emit path and options contract from the test contract alone, without tracing runtime calls. + +## Scope Boundaries + +**Deferred for later:** + +- Byte-identical markdown reproduction (exact whitespace and prose shape). Semantic `C2` parity is the v1 bar. +- Markdown-only installs without the instruction-file merge; the two-source merge remains the installable path. +- Generalization beyond the canonical corpus. v1 measures fidelity on `KOTOR1_Full`/`KOTOR2_Full`; a second author's guide importing cleanly is deferred, not assumed. +- Parsing the pre-2025-10-20 bold-inline-field guide format. Confirmed obsolete on the canonical `dev` branch; revisit only if a real snapshot using it is reported. +- Further validation/install pipeline unification, which shipped separately. + +**Outside this product's identity:** + +- Turning `mod-builds` into a writable store or authoring target. It is an external corpus ModSync consumes. +- A mandatory bespoke guide markup language. The guide author has explicitly and repeatedly declined to standardize instruction prose; optional authoring conventions she voluntarily adopts to disambiguate a section are not ruled out, but are not to be requested or assumed. + +## Dependencies / Assumptions + +- A `./mod-builds` clone (pinned to `KOTOR-Community-Portal/mod-builds`, `dev` branch) must be present for fidelity work and roundtrip tests. +- ModSync's own emitted instruction file is the only durable source of truth for `C2`. The historical hand-built TOML (`oldrepublicwizard/mod-builds`, frozen 2025-10-31) is a useful one-time cross-check for `C3` but is not authoritative and will drift further from the canonical guide over time. +- Per the committed parity test, the three missing KOTOR1 romance components are a known source divergence — the markdown source lacks them — not a loader bug, as of the last cross-check against the frozen TOML. +- The guide author has confirmed she will keep syntax/structure consistent on request (heading levels, field names, field order) but will not standardize instruction wording; permanent prose variance is a design constraint, not a temporary gap. + +## Outstanding Questions + +**Resolved by this update:** + +- ~~Which upstream is canonical?~~ Confirmed: `KOTOR-Community-Portal/mod-builds`, `dev` branch is canonical and active; `main` is the released-stable snapshot. `oldrepublicwizard/mod-builds` (the TOML translation repo) is a separate, non-canonical, now-abandoned project — not an authoritative source for guide content. +- ~~Where do Choose and CleanList appear in the corpus — as structure or as prose?~~ Confirmed: always prose. No structured Choose/Options block has ever appeared in the guide's history. + +**Resolve before planning:** + +- Does the master goal's "back to md without discrepancies" accept semantic `C2` parity with normalized prose and whitespace, or require an author-visible discrepancy budget? The latter materially expands scope. + +**Deferred to planning:** + +- The exact shape of the shared options contract and the gap-report surface on each of CLI and GUI. +- Whether AE6-AE8 are pursued as fully automated drafting, or as "detect and flag reviewable gap" only — the guide author's own position (see Dependencies) is that some of these may never be fully automatable and per-mod special-casing will remain necessary indefinitely. + +## Sources / Research + +- `src/ModSync.Tests/MarkdownTomlParityTests.cs:72-90` — `Kotor1Full_SourceFiles_CurrentlyContainKnownSemanticDivergences`: 186/189 and the three source-missing romance components. +- `src/ModSync.Tests/ModSync.Tests.csproj` — `DocumentationRoundTripTests`, `MarkdownImportTests`, `MarkdownFileTests` compile-exclusion status at time of original writing (since re-enabled on a separate branch). +- `src/ModSync.Core/Parsing/NaturalLanguageInstructionParser.cs` — supported prose action patterns; no Choose/CleanList drafting yet. +- `src/ModSync.Core/Parsing/MarkdownParser.cs` — structured `#### Options` parsing into `component.Options` (confirmed unused by the real corpus); no handling for the `:::note`/`:::warning` admonition-fence format the guide has used since 2025-10-20. +- `src/ModSync.Core/Ports/Guides/IGuideServices.cs`, `src/ModSync.Core/Ports/Guides/GuideServices.cs` — the intended shared ingest/emit ports (used only by tests today). +- `src/ModSync.Core/CLI/ModBuildConverter.cs` — CLI ingest calling `MarkdownParser`/`DraftInstructionService` directly. +- `docs/brainstorms/2026-05-29-mod-builds-pipeline-requirements.md` — a completed foundation this bet builds on. +- `STRATEGY.md` — guide ingestion and guide emission tracks; guide-import-fidelity metric. +- **Repo history survey (2026-07-30):** `KOTOR-Community-Portal/mod-builds`, `dev` branch, 850 commits. Format-era boundary confirmed at commit `f1c5ca546a94458873805a9959042a12020d1a4f` (2025-10-20, K1) and `2871947f421d4b9f295efeae6a3b73cff44d51c9` (2025-10-21, K2) — conversion to `:::note`/`:::warning` admonition fences. Zero occurrences of `#### Instructions`, `Guid:`, or structured Options blocks across all history. `oldrepublicwizard/mod-builds` (formerly `th3w1zard1/mod-builds`) confirmed as a separate, non-fork, TOML-only repository, last commit 2025-10-31, since abandoned. +- **Prior author correspondence (Discord DM export, 2023-05 through 2026, reviewed 2026-07-30):** guide field schema (Name/Description/Directions/Dependencies/Restrictions/InstallAfter/InstallBefore/Options), the author's explicit refusal to standardize instruction prose beyond syntax/structure, the HQ Blasters/redrob-cleanlist/K2CP+HD-Visas cases named directly as maximally hard, and the TOML-translation effort's abandonment and removal from active coordination. diff --git a/docs/plans/2026-07-30-001-feat-lossless-roundtrip-universal-pipeline-plan.md b/docs/plans/2026-07-30-001-feat-lossless-roundtrip-universal-pipeline-plan.md new file mode 100644 index 00000000..de051a08 --- /dev/null +++ b/docs/plans/2026-07-30-001-feat-lossless-roundtrip-universal-pipeline-plan.md @@ -0,0 +1,320 @@ +--- +date: 2026-07-30 +type: feat +status: active +origin: docs/brainstorms/2026-07-25-lossless-roundtrip-universal-pipeline-requirements.md +--- + +# Prove lossless roundtrip and unify guide ingest/emit pipeline + +## Summary + +Make natural-language prose parsing the primary mechanism for reading `mod-builds` guides (confirmed: the real corpus never uses ModSync's structured `#### Options`/`Guid:` markdown), add first-class parsing for the guide's current `:::note`/`:::warning` admonition-fence format, prove ModSync's own instruction file round-trips losslessly through the guide markdown and back (`C2`), and route both CLI and GUI through one shared ingest/emit path with a single options contract. + +This builds on top of the already-open, separately-shipped PR #196 (`fix(core): preserve GUID/instructions/options across markdown round-trip`), which fixed embedded-metadata-block parsing bugs this plan's own round-trip tests depend on. It does not duplicate that work. + +--- + +## Problem Frame + +Three confirmed gaps, per the origin brainstorm's repo-history and prior-author-correspondence research (see origin: docs/brainstorms/2026-07-25-lossless-roundtrip-universal-pipeline-requirements.md): + +1. **The parser's structural lever is unused by real content.** `MarkdownParser.cs`'s `#### Instructions`/`#### Options`/`**GUID:**` parsing is real, tested code that the actual guide has never once used across ~850 commits. `NaturalLanguageInstructionParser.cs` — an ordered ~40-pattern regex list with no gap-reporting contract — is the only mechanism that actually reads real guide content, but its failures are silent (verbose-log-only) and it has no concept of conditional/branching instructions. +2. **The current guide format is only narrowly handled.** The guide converted to Docusaurus-style `:::note`/`:::warning` admonition fences in Oct 2025. `MarkdownImportProfile.CreateDefault()` has fence-aware regex alternatives, but only for two hardcoded field names ("Installation Instructions", "Known Bugs"); every other field lacks fence support, and the definition-list `: ` prefix isn't explicitly stripped the way a separate, unrelated parser in `ModBuildConverter.cs` (`ParseSpoilerFreeMarkdown`) already does correctly. +3. **CLI and GUI each bypass the shared port.** `GuideIngestService`/`GuideEmitService` (`src/ModSync.Core/Ports/Guides/`) exist but are used only by tests. CLI `convert`, GUI file-open, and GUI paste each call `MarkdownParser`/`DraftInstructionService` directly, each with its own literal draft-on/off default (file-open: off, paste: on — a deliberate, documented asymmetry per `docs/knowledgebase/guide-ingestion.md` that must be preserved, not collapsed). + +--- + +## Requirements + +Traced to origin R1-R15 (see origin document for full text). Grouped here by implementation phase. + +**Parser coverage:** R5 (natural-language parsing is primary), R6 (admonition-fence format first-class), R7 (phrasing pattern coverage), R8 (draft-rate floor), R9 (unparsed prose surfaces as a reviewable gap, never silently dropped). + +**Roundtrip fidelity:** R1 (C2 lossless loop), R2 (metadata survives C2), R3 (C1 ingest completeness), R4 (C3 content parity, historical/best-effort only per origin Key Decisions). + +**Unified pipeline:** R10 (CLI/GUI share one ingest/emit path), R11 (same guide + same options → same output on both surfaces). + +**Measurement and corpus handling:** R12 (C1/C2/draft-floor proven by tests in default+agent flows), R13 (regression guards absolute invariants, not a ratchet), R14 (corpus read-only), R15 (pinned repo/ref, tests skip cleanly when absent). + +--- + +## Key Technical Decisions + +- **Generalize the existing fence support rather than replacing it.** `MarkdownImportProfile`'s two hardcoded fence alternatives (Installation Instructions, Known Bugs) become a general per-field fence pattern applied uniformly, reusing the definition-list-stripping logic already correct in `ModBuildConverter.cs:1672-1679`'s `ParseSpoilerFreeMarkdown` rather than inventing new stripping logic. +- **Unparsed-gap reporting is a result-contract change, not a regex change.** `DraftInstructionResult` gains a field for unparsed/undraftable prose units; `DraftInstructionService.GenerateDraftInstructions` returns one result per component with Directions — including zero-draft ones — instead of omitting them. `NaturalLanguageInstructionParser.ParseInstructionUnit`'s three real outcomes (matched / recognized-as-commentary / looked-actionable-but-unmatched) become distinguishable to the caller instead of collapsing into one silent no-op. +- **AE7/AE8 (redrob cleanlist, K2CP+HD-Visas) are "detect and flag a gap," not "fully auto-draft," per confirmed scope.** Both are greenfield — no existing scaffolding. The bar is: the conditional nature of the instruction is either correctly captured as structure, or explicitly surfaced as a reviewable gap. Silent misresolution (wrong unconditional deletion, merged conditional+unconditional steps) is the failure mode to eliminate; full automation of every such case is not required. +- **`GuideIngestService` is rewired to call `MarkdownParser` for markdown content**, not the generic `ModComponentSerializationService.DeserializeModComponentFromString` deserializer it uses today — that's the only way to populate preamble/epilogue/widescreen/Aspyr content and the parse trace on the port's result, all of which already exist on `MarkdownParserResult` but are currently dropped at the port boundary. +- **The draft-on/off asymmetry is preserved as an explicit shared option value, not collapsed.** File-open defaults off, paste and `--parse-directions` opt in — unification means one options field with per-surface-supplied defaults, not one hardcoded behavior. +- **Corpus pin: `KOTOR-Community-Portal/mod-builds`, `dev` branch, at a recorded commit** — not the frozen `oldrepublicwizard/mod-builds` TOML repo, and not the `th3w1zard1/mod-builds` mirror `AGENTS.md` currently tells contributors to clone (a doc correction, tracked in Scope Boundaries). +- **New C2 round-trip test suite, not a repurposing of the three dormant MD-anchored suites.** `DocumentationRoundTripTests`/`MarkdownImportTests`/`MarkdownFileTests` compare MD-to-MD or name-list parity; none compare against ModSync's own instruction file, so none can prove `C2`. They still need R15's clean-skip fix independently. + +--- + +## Implementation Units + +### Phase A: Parser Core + +### U1. Generalize admonition-fence parsing to any field + +**Goal:** Every guide field (not just Installation Instructions/Known Bugs) parses correctly whether written as plain bold-inline text or wrapped in a `:::note`/`:::warning` fence with `: ` definition-list continuation lines, with the `: ` prefix stripped from captured content. + +**Requirements:** R6 + +**Dependencies:** None + +**Files:** +- `src/ModSync.Core/Parsing/MarkdownImportProfile.cs` (generalize the fence-pattern approach beyond the two current hardcoded alternatives) +- `src/ModSync.Core/Parsing/MarkdownParser.cs` (if extraction needs a shared post-processing step to strip `: ` prefixes from multi-line fence captures) +- `src/ModSync.Tests/MarkdownFileTests.cs` or a new test file for fence-format coverage + +**Approach:** Mirror the correct definition-list stripping already implemented in `ModBuildConverter.cs`'s `ParseSpoilerFreeMarkdown` (lines ~1672-1679) rather than reimplementing it. Keep the plain bold-inline alternative as a fallback for any field not yet converted to fence style, since both styles can coexist across different fields in the same document. + +**Patterns to follow:** `ModBuildConverter.cs`'s existing `:::` fence stripping; `MarkdownImportProfile.CreateDefault()`'s existing `InstallationInstructionsPattern`/`KnownBugsPattern` as the starting shape to generalize. + +**Test scenarios:** +- Happy path: a field written in `:::note` fence style with a `: ` prefixed single-line body parses identically to the same content written bold-inline. +- Happy path: a multi-line fence body (several `: `-prefixed lines) has every line's prefix stripped and lines joined into one coherent prose value. +- Edge case: a document mixing fence-style fields and bold-inline fields in the same component parses both correctly. +- Edge case: a `:::warning` fence for a field with no existing hardcoded alternative (i.e., a field name never seen in the two-field hardcode) still parses via the generalized pattern. +- Error path: a malformed/unclosed fence does not crash the parser; it degrades to treating the content as an unrecognized field rather than throwing. + +**Verification:** New tests pass; the two existing fence-dependent tests (if any reference `InstallationInstructionsPattern`/`KnownBugsPattern` directly) continue to pass unchanged. + +--- + +### U2. Add unparsed-gap reporting to the draft pipeline + +**Goal:** Prose that the natural-language parser cannot interpret as an instruction is never silently dropped — it surfaces as an explicit, reviewable gap the caller can render. + +**Requirements:** R9 + +**Dependencies:** None + +**Files:** +- `src/ModSync.Core/Parsing/NaturalLanguageInstructionParser.cs` (`ParseInstructionUnit` outcome signal) +- `src/ModSync.Core/Parsing/DraftInstructionService.cs` (`DraftInstructionResult` shape, `GenerateDraftInstructions` per-component result emission) +- `src/ModSync.Tests/` — new or extended tests for gap surfacing + +**Approach:** Distinguish the three real outcomes `ParseInstructionUnit` already computes internally (matched / recognized-as-commentary / looked-actionable-but-unmatched) and return that distinction rather than only logging it at verbose level. `GenerateDraftInstructions` returns one `DraftInstructionResult` per component that has Directions prose, including components where zero instructions drafted, carrying the unparsed unit text so CLI and GUI can both render "N of M directions produced no draft." + +**Test scenarios:** +- Happy path: prose that fully matches known patterns produces a result with zero unparsed units. +- Edge case: prose recognized as pure commentary (matches `IsInformationalOnly`) is not counted as an unparsed gap — it's correctly skipped, not flagged. +- Edge case: prose that contains an action verb but matches no pattern is flagged as an unparsed gap distinct from commentary. +- Integration: a component with Directions but zero successfully-drafted instructions still appears in `GenerateDraftInstructions`'s result list (today it's omitted). + +**Verification:** `DraftInstructionResult` for a known-gap fixture (e.g. a deliberately unparseable sentence) shows the gap; existing components that draft cleanly show zero gaps and unchanged instruction output. + +--- + +### U3. Clause-level conditional decomposition + +**Goal:** A single Directions sentence expressing two distinct actions — one unconditional, one conditional on another mod being present — decomposes into two separate instructions rather than staying one flat unit. + +**Requirements:** R7 (AE8) + +**Dependencies:** U2 (gap reporting should exist so genuinely undecomposable conditionals still surface cleanly rather than silently misparsing) + +**Files:** +- `src/ModSync.Core/Parsing/NaturalLanguageInstructionParser.cs` (`SplitIntoProcessingUnits`, conditional-clause pattern) +- `src/ModSync.Tests/` — AE8 fixture test + +**Approach:** Extend the sentence/clause splitter to recognize an explicit "if using X, additionally/also Y" clause boundary as a second processing unit, tagged with its condition, rather than feeding the whole sentence through the flat pattern list as one unit. This is squarely R7's most concrete named case (K2CP+HD-Visas: delete a base file set unconditionally, delete three more files only if a second specific mod is also used). + +**Test scenarios:** +- Happy path: the literal K2CP+HD-Visas Directions text (see origin document's quoted example) decomposes into an unconditional deletion instruction and a separate HD-Visas-conditional deletion instruction. +- Covers AE8. Given the K2CP mod's Directions with the nested "if also using HD Visas" clause, when ingested, both the unconditional and conditional deletions are captured as distinct instructions, not merged into one. +- Edge case: a sentence with no conditional clause is unaffected by the new splitter logic (no regression to existing single-unit parsing). + +**Verification:** AE8 fixture passes; existing `NaturalLanguageInstructionParser` tests for non-conditional sentences are unaffected. + +--- + +### U4. Redrob cleanlist-style conditional deletion + +**Goal:** A Directions entry whose deletion list depends on an externally-maintained cleanlist (not enumerable from the guide text alone) is captured as a conditional reference rather than silently resolved to a wrong fixed file list. + +**Requirements:** R7 (AE7) + +**Dependencies:** U2 + +**Files:** +- `src/ModSync.Core/Parsing/NaturalLanguageInstructionParser.cs` or a new dedicated parsing unit for cleanlist-style references +- `src/ModSync.Tests/` — AE7 fixture test + +**Approach:** Greenfield — no existing scaffolding (confirmed in research). Per the confirmed scope decision, the bar is detecting that a deletion is conditional-on-an-external-list and preserving that as metadata or an explicit gap — not dynamically fetching/parsing the actual `cleanlist_k1.txt` file from the corpus, which is out of scope here (see Scope Boundaries). + +**Test scenarios:** +- Covers AE7. Given a Directions entry referencing conditional per-mod deletion driven by an external cleanlist, when ingested, the conditional nature is preserved as metadata or surfaces as an explicit gap — never silently resolved to a wrong fixed file list. +- Edge case: a Directions entry with a literal, non-cleanlist-referencing file deletion list is unaffected and drafts normally. + +**Verification:** AE7 fixture passes without silently fabricating a wrong file list. + +--- + +### U5. Verify HQ Blasters-style multi-step sequence coverage + +**Goal:** Confirm the existing pattern set correctly handles the HQ Blasters sequence (delete a file from TSLPatchdata to force an intentional single patcher error, run the patcher, then post-install rename and delete several files) end-to-end, or identify and close the specific gap. + +**Requirements:** R7 (AE6) + +**Dependencies:** None + +**Files:** +- `src/ModSync.Core/Parsing/NaturalLanguageInstructionParser.cs` (verify/extend Patcher + rename + delete pattern sequencing) +- `src/ModSync.Tests/` — AE6 fixture test + +**Approach:** This may turn out to be verification-only if the existing Patcher/rename/delete patterns already compose correctly across a multi-step sentence sequence — confirm with the literal HQ Blasters text (see origin document) before assuming new patterns are needed. + +**Test scenarios:** +- Covers AE6. Given the HQ Blasters Directions text verbatim, when ingested, either the full instruction sequence drafts correctly, or the undraftable portion surfaces as an explicit gap per U2 — never silently dropped or partially applied without signal. + +**Verification:** AE6 fixture passes or produces a clean, reviewable gap (not a silent partial result). + +--- + +### Phase B: Unified Pipeline + +### U6. Extend the guide ingest/emit ports + +**Goal:** `IGuideIngestService`/`IGuideEmitService` carry everything `MarkdownParser`/`MarkdownParserResult` already produce — preamble/epilogue/widescreen/Aspyr content, parser profile/options, parse trace, and U2's unparsed-gap surface — instead of dropping them at the port boundary. + +**Requirements:** R2, R10 + +**Dependencies:** U1, U2 (the port should carry the new fence-parsing and gap-reporting capabilities, not just the old ones) + +**Files:** +- `src/ModSync.Core/Ports/Guides/IGuideServices.cs` +- `src/ModSync.Core/Ports/Guides/GuideServices.cs` +- `src/ModSync.Tests/` — port contract tests + +**Approach:** Rewire `GuideIngestService.IngestFromText` to call `MarkdownParser` directly for markdown-format content instead of the generic `ModComponentSerializationService.DeserializeModComponentFromString` deserializer it uses today, so the richer `MarkdownParserResult` fields become available to populate the extended `GuideIngestResult`. Extend `GuideEmitService.EmitMarkdown`/`EmitMarkdownAsync` to accept widescreen/Aspyr content alongside the existing preamble/epilogue parameters. + +**Test scenarios:** +- Happy path: ingesting a guide with preamble, epilogue, widescreen, and Aspyr sections populates all four on the port result. +- Integration: a parse that produces U2-style unparsed gaps surfaces them on the port result, not only when calling `MarkdownParser` directly. +- Edge case: ingesting non-markdown formats (TOML/YAML/JSON) through the same port continues to work unchanged (the markdown-specific rewiring must not regress other formats). + +**Verification:** Existing port tests continue to pass; new tests confirm preamble/epilogue/widescreen/Aspyr/trace/gap fields are populated for markdown input. + +--- + +### U7. Route CLI and GUI through the extended port + +**Goal:** CLI `convert`/`merge` and GUI file-open/paste ingest and emit guides through the U6 port instead of calling `MarkdownParser`/`DraftInstructionService` directly, with the paste-vs-file-open draft-on/off asymmetry expressed as one explicit option value passed by each caller rather than three independent hardcoded literals. + +**Requirements:** R10, R11 + +**Dependencies:** U6 + +**Files:** +- `src/ModSync.Core/CLI/ModBuildConverter.cs` +- `src/ModSync.GUI/Services/FileLoadingService.cs` +- `src/ModSync.GUI/ViewModels/RegexImportDialogViewModel.cs` (or wherever the editor/config ingest path lives) +- `src/ModSync.Tests/` — CLI/GUI parity tests + +**Approach:** Replace each surface's direct `MarkdownParser`/`DraftInstructionService` calls with calls through `IGuideIngestService`/`IGuideEmitService`, passing each surface's existing draft-flag default as the option value (CLI convert: off unless `--parse-directions`; GUI file-open: off; GUI paste: on) rather than changing any surface's actual default behavior. + +**Test scenarios:** +- Covers AE4 (origin R11). Given one guide loaded via CLI and GUI with drafting off, both produce identical components and instructions; with drafting on, both produce identical drafts. +- Edge case: GUI paste's draft-on default and GUI file-open's draft-off default are both still correct after the routing change (no behavior collapse). +- Integration: CLI `convert --parse-directions` produces the same drafted instructions as GUI paste for the same input guide. + +**Verification:** AE4 passes; no direct `MarkdownParser`/`DraftInstructionService` calls remain in `ModBuildConverter.cs` or the GUI ingest paths outside the port. + +--- + +### Phase C: Measurement and Regression Safety + +### U8. Self-contained C2 round-trip regression test + +**Goal:** A regression guard proving `IF → emitted guide → IF` preserves every component, instruction, and Choose tree, using inline test data — no external corpus dependency, so it always runs. + +**Requirements:** R1, R2, R12, R13 (AE1) + +**Dependencies:** U1-U7 (the invariant should be tested against the upgraded parser/pipeline, not the pre-upgrade one) + +**Files:** +- `src/ModSync.Tests/C2RoundtripInvariantTests.cs` (new — note this file exists on a separate, unmerged branch from prior work; this unit creates the equivalent on top of this plan's branch, verifying it doesn't already exist before writing) + +**Approach:** Mirror the self-contained-fixture pattern (inline instruction file with a Choose component, no corpus dependency) rather than depending on `./mod-builds`. + +**Test scenarios:** +- Covers AE1. Given an instruction file with a Choose component, when it is emitted and re-ingested, then the Choose tree and each branch's instructions are identical to the original. +- Happy path: a simple component with instructions round-trips with all fields intact. +- Edge case: multiple components with mixed Choose/non-Choose content preserve count and order. + +**Verification:** Test passes without requiring `./mod-builds` to be present. + +--- + +### U9. Corpus-pinned measurement and clean-skip fixes + +**Goal:** Pin the corpus to `KOTOR-Community-Portal/mod-builds` `dev` at a recorded commit; add clean-skip-when-absent to every corpus-dependent test that currently hard-fails; measure and record the R8 draft-rate floor and R4 exception list from a live pinned read. + +**Requirements:** R4, R8, R12, R13, R14, R15 + +**Dependencies:** U1-U5 (measurement should reflect the upgraded parser, not the baseline one) + +**Files:** +- `src/ModSync.Tests/MarkdownTomlParityTests.cs` (currently hard-fails via `Assert.That(File.Exists(...), Is.True)` when `./mod-builds` is absent — add clean skip) +- `src/ModSync.Tests/DocumentationRoundTripTests.cs`, `MarkdownImportTests.cs`, `MarkdownFileTests.cs` (same clean-skip gap, confirmed via research) +- `AGENTS.md` (correct the `th3w1zard1/mod-builds` clone instruction to `KOTOR-Community-Portal/mod-builds` `dev`) +- A recorded-baseline artifact (e.g. a committed fixture or doc noting the measured draft-rate floor and exception list at the pinned commit) + +**Approach:** Introduce one shared skip-check pattern (e.g. a helper checking `./mod-builds` exists before the corpus-dependent tests run, using `Assert.Ignore`/`Assert.Inconclusive` rather than `Assert.Fail`) and apply it consistently across all four files, per the R15 gap confirmed identical across all of them. Note that `MarkdownTomlParityTests.cs`'s current fixture layout assumes one `./mod-builds` clone contains both the canonical guide content and the frozen TOML — since those now live in two different repos per the origin document's Key Decisions, this unit must also decide (and document) how the pinned local corpus is assembled for this test (e.g., vendoring the frozen TOML snapshot separately, or dropping the TOML-comparison test in favor of a documented one-time historical record). + +**Test scenarios:** +- Edge case: each of the four corpus-dependent test files skips cleanly (not a hard failure) when `./mod-builds` is absent. +- Integration: with `./mod-builds` present at the pinned commit, the draft-rate floor and C3 exception list are measured and match the recorded baseline. +- Test expectation: none for the `AGENTS.md` correction — pure documentation fix, no behavioral change. + +**Verification:** All four files run green in CI without `./mod-builds`; with it present at the pinned commit, they produce the recorded baseline numbers. + +--- + +## Scope Boundaries + +**In scope** +- Natural-language parser upgrades for the current (post-Oct-2025) guide format only. +- Unparsed-gap reporting contract change. +- The three named acceptance-example cases (AE6-AE8), to the "detect and flag" bar, not full automation guarantee. +- CLI/GUI pipeline unification via the existing ports. +- Corpus pin correction and clean-skip test fixes. + +**Deferred for later** +- Byte-identical markdown reproduction (per origin document). +- The pre-Oct-2025 bold-inline guide format (per origin document, confirmed obsolete). +- Full automated dynamic parsing of redrob's actual `cleanlist_k1.txt` file content (U4 only detects/flags the conditional reference; fetching and interpreting the external cleanlist is separate future work). +- Generalization beyond `KOTOR1_Full`/`KOTOR2_Full` to other authors' guides. + +**Outside this product's identity** +- A mandatory bespoke guide markup language (per origin document — the guide author has explicitly declined prose standardization). +- Turning `mod-builds` into a writable store. + +--- + +## Dependencies / Assumptions + +- Builds on top of PR #196's already-shipped fixes to embedded-metadata-block parsing (GUID/instruction preservation); does not duplicate that work. +- A `./mod-builds` clone pinned to `KOTOR-Community-Portal/mod-builds` `dev` branch is required for U9's measurement work; U8's regression test is self-contained and does not require it. +- The frozen `oldrepublicwizard/mod-builds` TOML (last commit 2025-10-31) is available only as a historical snapshot for U9's one-time C3 cross-check, not a continuously-updated source. + +## Risk Analysis + +| Risk | Likelihood | Impact | Mitigation | +|------|-----------|--------|------------| +| U3/U4 (conditional decomposition, cleanlist detection) prove harder than scoped and drift toward full automation | Medium | Medium | Confirmed scope bar is "detect and flag," not full automation — hold the line at that bar per Key Technical Decisions | +| U7's port rewiring silently changes CLI/GUI draft-on/off behavior for existing users | Low | High | AE4 test explicitly proves parity; existing per-surface defaults are passed through as data, not re-decided | +| U9's corpus-repo split (guide content vs. frozen TOML in two different repos) complicates the existing test's fixture assumptions | Medium | Low | Explicitly called out as a decision this unit must make and document, not silently paper over | + +--- + +## Verification + +- All nine units' test scenarios pass. +- `dotnet build ModSync.sln` succeeds with no new errors. +- U8's C2 regression test runs in the default test flow without requiring `./mod-builds`. +- U9's corpus-dependent tests skip cleanly without `./mod-builds`, and produce the recorded baseline with it present at the pinned commit. +- No direct `MarkdownParser`/`DraftInstructionService` calls remain in `ModBuildConverter.cs` or GUI ingest paths outside `IGuideIngestService`/`IGuideEmitService`. diff --git a/src/ModSync.Core/CLI/ModBuildConverter.cs b/src/ModSync.Core/CLI/ModBuildConverter.cs index d898b25e..0ed3916f 100644 --- a/src/ModSync.Core/CLI/ModBuildConverter.cs +++ b/src/ModSync.Core/CLI/ModBuildConverter.cs @@ -1912,20 +1912,38 @@ private static async Task RunConvertAsync(ConvertOptions opts) } List components; + Ports.Guides.GuideIngestResult ingestResult; try { + string content; + string formatHint; if (opts.UseStdin) { - string stdinContent = await Console.In.ReadToEndAsync().ConfigureAwait(false); - components = (await ModComponentSerializationService - .DeserializeModComponentFromStringAsync(stdinContent) - .ConfigureAwait(false)).ToList(); + content = await Console.In.ReadToEndAsync().ConfigureAwait(false); + formatHint = null; } else { - components = await FileLoadingService.LoadFromFileAsync(opts.InputPath).ConfigureAwait(false); + (content, formatHint) = await FileLoadingService.ReadFileContentAndFormatHintAsync(opts.InputPath).ConfigureAwait(false); } + if (opts.ParseDirections) + { + msg = "Drafting instructions from natural-language Directions prose..."; + if (s_progressDisplay != null) + { + s_progressDisplay.WriteScrollingLog(msg); + } + else + { + await Logger.LogVerboseAsync(msg).ConfigureAwait(false); + } + } + + ingestResult = await Task.Run(() => Ports.Guides.GuideIngestService.Instance.IngestFromText( + content, formatHint, opts.ParseDirections)).ConfigureAwait(false); + components = ingestResult.Components.ToList(); + // Handle dependency resolution components = (List)HandleDependencyResolutionErrors(components, opts.IgnoreErrors, "Convert"); @@ -1938,6 +1956,20 @@ private static async Task RunConvertAsync(ConvertOptions opts) { await Logger.LogVerboseAsync(msg).ConfigureAwait(false); } + + if (opts.ParseDirections) + { + int draftedCount = ingestResult.DraftResults.Count(r => r.DraftInstructionCount > 0); + msg = $"Drafted instructions for {draftedCount} component(s) - all drafts are flagged for review"; + if (s_progressDisplay != null) + { + s_progressDisplay.WriteScrollingLog(msg); + } + else + { + await Logger.LogAsync(msg).ConfigureAwait(false); + } + } } catch (Exception ex) { @@ -1950,34 +1982,7 @@ private static async Task RunConvertAsync(ConvertOptions opts) throw; } - IReadOnlyList draftResults = null; - if (opts.ParseDirections) - { - msg = "Drafting instructions from natural-language Directions prose..."; - if (s_progressDisplay != null) - { - s_progressDisplay.WriteScrollingLog(msg); - } - else - { - await Logger.LogVerboseAsync(msg).ConfigureAwait(false); - } - - draftResults = Parsing.DraftInstructionService.GenerateDraftInstructions( - components, - logInfo: message => Logger.Log(message), - logVerbose: message => Logger.LogVerbose(message)); - - msg = $"Drafted instructions for {draftResults.Count} component(s) - all drafts are flagged for review"; - if (s_progressDisplay != null) - { - s_progressDisplay.WriteScrollingLog(msg); - } - else - { - await Logger.LogAsync(msg).ConfigureAwait(false); - } - } + IReadOnlyList draftResults = opts.ParseDirections ? ingestResult.DraftResults : null; if (opts.Download) { @@ -2145,11 +2150,18 @@ private static async Task RunConvertAsync(ConvertOptions opts) // Create validation context to track issues for serialization var validationContext = new ComponentValidationContext(); - // Flag prose-drafted instructions for review in the serialized output (never auto-trusted) + // Flag prose-drafted instructions for review in the serialized output (never auto-trusted). + // DraftResults now includes components whose Directions produced zero drafts (U2 gap + // reporting) - only flag components that actually drafted instructions. if (draftResults != null) { foreach (Parsing.DraftInstructionResult draftResult in draftResults) { + if (draftResult.DraftInstructionCount == 0) + { + continue; + } + validationContext.AddModComponentIssue( draftResult.Component.Guid, Parsing.DraftInstructionService.ReviewFlagMessage); diff --git a/src/ModSync.Core/ModComponent.cs b/src/ModSync.Core/ModComponent.cs index f1502272..88f4f1d0 100644 --- a/src/ModSync.Core/ModComponent.cs +++ b/src/ModSync.Core/ModComponent.cs @@ -807,8 +807,10 @@ public string SerializeComponent() return Services.ModComponentSerializationService.SerializeSingleComponentAsTomlString(this); } + /// The raw TOML content to deserialize. + /// See . [CanBeNull] - public static ModComponent DeserializeTomlComponent([NotNull] string tomlString) + public static ModComponent DeserializeTomlComponent([NotNull] string tomlString, bool requireName = true) { if (tomlString is null) { @@ -816,7 +818,7 @@ public static ModComponent DeserializeTomlComponent([NotNull] string tomlString) } // Use the unified deserialization service - IReadOnlyList components = Services.ModComponentSerializationService.DeserializeModComponentFromTomlString(tomlString); + IReadOnlyList components = Services.ModComponentSerializationService.DeserializeModComponentFromTomlString(tomlString, requireName); return components?.FirstOrDefault(); } public async Task InstallAsync( diff --git a/src/ModSync.Core/Parsing/DraftInstructionService.cs b/src/ModSync.Core/Parsing/DraftInstructionService.cs index 33bf1317..75edeeef 100644 --- a/src/ModSync.Core/Parsing/DraftInstructionService.cs +++ b/src/ModSync.Core/Parsing/DraftInstructionService.cs @@ -22,10 +22,39 @@ public sealed class DraftInstructionResult public int DraftInstructionCount { get; } - public DraftInstructionResult([NotNull] ModComponent component, int draftInstructionCount) + /// + /// Directions prose that contained an action verb but matched no known instruction pattern. + /// Never populated with commentary/informational prose - only genuine unparsed gaps. Callers + /// should render these (e.g. "N of M directions produced no draft") rather than drop them silently. + /// + [NotNull] + [ItemNotNull] + public IReadOnlyList UnparsedGaps { get; } + + public bool HasUnparsedGaps => UnparsedGaps.Count > 0; + + /// + /// Human-readable notes for instructions drafted from a nested conditional clause (the + /// K2CP+HD-Visas pattern: "delete these files; if also using X, additionally delete these"). + /// Each note names the mod the draft is conditional on - the drafted instruction is never + /// auto-applied unconditionally, but this distinguishes it from an ordinary unconditional draft. + /// + [NotNull] + [ItemNotNull] + public IReadOnlyList ConditionalDrafts { get; } + + public bool HasConditionalDrafts => ConditionalDrafts.Count > 0; + + public DraftInstructionResult( + [NotNull] ModComponent component, + int draftInstructionCount, + [CanBeNull][ItemNotNull] IReadOnlyList unparsedGaps = null, + [CanBeNull][ItemNotNull] IReadOnlyList conditionalDrafts = null) { Component = component ?? throw new ArgumentNullException(nameof(component)); DraftInstructionCount = draftInstructionCount; + UnparsedGaps = unparsedGaps ?? Array.Empty(); + ConditionalDrafts = conditionalDrafts ?? Array.Empty(); } } @@ -45,7 +74,7 @@ public static class DraftInstructionService public const string ReviewFlagMessage = "DRAFT INSTRUCTIONS: parsed from guide prose by the natural-language importer. Review before installing - never auto-trusted."; - [NotNull] private const string ModDirectoryPlaceholder = "<>"; + [NotNull] internal const string ModDirectoryPlaceholder = "<>"; [NotNull] private const string KotorDirectoryPlaceholder = "<>"; [NotNull] private const string LegacyGameDirectoryPlaceholder = "<>"; @@ -53,7 +82,10 @@ public static class DraftInstructionService /// Generates draft instructions for every component that has natural-language Directions prose /// but no authored instructions. Components that already have instructions are never touched. /// - /// One result per component that received at least one draft instruction. + /// + /// One result per component that has Directions prose to draft from - including components where + /// zero instructions were successfully drafted, so callers can render unparsed gaps for review. + /// [NotNull] [ItemNotNull] public static IReadOnlyList GenerateDraftInstructions( @@ -79,12 +111,16 @@ public static IReadOnlyList GenerateDraftInstructions( } ObservableCollection parsed; + IReadOnlyList unparsedGaps; + IReadOnlyList conditionalDrafts; try { parsed = parser.ParseInstructions( component.Directions, string.IsNullOrWhiteSpace(component.DownloadInstructions) ? null : component.DownloadInstructions, - component); + component, + out unparsedGaps, + out conditionalDrafts); } catch (Exception ex) { @@ -111,8 +147,20 @@ public static IReadOnlyList GenerateDraftInstructions( { ApplyReviewFlag(component); info($"[DraftInstructions] Drafted {added} instruction(s) from prose for '{component.Name}' - flagged for review."); - results.Add(new DraftInstructionResult(component, added)); } + + if (unparsedGaps.Count > 0) + { + info($"[DraftInstructions] {unparsedGaps.Count} direction(s) for '{component.Name}' produced no draft - review needed."); + } + + if (conditionalDrafts.Count > 0) + { + ApplyConditionalDraftNote(component, conditionalDrafts); + info($"[DraftInstructions] {conditionalDrafts.Count} draft(s) for '{component.Name}' are conditional on another mod - review needed."); + } + + results.Add(new DraftInstructionResult(component, added, unparsedGaps, conditionalDrafts)); } return results; @@ -181,15 +229,43 @@ public static void ApplyReviewFlag([NotNull] ModComponent component) throw new ArgumentNullException(nameof(component)); } + AppendWarningIfMissing(component, ReviewFlagMessage, prepend: true); + } + + /// + /// Appends conditional-draft notes (see ) to a + /// component's so a reviewer sees, alongside the + /// general draft review flag, exactly which drafted instructions are conditional on another mod. + /// Does not duplicate notes already present. + /// + private static void ApplyConditionalDraftNote( + [NotNull] ModComponent component, + [NotNull][ItemNotNull] IReadOnlyList conditionalDrafts) + { + foreach (string note in conditionalDrafts) + { + AppendWarningIfMissing(component, note, prepend: false); + } + } + + /// + /// Adds to a component's + /// unless it's already present. controls whether new text goes before + /// or after any existing warning content. + /// + private static void AppendWarningIfMissing([NotNull] ModComponent component, [NotNull] string text, bool prepend) + { if (string.IsNullOrWhiteSpace(component.InstallationWarning)) { - component.InstallationWarning = ReviewFlagMessage; + component.InstallationWarning = text; return; } - if (component.InstallationWarning.IndexOf(ReviewFlagMessage, StringComparison.Ordinal) < 0) + if (component.InstallationWarning.IndexOf(text, StringComparison.Ordinal) < 0) { - component.InstallationWarning = ReviewFlagMessage + Environment.NewLine + component.InstallationWarning; + component.InstallationWarning = prepend + ? text + Environment.NewLine + component.InstallationWarning + : component.InstallationWarning + Environment.NewLine + text; } } diff --git a/src/ModSync.Core/Parsing/MarkdownImportProfile.cs b/src/ModSync.Core/Parsing/MarkdownImportProfile.cs index adca8b9b..fcbc0d9d 100644 --- a/src/ModSync.Core/Parsing/MarkdownImportProfile.cs +++ b/src/ModSync.Core/Parsing/MarkdownImportProfile.cs @@ -572,12 +572,22 @@ public MarkdownImportProfile Clone() return clone; } + /// + /// Builds a regex alternative matching a field expressed as a Docusaurus-style admonition + /// fence (:::note/:::warning/:::tip/:::danger/:::info) + /// wrapping a definition-list body (: <content>). The guide has used this + /// convention for every field since October 2025; any admonition type may wrap any field, + /// so this generalizes rather than hardcoding one admonition type per field. + /// + private static string BuildFencePattern(string titlePattern, string groupName) => + @":::(?:note|warning|tip|danger|info)\s*\r?\n\s*(?:" + titlePattern + @")\s*\r?\n:\s*(?<" + groupName + @">(?:(?!\r?\n\s*:::).)*?)\r?\n\s*:::"; + public static MarkdownImportProfile CreateDefault() { // Shared boundary: next bold **Field:**, plain Field:, horizontal rule, or markdown heading. // Bold alternatives are listed first so **Name:** wins when both styles appear. const string fieldBoundary = - @"(?:\*\*[^*\r\n]{1,100}:\*\*|(?:Name|Author|Description|Masters|Category\s*&\s*Tier|Non-English Functionality|Installation Method|Installation Instructions|Install Instructions|Download Instructions|Usage Warning|Screenshots|Known Bugs|Installation Warning|Compatibility Warning|Steam Notes)\s*:|#{2,3}\s|_{3,}|-{3,})"; + @"(?:\*\*[^*\r\n]{1,100}:\*\*|(?:Name|Author|Description|Masters|Category\s*&\s*Tier|Non-English Functionality|Installation Method|Installation Instructions|Install Instructions|Download Instructions|Usage Warning|Screenshots|Known Bugs|Installation Warning|Compatibility Warning|Steam Notes)\s*:|#{2,3}\s|_{3,}|-{3,}|:::(?:note|warning|tip|danger|info))"; const string defaultRawPattern = @"(?ms)^###\s*(?.+?)\s*\r?\n" + @@ -608,23 +618,29 @@ public static MarkdownImportProfile CreateDefault() HeadingPattern = @"^###\s+(?.+?)(?:\s*\[.*?\])?\s*$", NamePattern = @"(?:\*\*Name:\*\*|Name:)\s*(?:\[(?(?[^\]]+))\]\([^)]+\)|(?[^\r\n]+))[^\r\n]*", AuthorPattern = @"(?:\*\*Author:\*\*|Author:)\s*(?[^\r\n]+)", - DescriptionPattern = @"(?:\*\*Description:\*\*|Description:)\s*(?" + multilineFieldBody + @")", + DescriptionPattern = BuildFencePattern("Description", "description") + + @"|(?:\*\*Description:\*\*|Description:)\s*(?" + multilineFieldBody + @")", ModLinkPattern = @"\[(?