diff --git a/src/GenWave.Ads/AdRenderService.cs b/src/GenWave.Ads/AdRenderService.cs index 84fcde13..70750153 100644 --- a/src/GenWave.Ads/AdRenderService.cs +++ b/src/GenWave.Ads/AdRenderService.cs @@ -349,6 +349,12 @@ async Task RenderPreviewCoreAsync( return (null, $"render: stored script no longer parses ({reason})"); } + // SPEC F200.3 — every unknown-tag fold note the re-parse carried reaches the application log at + // INFO, structured (never interpolated): the "booth log" F200.3 names has no dedicated sink + // reachable from GenWave.Ads, only ILogger, so this IS that surface. + foreach (var note in script.Notes) + logger.LogInformation("Ad spot {Id} parse note {Note}", spot.Id, note); + var (bed, bedFailure) = await ResolveBedAsync(spot.BedMediaId, ct); if (bedFailure is not null) return (null, bedFailure); diff --git a/src/GenWave.Ads/AdScript.cs b/src/GenWave.Ads/AdScript.cs index 32a96ed6..0f317bfa 100644 --- a/src/GenWave.Ads/AdScript.cs +++ b/src/GenWave.Ads/AdScript.cs @@ -5,5 +5,12 @@ namespace GenWave.Ads; /// cref="AdScriptValidator.Validate"/> hands back on . /// Render (PLAN T401) reads directly for its cast-of-voices assembly. /// -/// Every parsed line, in script order, each carrying its own voice tag. -public sealed record AdScript(IReadOnlyList Lines); +/// Every parsed line, in script order, each carrying its own voice tag — already +/// folded onto the known cast (SPEC F200.1): a line whose original tag was not /VOICE1/VOICE2 is attributed to here, its copy kept verbatim. +/// One entry per DISTINCT unknown tag the script carried (SPEC F200.1/F200.3), in +/// first-seen order — "unknown-tag:{TAG}". Empty when every line's tag was already known. Never +/// a validator failure (STORY-467 AC6): returns this +/// unchanged on . +public sealed record AdScript(IReadOnlyList Lines, IReadOnlyList Notes); diff --git a/src/GenWave.Ads/AdScriptEcho.cs b/src/GenWave.Ads/AdScriptEcho.cs new file mode 100644 index 00000000..d50dd183 --- /dev/null +++ b/src/GenWave.Ads/AdScriptEcho.cs @@ -0,0 +1,27 @@ +namespace GenWave.Ads; + +/// +/// Bounds an untrusted string before it reaches a validation violation's Reason — the ONE place +/// the CWE-117 log-forging discipline lives, shared by (its own tag +/// echoes) and (its own stage-direction echo), rather +/// than each duplicating the same const and one-liner (PLAN T552 review N1). Every violation Reason is +/// logged and surfaced verbatim (STORY-390 AC9's 400), so an untrusted script's own text must never +/// reach it unbounded. +/// +internal static class AdScriptEcho +{ + /// Cap for a value echoed into a violation Reason (the original + /// AdScriptParser.MaxEchoedChars/AdScriptValidator.MaxEchoedChars precedent, PLAN + /// T399 review F6, CWE-117 log forging). + const int MaxEchoedChars = 120; + + /// Truncates to , appending an + /// ellipsis when it was cut, so a violation Reason never carries an unbounded echo of untrusted + /// text. Bounds LENGTH only — no control-character strip: 's own call + /// sites pass a tag that already matched TagPattern (^[A-Z][A-Z0-9]*$, which admits no + /// control character), and 's own call sites echo a regex-matched + /// run (a phone-shaped digit group, a stage-direction shape) rather than a whole raw line — a future + /// call site that would echo genuinely unvalidated freeform text into a Reason must add its own + /// control-character strip first. + public static string ForReason(string text) => text.Length <= MaxEchoedChars ? text : text[..MaxEchoedChars] + "…"; +} diff --git a/src/GenWave.Ads/AdScriptParseNotes.cs b/src/GenWave.Ads/AdScriptParseNotes.cs new file mode 100644 index 00000000..355fdf91 --- /dev/null +++ b/src/GenWave.Ads/AdScriptParseNotes.cs @@ -0,0 +1,21 @@ +namespace GenWave.Ads; + +/// +/// The one PUBLIC seam a stored spot's parse notes reach the Host wire through (SPEC F200.3, +/// STORY-467; PLAN T551) — itself stays (no +/// InternalsVisibleTo widened for GenWave.Host just to reach this one read), so +/// AdsController.ToDto calls instead of the parser directly. +/// +public static class AdScriptParseNotes +{ + /// Re-parses structurally — the SAME + /// AdScriptParser.Parse(script, int.MaxValue) re-parse AdRenderService already runs + /// (never a re-validation: the per-line length rule was already enforced at write time) — and + /// returns its . Empty, never a throw, when + /// is or no longer parses (e.g. hand-edited since it was saved). + public static IReadOnlyList For(string? script) + { + var parsed = AdScriptParser.Parse(script ?? "", int.MaxValue); + return parsed is AdScriptValidationResult.Accepted(var parsedScript) ? parsedScript.Notes : []; + } +} diff --git a/src/GenWave.Ads/AdScriptParser.cs b/src/GenWave.Ads/AdScriptParser.cs index 816ecd6a..12439845 100644 --- a/src/GenWave.Ads/AdScriptParser.cs +++ b/src/GenWave.Ads/AdScriptParser.cs @@ -5,8 +5,9 @@ namespace GenWave.Ads; /// /// The format stage of (SPEC F160.3, STORY-390 AC1/AC8) — the /// CrosstalkScriptParser shape narrowed to the ad wire format: TAG: line, 1-3 DISTINCT -/// uppercase-alphanumeric voice tags, required, each line's text bounded by -/// the caller's per-line char ceiling. Fail-closed, first-rule-wins: the first line/rule that breaks +/// uppercase-alphanumeric voice tags AFTER FOLDING (SPEC F200.1/F200.2 — see this class's own "not +/// every tag is a voice" remarks below), required, each line's text bounded +/// by the caller's per-line char ceiling. Fail-closed, first-rule-wins: the first line/rule that breaks /// the shape is the reason returned, never a full list. /// /// @@ -23,6 +24,18 @@ namespace GenWave.Ads; /// FIRST untagged line among the NON-BLANK lines (blanks are dropped before this numbering runs, so a /// blank interior line never shifts it). /// +/// +/// +/// Not every -shaped tag is a voice (SPEC F200.1, STORY-467): only +/// — , VOICE1, VOICE2 — cast a distinct +/// voice. A tag that matched but is not in (e.g. +/// NARRATOR, VOICE 2 once its space fails the pattern) folds onto +/// instead of refusing — its copy is kept, attributed to the announcer — and +/// records one entry per DISTINCT unknown tag +/// (FoldUnknownTags). / and the "no +/// line" rule both run over the FOLDED lines (SPEC F200.2), so a script that +/// is entirely unknown tags is a valid one-voice, all- script. +/// /// internal static partial class AdScriptParser { @@ -30,13 +43,18 @@ internal static partial class AdScriptParser public const string AnnouncerTag = "ANNOUNCER"; const int MinVoiceTags = 1; + /// F160.3's "1–3" upper bound, kept as a documented invariant: with + /// at exactly three and every other tag folding (F200.2), the > MaxVoiceTags arm is only + /// reachable if the known cast ever grows past three. const int MaxVoiceTags = 3; - /// Cap for a tag echoed into a violation reason (the CrosstalkScriptParser - /// MaxEchoedLineChars precedent, F127.11, PLAN T399 review F6) — an untrusted script's tag - /// text reaches a Reason that is logged and surfaced verbatim (STORY-390 AC9's 400), never an - /// unbounded echo. - const int MaxEchoedChars = 120; + /// The full known cast (SPEC F200.1) — / + /// are the SAME two tags AdScriptPromptBuilder tells the + /// writing model about, duplicated here as the one place the parser itself needs to know which tags + /// are cast, never merely shape-matched. Any other -shaped tag folds onto + /// (FoldUnknownTags) rather than refusing. + static readonly IReadOnlySet KnownTags = + new HashSet(StringComparer.Ordinal) { AnnouncerTag, AdCastPicker.Voice1Tag, AdCastPicker.Voice2Tag }; public static AdScriptValidationResult Parse(string rawScript, int maxLineChars) { @@ -65,14 +83,45 @@ public static AdScriptValidationResult Parse(string rawScript, int maxLineChars) return new AdScriptValidationResult.Refused(violation); } - var distinctTags = lines.Select(line => line.Tag).Distinct(StringComparer.Ordinal).ToList(); + var (foldedLines, notes) = FoldUnknownTags(lines); + + var distinctTags = foldedLines.Select(line => line.Tag).Distinct(StringComparer.Ordinal).ToList(); if (distinctTags.Count is < MinVoiceTags or > MaxVoiceTags) return Refused($"expected {MinVoiceTags}-{MaxVoiceTags} distinct voice tags, got {distinctTags.Count}"); if (!distinctTags.Contains(AnnouncerTag, StringComparer.Ordinal)) return Refused($"no {AnnouncerTag} line appeared — every spot needs the {AnnouncerTag} voice"); - return new AdScriptValidationResult.Accepted(new AdScript(lines)); + return new AdScriptValidationResult.Accepted(new AdScript(foldedLines, notes)); + } + + /// Folds every line whose tag is not in onto + /// (SPEC F200.1) — the line's own is kept + /// verbatim, only its changes. One note per DISTINCT original tag, in + /// first-seen order (SPEC F200.1: "a tag appearing on three lines yields one note"); the tag echoed + /// through , the same untrusted-echo bound every other + /// logged/surfaced tag in this class goes through. + static (IReadOnlyList Lines, IReadOnlyList Notes) FoldUnknownTags( + IReadOnlyList lines) + { + var foldedLines = new List(lines.Count); + var notes = new List(); + var seenUnknownTags = new HashSet(StringComparer.Ordinal); + + foreach (var line in lines) + { + if (KnownTags.Contains(line.Tag)) + { + foldedLines.Add(line); + continue; + } + + foldedLines.Add(line with { Tag = AnnouncerTag }); + if (seenUnknownTags.Add(line.Tag)) + notes.Add($"unknown-tag:{AdScriptEcho.ForReason(line.Tag)}"); + } + + return (foldedLines, notes); } /// The plain-sentence pre-pass itself (SPEC F174.6, PLAN T444 ruling): classifies every @@ -136,10 +185,10 @@ public static AdScriptValidationResult Parse(string rawScript, int maxLineChars) static (AdScriptLine? Line, AdScriptViolation? Violation) CheckLine(string tag, string text, int maxLineChars) { if (text.Length == 0) - return (null, FormatViolation($"the {EchoForReason(tag)} line has no spoken text")); + return (null, FormatViolation($"the {AdScriptEcho.ForReason(tag)} line has no spoken text")); if (text.Length > maxLineChars) - return (null, FormatViolation($"the {EchoForReason(tag)} line ({text.Length} chars) exceeds the {maxLineChars}-char per-line budget")); + return (null, FormatViolation($"the {AdScriptEcho.ForReason(tag)} line ({text.Length} chars) exceeds the {maxLineChars}-char per-line budget")); return (new AdScriptLine(tag, text), null); } @@ -148,14 +197,6 @@ public static AdScriptValidationResult Parse(string rawScript, int maxLineChars) static AdScriptViolation FormatViolation(string reason) => new(AdScriptRuleIds.Format, reason); - /// Bounds an untrusted tag echoed into a violation Reason to - /// (CWE-117 log forging — PLAN T399 review F6). PLAN T444 ruling: every call site passes a - /// that already matched - /// (^[A-Z][A-Z0-9]*$, which admits no control character), so the control-character strip - /// this method carried is unreachable and has been removed — a future call site that echoes - /// raw, unvalidated text into a Reason must bring that strip back, pinned by a fact. - static string EchoForReason(string tag) => tag.Length <= MaxEchoedChars ? tag : tag[..MaxEchoedChars] + "…"; - // Must start with a letter (PLAN T399 review N4) — a digits-only tag ("12") is not a plausible // voice name, so a line whose would-be tag is pure digits reads as malformed FORMAT rather than // silently accepting a nonsense tag. A digit sequence appearing later, inside a line's spoken diff --git a/src/GenWave.Ads/AdScriptRuleIds.cs b/src/GenWave.Ads/AdScriptRuleIds.cs index f65af999..b27349d2 100644 --- a/src/GenWave.Ads/AdScriptRuleIds.cs +++ b/src/GenWave.Ads/AdScriptRuleIds.cs @@ -19,9 +19,14 @@ public static class AdScriptRuleIds /// The script named a blocklisted real-world brand. public const string BrandCollision = "brand_collision"; - /// A phone-shaped digit run does not contain 555. + /// A phone-shaped digit run does not contain 555 (no sponsor phone on file), or does not + /// match the sponsor's own number (one is on file). public const string PhoneShape = "phone_shape"; /// A profane word under the everyone audience posture. public const string AudiencePosture = "audience_posture"; + + /// A parenthetical, bracketed beat, or asterisked aside survived hygiene and still + /// appears in a line's spoken text (SPEC F201.2, STORY-468). + public const string StageDirection = "stage_direction"; } diff --git a/src/GenWave.Ads/AdScriptValidationRequest.cs b/src/GenWave.Ads/AdScriptValidationRequest.cs index f0fdba06..088fb347 100644 --- a/src/GenWave.Ads/AdScriptValidationRequest.cs +++ b/src/GenWave.Ads/AdScriptValidationRequest.cs @@ -26,7 +26,9 @@ namespace GenWave.Ads; /// an owner sponsor with a non-blank value here, 's 555 rule (SPEC /// F172.5) treats a phone-shaped run whose DIGITS equal this value's digits as allowed — the /// comparison is formatting-insensitive (punctuation stripped from both sides), the surrounding script -/// text is still read raw. Every OTHER non-555 phone-shaped run still refuses. +/// text is still read raw. Every OTHER phone-shaped run refuses (SPEC F199.3) — including one that +/// itself contains 555 — once a real sponsor phone is on file; only with no phone on file does the +/// looser "contains 555" rule apply. /// Whether this script belongs to a pack-owned sponsor — the parody posture /// SPEC F172.5 keeps unchanged for packs (neither skip above ever runs). Defaults (PLAN T438 ruling: fail closed, not fail open) so every caller that predates diff --git a/src/GenWave.Ads/AdScriptValidator.cs b/src/GenWave.Ads/AdScriptValidator.cs index 7339b944..89708fed 100644 --- a/src/GenWave.Ads/AdScriptValidator.cs +++ b/src/GenWave.Ads/AdScriptValidator.cs @@ -1,3 +1,4 @@ +using System.Text.RegularExpressions; using GenWave.Core.Abstractions; using GenWave.Core.Domain; @@ -6,12 +7,14 @@ namespace GenWave.Ads; /// /// Pure, fail-closed, first-rule-wins validation of an ad script (SPEC F160.3, STORY-390) — runs on /// EVERY path a script reaches the air from: the LLM writer (T400), the owner editor's save (T403), -/// and a catalog pack's install preview (T405). Five checks, in this fixed order, the first violation +/// and a catalog pack's install preview (T405). Six checks, in this fixed order, the first violation /// wins: /// /// /// Format () — TAG: line, 1-3 distinct voice tags, /// ANNOUNCER required, per-line Llm:MaxCopyChars. +/// Stage direction — a parenthetical/bracketed/asterisked aside that survived hygiene +/// (SPEC F201.2, STORY-468) — a shape rule, checked immediately after Format and before Duration. /// Duration — estimated total read time against spot_seconds + /// tolerance. /// Brand collision — the shipped, folded blocklist. @@ -59,7 +62,7 @@ namespace GenWave.Ads; /// renamed.) /// /// -public static class AdScriptValidator +public static partial class AdScriptValidator { /// The house spoken-rate constant (chars/second) — see 's own /// remarks and this class's own "duration is text-driven" summary above. @@ -76,6 +79,13 @@ public static AdScriptValidationResult Validate( if (parsed is not AdScriptValidationResult.Accepted(var script)) return parsed; + // A shape rule, run right after the parse (SPEC F201.2, STORY-468) — the backstop for the + // three stage-direction shapes AdScriptWriter.ApplyLineAwareHygiene already strips from an + // LLM-authored script (GenWave.Tts): an owner-typed save, a pack script, or a hygiene bug + // never silently airs one of these instead of being named here. + if (CheckStageDirection(script) is { } stageDirectionViolation) + return Refused(stageDirectionViolation); + if (CheckDuration(script, request, durationEstimator) is { } durationViolation) return Refused(durationViolation); @@ -196,18 +206,69 @@ static string StripAllOccurrences(string variant, string foldedLiteral) // on file, passes null through — the plain 555 rule with no exemption. var allowedPhone = request.IsPackOwned ? null : request.SponsorPhone; + // The reason's own wording tracks which rule actually ran (SPEC F199.3, PLAN T552 ruling): with + // a real sponsor phone on file the "contains 555" framing went false the moment F199.3 tightened + // the skip to ONLY that exact number (STORY-466 AC6) — a differing run that itself contains 555 + // still refuses, so telling the operator it "does not contain 555" would be a lie. + var hasAllowedPhone = !string.IsNullOrWhiteSpace(allowedPhone); + // Checked per line, never a whole-script joined string (PLAN T399 review N8) — a digit // fragment ending one voice's line must never combine with a fragment opening the next // line's into a phone-shaped run that existed in neither line alone. foreach (var line in script.Lines) { - if (PhoneShapeCheck.FindViolation(line.Text, allowedPhone) is { } phoneRun) - return new AdScriptViolation(AdScriptRuleIds.PhoneShape, $"a phone-shaped digit run (\"{phoneRun}\") does not contain 555"); + if (PhoneShapeCheck.FindViolation(line.Text, allowedPhone) is not { } phoneRun) + continue; + + var reason = hasAllowedPhone + ? $"a phone-shaped digit run (\"{phoneRun}\") is not the sponsor's own number" + : $"a phone-shaped digit run (\"{phoneRun}\") does not contain 555"; + return new AdScriptViolation(AdScriptRuleIds.PhoneShape, reason); + } + + return null; + } + + /// SPEC F201.2, STORY-468 AC6 — the backstop for the three shapes + /// AdScriptWriter.ApplyLineAwareHygiene (GenWave.Tts) already strips from an LLM-authored + /// script's text: a script that reaches this validator by ANY other path (owner-typed save, a pack + /// install, a hygiene bug) still refuses if a parenthetical, bracketed beat, or asterisked aside + /// survives in its spoken text. Checked per line, first match wins, naming both the 1-based line + /// number and a bounded echo of the offending run. + static AdScriptViolation? CheckStageDirection(AdScript script) + { + for (var i = 0; i < script.Lines.Count; i++) + { + var match = StageDirectionResiduePattern().Match(script.Lines[i].Text); + if (!match.Success) + continue; + + return new AdScriptViolation( + AdScriptRuleIds.StageDirection, + $"line {i + 1} still carries a stage direction (\"{AdScriptEcho.ForReason(match.Value)}\")"); } return null; } + /// The SAME three shapes AdScriptWriter.ApplyLineAwareHygiene (GenWave.Tts) strips + /// before an LLM-authored script's text ever reaches this validator — duplicated here as a literal + /// pattern because of the L10 boundary (GenWave.Tts cannot reference GenWave.Ads, and Ads does not + /// reference Tts); a Core-level shared shape is the seam if the two ever drift. + /// + /// + /// Each shape must carry at least one LETTER to count (gh-#706 first-contact finding): a stage + /// direction is always a word or words, never a bare digit run — 's + /// own (ddd) ddd-dddd alternative (SPEC F197.1) means a sponsor's own area code can arrive + /// wrapped in real parentheses ("(406) 222-0100"), and that grouping must never trip this + /// check. The letter class ([A-Za-z]) is ASCII-only by design (PLAN T552 review N4): a shape whose only + /// "letters" are non-ASCII (an accented word, a non-Latin script) carries no [A-Za-z] + /// character and so is left alone by this check. + /// + /// + [GeneratedRegex(@"\([^()\n]*[A-Za-z][^()\n]*\)|\[[^\[\]\n]*[A-Za-z][^\[\]\n]*\]|\*[^*\n]*[A-Za-z][^*\n]*\*")] + private static partial Regex StageDirectionResiduePattern(); + static AdScriptViolation? CheckAudiencePosture(IReadOnlyList foldedVariants) { if (FoldedWordListMatcher.FirstMatch(foldedVariants, AdProfanityList.FoldedEntries) is null) diff --git a/src/GenWave.Ads/PhoneShapeCheck.cs b/src/GenWave.Ads/PhoneShapeCheck.cs index 05553e3b..68bb4035 100644 --- a/src/GenWave.Ads/PhoneShapeCheck.cs +++ b/src/GenWave.Ads/PhoneShapeCheck.cs @@ -19,22 +19,23 @@ internal static class PhoneShapeCheck { const string RequiredDigits = "555"; - /// The first phone-shaped run (as it appeared in the raw text) that does not contain - /// 555 AND does not match the digits of , or when - /// every phone-shaped run clears one of those two. Callers check ONE LINE at a time (PLAN T399 - /// review N8) — never a whole script joined into one string — so a digit fragment ending one line - /// can never combine with a digit fragment opening the next into a synthesized run that existed in - /// neither line alone. + /// The first phone-shaped run (as it appeared in the raw text) that violates the rule, or + /// when every run clears it. WITHOUT a real + /// on file the rule is the plain one: a run violates unless it contains 555. WITH one on file + /// the rule tightens (SPEC F199.3, STORY-466 AC6): a run violates unless its OWN digits equal + /// 's digits exactly — a DIFFERENT run that merely contains + /// 555 (e.g. 's own example placeholder surviving hygiene, or any + /// other stray 555 number) is no longer exempt just for containing it. Callers check ONE LINE at a + /// time (PLAN T399 review N8) — never a whole script joined into one string — so a digit fragment + /// ending one line can never combine with a digit fragment opening the next into a synthesized run + /// that existed in neither line alone. /// One line of raw (never folded) script text. /// The owner-sponsor phone skip (SPEC F172.5) — the sponsor's own RAW /// phone number, exactly as it is stored, never pre-digitized by the caller (PLAN T438 ruling: ONE /// normalization point, , rather than the caller and this method each /// stripping punctuation their own way). Blank or whitespace-only is treated the SAME as - /// — no exemption. When it normalizes to a real digit string, a phone-shaped - /// run whose own digits equal it exactly passes even without a 555; every OTHER phone-shaped run in - /// is still checked against the 555 rule as usual. - /// (the default) runs the plain 555 rule with no exemption, unchanged from before this parameter - /// existed. + /// — no exemption, and the plain 555 rule runs unchanged from before this + /// parameter existed. public static string? FindViolation(string rawText, string? allowedPhone = null) { var allowedDigits = string.IsNullOrWhiteSpace(allowedPhone) ? null : DigitsOf(allowedPhone); @@ -42,11 +43,22 @@ internal static class PhoneShapeCheck foreach (Match match in PhoneShape.Regex.Matches(rawText)) { var digits = DigitsOf(match.Value); - if (digits.Length < 7 || digits.Contains(RequiredDigits, StringComparison.Ordinal)) + if (digits.Length < 7) continue; - if (allowedDigits is not null && string.Equals(digits, allowedDigits, StringComparison.Ordinal)) - continue; // The sponsor's own literal number (SPEC F172.5) — everything else still refuses. + if (allowedDigits is not null) + { + // SPEC F199.3: a sponsor phone on file means ONLY that exact number clears this line — + // a run that differs refuses even when it contains 555 (STORY-466 AC6), so hygiene's + // own F199.2 rewrite (or its failure to run at all) stays honestly checked. + if (string.Equals(digits, allowedDigits, StringComparison.Ordinal)) + continue; + + return match.Value.Trim(); + } + + if (digits.Contains(RequiredDigits, StringComparison.Ordinal)) + continue; return match.Value.Trim(); } diff --git a/src/GenWave.Host/Api/AdSpotDto.cs b/src/GenWave.Host/Api/AdSpotDto.cs index 8f481ef4..90ec19ff 100644 --- a/src/GenWave.Host/Api/AdSpotDto.cs +++ b/src/GenWave.Host/Api/AdSpotDto.cs @@ -25,7 +25,10 @@ namespace GenWave.Host.Api; /// (STORY-433; PLAN T457; gh-#745) is the configured render pass /// cadence (Ads:WorkerIntervalMinutes) for a spot in the state, /// and for every other state — the UI uses it to tell the operator roughly when -/// the spot will be picked up, not a guaranteed bound. +/// the spot will be picked up, not a guaranteed bound. (SPEC F200.3; STORY-467; +/// PLAN T551) is one entry per distinct unknown speaker tag carried +/// ("unknown-tag:{TAG}", GenWave.Ads.AdScriptParseNotes.For's own re-parse) — empty, never +/// a validator failure, when every tag was already known or is null/unparseable. /// public sealed record AdSpotDto( long Id, @@ -50,4 +53,5 @@ public sealed record AdSpotDto( string Version, AdSpotJobDto? Job, AdSpotPreviewDto? Preview, - int? RenderWithinMinutes); + int? RenderWithinMinutes, + IReadOnlyList ParseNotes); diff --git a/src/GenWave.Host/Api/AdsController.cs b/src/GenWave.Host/Api/AdsController.cs index 52f5d802..b4364b04 100644 --- a/src/GenWave.Host/Api/AdsController.cs +++ b/src/GenWave.Host/Api/AdsController.cs @@ -1041,13 +1041,17 @@ static SponsorRefDto SponsorRefFor(long sponsorId, Sponsor? sponsor) => /// 's own page-wide dictionary), not merely its wire cross-reference — /// needs it to recompute the staleness key. /// is read ONCE by the caller (PLAN T442 ruling) — never re-read here per row; see - /// 's own remarks for why that hoist matters. + /// 's own remarks for why that hoist matters. + /// ParseNotes re-parses every row + /// (, SPEC F200.3) — goes through this same + /// method per row too; acceptable at admin page sizes (<= 50 rows). AdSpotDto ToDto(AdSpot spot, Sponsor? sponsor, AdLiveSettings liveSettings) => new( spot.Id, spot.SponsorId, spot.SponsorName, SponsorRefFor(spot.SponsorId, sponsor), spot.Title, spot.Brief, spot.Script, AdSourceTokens.ToToken(spot.Source), spot.PackSlug, spot.SpotSeconds, DeserializeVoicePlan(spot.VoicePlan), spot.BedMediaId, AdStateTokens.ToToken(spot.State), spot.FailReason, spot.MediaId, spot.CreatedAt, spot.StateChangedAt, spot.RenderedAt, spot.RetiredAt, spot.Version, - ToJobDto(spot), ToPreviewDto(spot, sponsor, liveSettings), RenderWindowFor(spot)); + ToJobDto(spot), ToPreviewDto(spot, sponsor, liveSettings), RenderWindowFor(spot), + AdScriptParseNotes.For(spot.Script)); /// The configured worker interval for an approved spot, else /// (STORY-433; PLAN T457; gh-#745). diff --git a/src/GenWave.Tts/AdScriptPromptBuilder.cs b/src/GenWave.Tts/AdScriptPromptBuilder.cs index e8686eff..560cd12f 100644 --- a/src/GenWave.Tts/AdScriptPromptBuilder.cs +++ b/src/GenWave.Tts/AdScriptPromptBuilder.cs @@ -83,8 +83,9 @@ public static string BuildSystemPrompt(AdScriptWriteRequest request) "one instead. Any tagline, phone number, address, or website given under \"Sponsor:\" " + "below is a real fact you may speak verbatim - never invent facts beyond what is given " + "there. Any phone number spoken must use the fictional 555 exchange, for example " + - "555-0142, unless the sponsor's real phone number is given under \"Sponsor:\" below, in " + - "which case speak that one instead. No stage directions, no emoji, no markdown formatting."; + $"{ExamplePhone(request.SponsorName)}, unless the sponsor's real phone number is given " + + "under \"Sponsor:\" below, in which case speak that one instead. No stage directions, no " + + "emoji, no markdown formatting."; var postureLine = request.Posture == AudiencePosture.Everyone ? " Keep the language family-friendly." @@ -179,5 +180,36 @@ public static string BuildReaskLine(string ruleId, string reason) => $"Your last draft violated the '{ruleId}' rule: {reason}. Write a new draft that fixes this " + "and obeys every other instruction above."; + /// + /// The fictional example phone cites (SPEC F199.1, STORY-466 AC1/AC2): + /// "555-01" + hash % 100 of the sponsor name trimmed and lower-cased (the sponsor's slug — there + /// is no separate slug column). The hash is , not , + /// which is randomized per process and would change a sponsor's example on every restart. + /// + internal static string ExamplePhone(string sponsorName) + { + var slug = sponsorName.Trim().ToLowerInvariant(); + var lastTwoDigits = Fnv1a32(Encoding.UTF8.GetBytes(slug)) % 100; + return $"555-01{lastTwoDigits:D2}"; + } + + /// FNV-1a, 32-bit, over raw bytes — a small, process-stable, non-cryptographic hash (see + /// 's own remarks for why this exists instead of + /// ). + static uint Fnv1a32(byte[] bytes) + { + const uint FnvOffsetBasis = 2166136261; + const uint FnvPrime = 16777619; + + var hash = FnvOffsetBasis; + foreach (var b in bytes) + { + hash ^= b; + hash *= FnvPrime; + } + + return hash; + } + static string Truncate(string text, int maxChars) => text.Length <= maxChars ? text : text[..maxChars]; } diff --git a/src/GenWave.Tts/AdScriptWriter.cs b/src/GenWave.Tts/AdScriptWriter.cs index f408aed6..5d1b6443 100644 --- a/src/GenWave.Tts/AdScriptWriter.cs +++ b/src/GenWave.Tts/AdScriptWriter.cs @@ -189,6 +189,7 @@ async Task AttemptAsync( } var cleaned = ApplyLineAwareHygiene(reply.Content); + cleaned = ApplyPhoneHygiene(cleaned, request.Phone); if (cleaned.Length == 0) { return Resolved(Failed( @@ -288,7 +289,7 @@ AdScriptWriteResult.Failed Failed( /// reject branches already use (a shape mistake is , a /// length/duration miss is , a content-truth-shaped miss is /// ) rather than flattening every refusal to one bucket. - /// The five rule id tokens are GenWave.Ads.AdScriptRuleIds' own wire vocabulary, duplicated + /// The six rule id tokens are GenWave.Ads.AdScriptRuleIds' own wire vocabulary, duplicated /// here as literal strings — this project cannot reference that one (L10) — mirrors /// AdScriptPromptBuilder's own AnnouncerTag duplication for the identical reason. An /// unrecognized rule id (a rule GenWave.Ads adds later without a matching update here) falls @@ -300,6 +301,7 @@ AdScriptWriteResult.Failed Failed( static LlmCallCause MapRuleIdToCause(string ruleId) => ruleId switch { "format" => LlmCallCause.MalformedResponse, + "stage_direction" => LlmCallCause.MalformedResponse, "duration" => LlmCallCause.OverLength, "brand_collision" => LlmCallCause.TruthGateReject, "phone_shape" => LlmCallCause.TruthGateReject, @@ -366,10 +368,27 @@ static string BoundReason(string reason) /// /// Blank interior lines are dropped (the AdScriptParser.Parse/CrosstalkScriptParser.Parse /// precedent: accidental double-spacing between beats is a formatting quirk, never a shape - /// violation). A line whose text is empty after hygiene keeps its own bare TAG: (never - /// silently dropped whole) — so AdScriptValidator reports the honest, specific "the {tag} - /// line has no spoken text" reason rather than a misleading "no {tag} line appeared" for a line that - /// DID arrive, just empty — unless an untagged continuation line follows and fills it (below). + /// violation). Every line's TEXT also has its own stage directions stripped, ANYWHERE in the text + /// and not only inside the tag (SPEC F201.1, STORY-468): a (parenthetical), a + /// [bracketed] beat, or an *asterisked* aside — each required to carry at least one + /// LETTER, so a sponsor's own (406) 222-0100-shaped phone number never loses its area code + /// to this pass — is removed whole, BEFORE ever runs + /// on what remains, so a multi-word aside like *long pause* never survives as spoken words + /// the way a bare emphasis-mark strip alone would leave it. A line where a shape actually matched + /// has its surrounding whitespace collapsed and a space left dangling before trailing punctuation + /// tidied away; a line where nothing matched is returned byte-for-byte untouched, so this pass never + /// rewrites legitimate copy that merely contains an ellipsis, a deliberately spaced colon, or a + /// stray space near punctuation of its own (PLAN T552 review F1). + /// + /// + /// + /// A line whose text is STILL empty once continuation-joining (above) has had its own chance to + /// fill it is dropped WHOLE — before the "nobody is ANNOUNCER" election below ever runs, so an + /// emptied line can never cast a vote for its own tag (F201.1, PLAN T552 review F2) — and never + /// surfaced as a bare TAG: the way it was before STORY-468. AdScriptValidator's own + /// stage-direction rule (SPEC F201.2) is the backstop that NAMES any of the same three shapes a + /// script still carries after this pass, never this writer silently forwarding an empty line for the + /// validator to explain. /// /// /// @@ -419,7 +438,7 @@ internal static string ApplyLineAwareHygiene(string raw) text = line; } - var cleanedText = LlmCopyWriter.ApplyCopyHygiene(text); + var cleanedText = LlmCopyWriter.ApplyCopyHygiene(StripStageDirections(text)); if (tag.Length == 0) { // No speaker: continuation prose joins the previous voice's line (filling a bare tag); @@ -432,6 +451,13 @@ internal static string ApplyLineAwareHygiene(string raw) lines.Add((tag, cleanedText)); } + // F201.1 — a line whose text is STILL empty once continuation-joining above has had its own + // chance to fill it is dropped WHOLE here, BEFORE the "nobody is ANNOUNCER" election below + // reads lines.Count/lines.GroupBy: an emptied line (e.g. a line that was pure stage direction) + // must never cast a vote for its own tag, and must never be surfaced as a bare "TAG:" (STORY-468 + // AC5, PLAN T552 review F2). + lines.RemoveAll(l => l.Text.Length == 0); + if (lines.Count > 0 && lines.TrueForAll(l => l.Tag != AdScriptPromptBuilder.AnnouncerTag)) { var lead = lines @@ -446,9 +472,215 @@ internal static string ApplyLineAwareHygiene(string raw) } } - return string.Join('\n', lines.Select(l => l.Text.Length == 0 ? $"{l.Tag}:" : $"{l.Tag}: {l.Text}")); + // Emptied lines are already gone (removed above, before the election). A script that empties + // entirely falls out of this Join as string.Empty — AdScriptValidator's existing "the script + // has no lines" refusal (AdScriptParser.Parse) handles that case. + return string.Join('\n', lines.Select(l => $"{l.Tag}: {l.Text}")); + } + + /// + /// SPEC F201.1, STORY-468 — strips a (parenthetical), a [bracketed] beat, and an + /// *asterisked* aside ANYWHERE in a line's text (not only when the shape wraps the whole + /// line), collapses the whitespace the removal leaves behind, and tidies a space stranded before + /// trailing punctuation. Run BEFORE so a multi-word + /// aside like *long pause* is removed whole — 's + /// own asterisk strip only catches a SINGLE-word run, then falls back to stripping the bare + /// */_ marks and leaving the words themselves spoken. + /// + /// + /// Each shape must carry at least one LETTER to count (gh-#706 first-contact finding): a stage + /// direction is always a word or words, never a bare digit run — 's + /// own (ddd) ddd-dddd alternative (SPEC F197.1) means a sponsor's own area code can arrive + /// wrapped in real parentheses ("(406) 222-0100"), and that grouping must survive THIS pass + /// untouched for (run after this method) to ever see it. The letter + /// class ([A-Za-z]) is ASCII-only by design (PLAN T552 review N4): a shape whose only "letters" are non-ASCII + /// (an accented word, a non-Latin script) carries no [A-Za-z] character and so is left alone by + /// this pass. + /// + /// + static string StripStageDirections(string text) + { + var stripped = StageDirectionParentheticalPattern().Replace(text, string.Empty); + stripped = StageDirectionBracketPattern().Replace(stripped, string.Empty); + stripped = StageDirectionAsteriskPattern().Replace(stripped, string.Empty); + + // PLAN T552 review F1: the shapes above only ever REMOVE characters, so an unchanged length means + // no shape matched — return the ORIGINAL text untouched rather than running the collapse/tidy + // passes below, which exist solely to repair the gap a real removal leaves behind. Running them + // unconditionally rewrote legitimate copy that never had a stage direction in it at all: an + // ellipsis ("wait ... then go") collapsed to "wait... then go", and a colon/period with + // deliberate spacing ("Remember : call now", "3 . 5 dollars") lost its spacing. + if (stripped.Length == text.Length) + return text; + + stripped = CollapseStrippedGapPattern().Replace(stripped, " ").Trim(); + return SpaceBeforePunctuationPattern().Replace(stripped, "$1"); + } + + [GeneratedRegex(@"\([^()\n]*[A-Za-z][^()\n]*\)")] + private static partial Regex StageDirectionParentheticalPattern(); + + [GeneratedRegex(@"\[[^\[\]\n]*[A-Za-z][^\[\]\n]*\]")] + private static partial Regex StageDirectionBracketPattern(); + + [GeneratedRegex(@"\*[^*\n]*[A-Za-z][^*\n]*\*")] + private static partial Regex StageDirectionAsteriskPattern(); + + [GeneratedRegex(@"\s{2,}")] + private static partial Regex CollapseStrippedGapPattern(); + + /// A stripped shape can leave a lone space stranded just before the punctuation that + /// followed it ("today [beat]." strips to "today .") — folded back against that + /// punctuation so the sentence reads "today.", never "today .". + [GeneratedRegex(@"\s+([.,;:!?])")] + private static partial Regex SpaceBeforePunctuationPattern(); + + /// + /// SPEC F199.2 hygiene, run AFTER on its already-tagged "TAG: + /// text" output: a phone-shaped digit run that is not the sponsor's own number is rewritten to the + /// sponsor's own number verbatim (STORY-466 AC3, F199.4 pinned); a sponsor with no + /// on file has the number AND its clause dropped instead (AC5). A + /// run whose DIGITS already equal the sponsor's own — even when formatted differently (parens vs + /// dashes) or itself containing "555" — is left exactly as written, its own formatting untouched + /// (AC4). Never touches a line's TAG, only the text after its colon. + /// + internal static string ApplyPhoneHygiene(string script, string? sponsorPhone) + { + var trimmedPhone = string.IsNullOrWhiteSpace(sponsorPhone) ? null : sponsorPhone.Trim(); + var phoneDigits = trimmedPhone is null ? null : DigitsOnly(trimmedPhone); + + var lines = new List(); + foreach (var line in script.Split('\n')) + { + var hygienic = ApplyPhoneHygieneToLine(line, trimmedPhone, phoneDigits); + if (hygienic is not null) + lines.Add(hygienic); + } + + return string.Join('\n', lines); + } + + /// Splits "TAG: text" at its first colon (the exact shape + /// always produces) and hygienes only the text half. A line whose text hygiene empties entirely + /// (AC5's "the clause goes" when nothing is left) is dropped — the whole line, tag included — by + /// returning ; a line with no text to begin with (a bare "TAG:") is returned + /// unchanged, never dropped, since emptiness there predates this pass. + static string? ApplyPhoneHygieneToLine(string line, string? sponsorPhone, string? sponsorDigits) + { + var colonIndex = line.IndexOf(':'); + if (colonIndex < 0) + return line; // never produced by ApplyLineAwareHygiene, but never mangled if it happens + + var tag = line[..colonIndex]; + var body = line[(colonIndex + 1)..]; + var text = body.Length > 0 && body[0] == ' ' ? body[1..] : body; + if (text.Length == 0) + return line; + + var cleanedText = ApplyPhoneHygieneToText(text, sponsorPhone, sponsorDigits); + return cleanedText.Length == 0 ? null : $"{tag}: {cleanedText}"; + } + + static string ApplyPhoneHygieneToText(string text, string? sponsorPhone, string? sponsorDigits) + { + var current = text; + var scanFrom = 0; + + while (true) + { + var match = PhoneShapedRunPattern().Match(current, scanFrom); + if (!match.Success) + return current; + + var runDigits = DigitsOnly(match.Value); + if (sponsorDigits is not null && string.Equals(runDigits, sponsorDigits, StringComparison.Ordinal)) + { + // AC4 — the sponsor's own number (even one that itself contains "555"), unchanged; + // advance past it so the next search never re-matches the same run. + scanFrom = match.Index + match.Length; + continue; + } + + if (sponsorPhone is not null) + { + // AC3, F199.4 pinned — replace with the sponsor's own number verbatim. Resume scanning + // just PAST the inserted replacement rather than resetting to 0: the text before it is + // already resolved, and there is nothing left to verify about what we just wrote. + current = ReplacePhoneRun(current, match, sponsorPhone); + scanFrom = match.Index + sponsorPhone.Length; + continue; + } + + // AC5 — no phone on file: the run and its clause are dropped, never replaced with anything + // phone-shaped, so a rescan from 0 can never re-trigger on our own output the way the + // replace branch above could. + current = DropPhoneClause(current, match.Index, match.Index + match.Length); + scanFrom = 0; + } + } + + static string ReplacePhoneRun(string text, Match match, string replacement) => + string.Concat(text.AsSpan(0, match.Index), replacement, text.AsSpan(match.Index + match.Length)); + + /// Deletes from the nearest preceding clause boundary through the matched run. The boundary + /// char itself (one of ) goes too — it only led into the dropped + /// clause ("Cravin's Diner, 555-0142." → "Cravin's Diner."). Any leading run of + /// boundary punctuation left on the remainder is stripped, so a line that was nothing but the phone + /// clause drops to rather than a bare "." (AC5); whitespace is collapsed + /// and, when the deletion reached the line start, the new first letter is capitalized (the F199.4 + /// pin). reads an empty result as "drop the whole line". + static string DropPhoneClause(string text, int matchStart, int matchEnd) + { + var boundary = 0; + for (var i = matchStart - 1; i >= 0; i--) + { + if (Array.IndexOf(ClauseBoundaryChars, text[i]) < 0) + continue; + boundary = i; + break; + } + + var joined = LeadingClauseBoundaryPattern().Replace(text[..boundary] + text[matchEnd..], string.Empty); + var remaining = CollapseWhitespacePattern().Replace(joined, " ").Trim(); + if (remaining.Length == 0) + return string.Empty; + + return boundary == 0 ? char.ToUpperInvariant(remaining[0]) + remaining[1..] : remaining; } + /// Punctuation that ends a clause (never a phone-run separator itself — those are + /// narrowly -/./space inside 's own digit groups). + /// + static readonly char[] ClauseBoundaryChars = ['.', ',', ';', ':', '!', '?']; + + static string DigitsOnly(string text) => new(text.Where(char.IsAsciiDigit).ToArray()); + + // NANP-shaped digit runs only (optional area code, then a 3-4 local grouping) — deliberately NOT + // a bare "555-\d{4}" match: the sponsor's own phone (e.g. "812-555-0199") contains "555-0199" as a + // substring, and matching only that tail would rewrite it to "812-812-555-0199". The optional + // area-code alternative is tried FIRST (.NET's default greedy order), which is what makes a full + // "812-555-0199" match as ONE run instead of splitting off its own "555-0199" tail. + // + // \b sits AFTER the optional leading paren, not before it (the PhoneShapeCheck.FindViolation + // precedent, GenWave.Ads — its own remarks explain the same fix): a paren is itself a non-word + // character, so a \b placed before it never finds a word/non-word transition when the paren is + // actually present (space-then-paren is non-word-to-non-word) — a leading \b there would make the + // paren alternative unreachable, and every "(NNN) NNN-NNNN" run would match only its own trailing + // "NNN-NNNN" tail. + [GeneratedRegex(@"(?:\(\d{3}\)\s?|\b\d{3}[-. ])?\b\d{3}[-. ]\d{4}\b")] + private static partial Regex PhoneShapedRunPattern(); + + [GeneratedRegex(@"\s+")] + private static partial Regex CollapseWhitespacePattern(); + + /// A leading run of and/or whitespace — literally those + /// six characters, kept in sync by hand since GeneratedRegex needs a compile-time constant + /// and cannot read the array. What strips from the very front of its + /// own joined remainder, so an orphaned separator (or a bare terminal "." with nothing left to + /// terminate) never survives as the whole "sentence". + [GeneratedRegex(@"^[.,;:!?\s]+")] + private static partial Regex LeadingClauseBoundaryPattern(); + /// A raw tag longer than this is a sentence with a colon in it, never a voice. const int MaxRawTagChars = 40; diff --git a/tests/GenWave.Ads.Tests/Specs/Story390_AdScriptValidator.cs b/tests/GenWave.Ads.Tests/Specs/Story390_AdScriptValidator.cs index 86eac092..759e63ec 100644 --- a/tests/GenWave.Ads.Tests/Specs/Story390_AdScriptValidator.cs +++ b/tests/GenWave.Ads.Tests/Specs/Story390_AdScriptValidator.cs @@ -284,12 +284,16 @@ public void AHistoricalEstimateShorterThanTheTextTermDoesNotShrinkTheTotal() public sealed class ScenarioFormatRefuses { [Fact] - public void FourVoiceTagsRefuse() + public void AFourthTagFoldsRatherThanExceedingTheMax() { + // SPEC F200.1/F200.2 (STORY-467) supersedes this fact's original premise: the known cast + // is exactly ANNOUNCER/VOICE1/VOICE2 (3 tags, the same as MaxVoiceTags), so a fourth, + // TagPattern-shaped tag like VOICE3 is not in the cast and folds onto ANNOUNCER instead of + // adding a 4th distinct tag — the over-max branch of the format rule is therefore no longer + // reachable through unknown-tag growth; it stays Accepted (3 distinct tags after folding). var result = Validate("ANNOUNCER: one.\nVOICE1: two.\nVOICE2: three.\nVOICE3: four."); - var refused = Assert.IsType(result); - Assert.Equal(AdScriptRuleIds.Format, refused.Violation.RuleId); + Assert.IsType(result); } [Fact] @@ -489,8 +493,15 @@ public sealed class ScenarioFirstRuleWinsIsDeterministic [Fact] public void AFormatAndDurationViolationNamesFormatFirst() { + // SPEC F200.2 (STORY-467) retired the original "4 distinct tags" fixture here — a 4th + // TagPattern-shaped tag now folds onto ANNOUNCER instead of adding a distinct format + // violation (see AFourthTagFoldsRatherThanExceedingTheMax above). "Missing ANNOUNCER" is + // still a genuine, reachable format violation, so it stands in: this script is both + // missing the required ANNOUNCER voice AND, by its filler length, would also overrun + // duration — Format still wins, because AdScriptParser.Parse (format) runs before + // AdScriptValidator ever evaluates duration. var filler = new string('x', 180); - var result = Validate($"ANNOUNCER: {filler}\nVOICE1: {filler}\nVOICE2: {filler}\nVOICE3: {filler}"); + var result = Validate($"VOICE1: {filler}\nVOICE2: {filler}\nVOICE1: {filler}\nVOICE2: {filler}"); var refused = Assert.IsType(result); Assert.Equal(AdScriptRuleIds.Format, refused.Violation.RuleId); diff --git a/tests/GenWave.Ads.Tests/Specs/Story391_AdRenderService.cs b/tests/GenWave.Ads.Tests/Specs/Story391_AdRenderService.cs index b7d922b3..a47dee35 100644 --- a/tests/GenWave.Ads.Tests/Specs/Story391_AdRenderService.cs +++ b/tests/GenWave.Ads.Tests/Specs/Story391_AdRenderService.cs @@ -225,8 +225,10 @@ public async Task AnUnparseableScriptFailsWithoutEverReachingTheAuthor() var (service, author, store, _, _, _) = Build(); // A bare sentence is a legal one-line script (SPEC F174.6) — an unparseable fixture here // needs a violation the plain-sentence pre-pass cannot absorb: a tagged script with no - // ANNOUNCER line (PLAN T444 ruling). - var spot = MakeSpot(id: 2, script: "GUEST: no announcer line at all"); + // ANNOUNCER line (PLAN T444 ruling). The tag must be a KNOWN one (VOICE1) — SPEC F200.1 + // (STORY-467) folds any unrecognized tag like the former "GUEST" onto ANNOUNCER, which + // would make this script parse successfully instead of refusing. + var spot = MakeSpot(id: 2, script: "VOICE1: no announcer line at all"); await service.RenderAsync(spot, LiveSettings(), CancellationToken.None); diff --git a/tests/GenWave.Ads.Tests/Specs/Story417_OwnerSponsorsAreReal.cs b/tests/GenWave.Ads.Tests/Specs/Story417_OwnerSponsorsAreReal.cs index 29000406..4f48d8b2 100644 --- a/tests/GenWave.Ads.Tests/Specs/Story417_OwnerSponsorsAreReal.cs +++ b/tests/GenWave.Ads.Tests/Specs/Story417_OwnerSponsorsAreReal.cs @@ -76,6 +76,10 @@ public void AFormattingDifferenceFromTheSponsorsPhoneStillPasses() } } + // SPEC F199.3 (STORY-466): every filler phone line below moved from a plain "555-0100" placeholder + // to the sponsor's OWN {SponsorPhone} — OwnerRequest puts a real sponsor phone on file, and F199.3 + // tightened the phone-shape skip to "only that exact number", so a differing 555 filler would now + // itself refuse on phone_shape before ever reaching the brand check these facts exist to exercise. public sealed class ScenarioOwnerSponsorsOwnNamePassesTheBlocklist { [Fact] @@ -83,7 +87,7 @@ public void AScriptNamingTheSponsorPasses() { const string script = $"ANNOUNCER: {SponsorName} has a deal so good it's almost illegal.\n" + - "ANNOUNCER: Call 555-0100 today."; + $"ANNOUNCER: Call {SponsorPhone} today."; var result = Validate(script, OwnerRequest); @@ -98,7 +102,7 @@ public void AScriptNamingTheSponsorTwiceSeparatedOnlyByPunctuationPasses() { const string script = $"ANNOUNCER: Fresh every day at {SponsorName}. {SponsorName}, on Main Street.\n" + - "ANNOUNCER: Call 555-0100 today."; + $"ANNOUNCER: Call {SponsorPhone} today."; var result = Validate(script, OwnerRequest); @@ -114,7 +118,7 @@ public void AScriptNamingTheSponsorTwiceAcrossALineBreakPasses() { const string script = $"ANNOUNCER: {SponsorName}\n" + - $"ANNOUNCER: {SponsorName} — call 555-0100 today."; + $"ANNOUNCER: {SponsorName} — call {SponsorPhone} today."; var result = Validate(script, OwnerRequest); @@ -130,7 +134,7 @@ public void TheSponsorsNameGluedBetweenTwoOtherWordsNeverFabricatesACollision() { const string script = $"ANNOUNCER: Mountain {SponsorName} Dew has a deal so good it's almost illegal.\n" + - "ANNOUNCER: Call 555-0100 today."; + $"ANNOUNCER: Call {SponsorPhone} today."; var result = Validate(script, OwnerRequest); diff --git a/tests/GenWave.Ads.Tests/Specs/Story466_TheValidatorStillRefusesAStray555.cs b/tests/GenWave.Ads.Tests/Specs/Story466_TheValidatorStillRefusesAStray555.cs new file mode 100644 index 00000000..64753a29 --- /dev/null +++ b/tests/GenWave.Ads.Tests/Specs/Story466_TheValidatorStillRefusesAStray555.cs @@ -0,0 +1,51 @@ +// STORY-466 — The example phone never airs (validator half: AC6 · SPEC F199.3 · PLAN T550) +// +// AC1–AC5 live in tests/GenWave.Tts.Tests/Specs/Story466_TheExamplePhoneNeverAirs.cs (AC3–AC5 drive +// AdScriptWriter.ApplyPhoneHygiene directly). AC6 needs the REAL GenWave.Ads.AdScriptValidator — a +// hygiene bug (or a script that skipped hygiene entirely, e.g. an owner-typed save) must still be +// caught here, never silently aired — and GenWave.Tts.Tests does not (and must not) reference +// GenWave.Ads, so this one fact lives on this side of the L10 boundary instead. + +using GenWave.Ads.Tests.Fakes; +using GenWave.Core.Domain; + +namespace GenWave.Ads.Tests.Specs; + +public static class FeatureTheValidatorStillRefusesAStray555 +{ + static readonly AdScriptValidationRequest Request = new( + Posture: AudiencePosture.Everyone, MaxLineChars: 200, SpotSeconds: 30, ToleranceRatio: 0.4, + SponsorPhone: "812-555-0142", IsPackOwned: false); + + public sealed class ScenarioAStrayNumberAfterHygiene + { + // Given: a post-hygiene script carrying "555-0199" — not the sponsor's own "812-555-0142" on + // file — and F160 validation (SPEC F199.3: a real sponsor phone tightens the 555 skip to ONLY + // that exact number, so a differing run refuses even though it itself contains "555") + const string Script = + "ANNOUNCER: Cravin's Diner has a deal so good it's almost illegal.\n" + + "ANNOUNCER: Call 555-0199 today."; + + readonly AdScriptValidationResult result = + AdScriptValidator.Validate(Script, Request, new FakePatterDurationEstimator()); + + /// AC6 — the phone rule fails + [Fact] + public void StillFailsTheValidator() + { + var refused = Assert.IsType(result); + Assert.Equal(AdScriptRuleIds.PhoneShape, refused.Violation.RuleId); + } + + // PLAN T552 review N3: with a real sponsor phone on file, "does not contain 555" went false the + // moment F199.3 tightened the skip to ONLY the sponsor's own exact number — this run refuses + // even though it itself contains "555", so the reason must say WHY honestly instead of repeating + // a claim that is no longer true. + [Fact] + public void TheReasonNamesTheSponsorsOwnNumberNotThe555Claim() + { + var refused = Assert.IsType(result); + Assert.Contains("sponsor's own number", refused.Violation.Reason); + } + } +} diff --git a/tests/GenWave.Ads.Tests/Specs/Story467_UnknownSpeakerTagsFoldIntoTheAnnouncer.cs b/tests/GenWave.Ads.Tests/Specs/Story467_UnknownSpeakerTagsFoldIntoTheAnnouncer.cs index 12cf9944..52f82435 100644 --- a/tests/GenWave.Ads.Tests/Specs/Story467_UnknownSpeakerTagsFoldIntoTheAnnouncer.cs +++ b/tests/GenWave.Ads.Tests/Specs/Story467_UnknownSpeakerTagsFoldIntoTheAnnouncer.cs @@ -1,38 +1,87 @@ // STORY-467 — Unknown speaker tags fold into the announcer (gh-#742 · SPEC F200 · PLAN T551) // -// BDD specification — xUnit. RED at plan time: every fact is [Fact(Skip = Pending)] with a loud body — -// remove the Skip only in the task that makes it green. Each Given comment names the arrange the scenario needs. +// BDD specification — xUnit. GREEN: AdScriptParser.Parse folds any TagPattern-shaped tag that is not +// in the known cast (ANNOUNCER/VOICE1/VOICE2) onto ANNOUNCER, keeping the line's copy, and records one +// AdScript.Notes entry per distinct unknown tag. AC5 (the API's own parseNotes field) is specced +// separately in GenWave.Host.Tests/Specs/Story467_ParseNotesReachTheAdDetail.cs. + +using GenWave.Ads.Tests.Fakes; +using GenWave.Core.Domain; namespace GenWave.Ads.Tests.Specs; public static class FeatureUnknownspeakertagsfoldintotheannouncer { - const string Pending = "pending: T551 — Unknown speaker tags fold into the announcer (STORY-467)"; + // --------------------------------------------------------------------- + // Helpers + // --------------------------------------------------------------------- + + // STORY-467 ruling: AC1's own wording writes the second tag as "VOICE", shorthand for the cast tag + // VOICE1 — the known cast is exactly ANNOUNCER/VOICE1/VOICE2 (AdCastPicker.Voice1Tag/Voice2Tag; + // AdScriptPromptBuilder tells the writing model those three), so this fixture spells it VOICE1 + // verbatim rather than adding a bare "VOICE" to the cast. + const string NarratorLineScript = + "ANNOUNCER: Cravin's Diner has a deal so good it's almost illegal.\n" + + "VOICE1: Almost. Stop by tonight.\n" + + "NARRATOR: In a world of ordinary diners..."; + + static readonly AdScriptValidationRequest DefaultRequest = new( + Posture: AudiencePosture.Everyone, MaxLineChars: 200, SpotSeconds: 30, ToleranceRatio: 0.4); + + // --------------------------------------------------------------------- + // HAPPY PATH + // --------------------------------------------------------------------- public sealed class ScenarioAScriptWithANarratorLine { - // Given: ANNOUNCER, VOICE, NARRATOR lines through AdScriptParser + // Given: ANNOUNCER, VOICE1, NARRATOR lines through AdScriptParser (STORY-467 ruling above) + readonly AdScriptValidationResult.Accepted accepted; + + public ScenarioAScriptWithANarratorLine() + { + var result = AdScriptParser.Parse(NarratorLineScript, maxLineChars: 200); + accepted = Assert.IsType(result); + } /// AC1 — the NARRATOR copy is attributed to ANNOUNCER - [Fact(Skip = Pending)] - public void KeepsTheLine() => Assert.Fail(Pending); + [Fact] + public void KeepsTheLine() => + Assert.Contains( + accepted.Script.Lines, + line => line.Tag == AdScriptParser.AnnouncerTag && line.Text == "In a world of ordinary diners..."); /// AC2 — note "unknown-tag:NARRATOR" - [Fact(Skip = Pending)] - public void NamesTheTag() => Assert.Fail(Pending); + [Fact] + public void NamesTheTag() => + Assert.Contains("unknown-tag:NARRATOR", accepted.Script.Notes); /// AC3 — distinct voice count is 2 - [Fact(Skip = Pending)] - public void CountsKnownTagsOnly() => Assert.Fail(Pending); + [Fact] + public void CountsKnownTagsOnly() => + Assert.Equal(2, accepted.Script.Lines.Select(line => line.Tag).Distinct().Count()); } public sealed class ScenarioAScriptThatIsAllNarrator { // Given: every line NARRATOR + const string Script = + "NARRATOR: In a world of ordinary diners...\n" + + "NARRATOR: One stands apart."; + + readonly AdScriptValidationResult result; + + public ScenarioAScriptThatIsAllNarrator() + { + result = AdScriptParser.Parse(Script, maxLineChars: 200); + } /// AC4 — a valid one-voice script - [Fact(Skip = Pending)] - public void IsOneVoice() => Assert.Fail(Pending); + [Fact] + public void IsOneVoice() + { + var accepted = Assert.IsType(result); + Assert.Equal(["ANNOUNCER", "ANNOUNCER"], accepted.Script.Lines.Select(line => line.Tag)); + } } // --------------------------------------------------------------------- @@ -41,11 +90,17 @@ public sealed class ScenarioAScriptThatIsAllNarrator public sealed class ScenarioTheValidator { - // Given: the script of AC1 through F160 + // Given: the script of AC1 through F160 (AdScriptValidator.Validate, not the bare parser) + readonly AdScriptValidationResult result; + + public ScenarioTheValidator() + { + result = AdScriptValidator.Validate(NarratorLineScript, DefaultRequest, new FakePatterDurationEstimator()); + } /// AC6 — no rule fails for the tag - [Fact(Skip = Pending)] - public void IsNotAValidatorFailure() => Assert.Fail(Pending); + [Fact] + public void IsNotAValidatorFailure() => + Assert.IsType(result); } - } diff --git a/tests/GenWave.Ads.Tests/Specs/Story468_ResidueFailsValidation.cs b/tests/GenWave.Ads.Tests/Specs/Story468_ResidueFailsValidation.cs new file mode 100644 index 00000000..97004e91 --- /dev/null +++ b/tests/GenWave.Ads.Tests/Specs/Story468_ResidueFailsValidation.cs @@ -0,0 +1,41 @@ +// STORY-468 — Stage directions never reach the voice (validator half: AC6 · SPEC F201.2 · PLAN T552) +// +// AC1–AC5 live in tests/GenWave.Tts.Tests/Specs/Story468_StageDirectionsNeverReachTheVoice.cs (they +// drive AdScriptWriter.ApplyLineAwareHygiene directly). AC6 needs the REAL GenWave.Ads.AdScriptValidator +// — a hygiene bug, or a script that skipped hygiene entirely (e.g. an owner-typed save), must still be +// caught here, never silently aired — and GenWave.Tts.Tests does not (and must not) reference +// GenWave.Ads, so this one fact lives on this side of the L10 boundary instead. + +using GenWave.Ads.Tests.Fakes; +using GenWave.Core.Domain; + +namespace GenWave.Ads.Tests.Specs; + +public static class FeatureResidueFailsValidation +{ + static readonly AdScriptValidationRequest Request = + new(Posture: AudiencePosture.Everyone, MaxLineChars: 200, SpotSeconds: 30, ToleranceRatio: 0.4); + + public sealed class ScenarioResidueAfterHygiene + { + // Given: post-hygiene script text still carrying "(beat)" — a shape ApplyLineAwareHygiene + // should have already stripped, reaching this validator anyway (a hygiene bug, or a script + // that never passed through hygiene at all). + const string Script = + "ANNOUNCER: Cravin's Diner has a deal so good it's almost illegal.\n" + + "ANNOUNCER: Call now (beat) before it's gone."; + + readonly AdScriptValidationResult result = + AdScriptValidator.Validate(Script, Request, new FakePatterDurationEstimator()); + + /// AC6 — rule "stage_direction" names the line + [Fact] + public void FailsValidation() + { + var refused = Assert.IsType(result); + Assert.Equal(AdScriptRuleIds.StageDirection, refused.Violation.RuleId); + Assert.Contains("line 2", refused.Violation.Reason, StringComparison.Ordinal); + Assert.Contains("(beat)", refused.Violation.Reason, StringComparison.Ordinal); + } + } +} diff --git a/tests/GenWave.Host.Tests/Specs/Story467_ParseNotesReachTheAdDetail.cs b/tests/GenWave.Host.Tests/Specs/Story467_ParseNotesReachTheAdDetail.cs index 8bc5a5a4..117b1f96 100644 --- a/tests/GenWave.Host.Tests/Specs/Story467_ParseNotesReachTheAdDetail.cs +++ b/tests/GenWave.Host.Tests/Specs/Story467_ParseNotesReachTheAdDetail.cs @@ -1,21 +1,106 @@ // STORY-467 — Parse notes reach the ad detail (gh-#742 · SPEC F200.3 · PLAN T551) // -// BDD specification — xUnit. RED at plan time: every fact is [Fact(Skip = Pending)] with a loud body — -// remove the Skip only in the task that makes it green. Each Given comment names the arrange the scenario needs. +// BDD specification — xUnit through the deployed entry point (WebApplicationFactory against +// a real ephemeral Postgres — the Story392_AdsApi.cs/Story435_FailedJobKind.cs arc idiom): POST +// /api/ads with a script carrying a NARRATOR line (an unknown speaker tag, SPEC F200.1), then GET +// /api/ads/{id} and read the wire's own parseNotes field — never AdsController/AdScriptParseNotes +// directly. + +using System.Net; +using System.Net.Http.Json; +using Microsoft.AspNetCore.Hosting; +using Microsoft.AspNetCore.Mvc.Testing; +using Microsoft.AspNetCore.TestHost; +using Microsoft.Extensions.DependencyInjection.Extensions; +using Microsoft.Extensions.Hosting; +using GenWave.Host.Tests.Support; namespace GenWave.Host.Tests.Specs; public static class FeatureParsenotesreachtheaddetail { - const string Pending = "pending: T551 — Parse notes reach the ad detail (STORY-467)"; - - public sealed class ScenarioGetAdById + [Collection(Story467Collection.Name)] + public sealed class ScenarioGetAdById(Story467Arc arc) { // Given: WebApplicationFactory, an ad whose script carried a NARRATOR line, GET /api/ads/{id} /// AC5 — parseNotes contains the note - [Fact(Skip = Pending)] - public void ServesTheNote() => Assert.Fail(Pending); + [Fact] + public void ServesTheNote() => + Assert.Contains("unknown-tag:NARRATOR", arc.ParseNotes); } +} + +[CollectionDefinition(Name)] +public sealed class Story467Collection : ICollectionFixture +{ + public const string Name = "Story467ParseNotesReachTheAdDetail"; +} + +public sealed class Story467Arc : IAsyncLifetime +{ + const string NarratorLineScript = + "ANNOUNCER: Cravin's Diner has a deal so good it's almost illegal.\n" + + "VOICE1: Almost. Stop by tonight.\n" + + "NARRATOR: In a world of ordinary diners..."; + public IReadOnlyList ParseNotes { get; private set; } = []; + + public async Task InitializeAsync() + { + await using var database = await Story467Database.StartAsync(); + await using var factory = new Story467WebFactory(database); + var client = factory.CreateClient(); + var login = await client.PostAsJsonAsync( + "/api/auth/login", new { password = Story467WebFactory.Password }); + if (login.StatusCode != HttpStatusCode.NoContent) + throw new InvalidOperationException($"login unexpectedly returned {login.StatusCode}"); + + var sponsorId = await AdSpotJobTestHelpers.CreateSponsorAsync(client, "Narrator Line Sponsor"); + var (spotId, _) = await AdSpotJobTestHelpers.CreateDraftSpotWithScriptAsync( + client, sponsorId, "Narrator line spot", NarratorLineScript); + + var body = await AdSpotJobTestHelpers.GetSpotAsync(client, spotId); + ParseNotes = body.GetProperty("parseNotes").EnumerateArray().Select(note => note.GetString() ?? "").ToList(); + } + + public Task DisposeAsync() => Task.CompletedTask; +} + +file sealed class Story467WebFactory(EphemeralStationDatabase db) : WebApplicationFactory +{ + public const string Password = "test-password-t551-parse-notes"; + + protected override void ConfigureWebHost(IWebHostBuilder builder) + { + builder.UseEnvironment("Development"); + builder.UseSetting("ConnectionStrings:Library", db.LibraryConnectionString); + builder.UseSetting("ConnectionStrings:Station", db.StationConnectionString); + builder.UseSetting("Admin:Password", Password); + builder.UseSetting("Station:Id", "genwave-1"); + builder.UseSetting("Station:Name", "GWAV 108.8"); + builder.UseSetting("Station:Voice", "af_heart"); + builder.UseSetting("Station:Scope:LibraryIds:0", "1"); + + builder.ConfigureTestServices(services => + { + services.RemoveAll(); + }); + } +} + +file sealed class Story467Database : EphemeralStationDatabase +{ + Story467Database(string project, string composeFile, string libraryConnectionString, string stationConnectionString) + : base(project, composeFile, libraryConnectionString, stationConnectionString) + { + } + + public static async Task StartAsync() + { + var (project, composeFile, library, station) = Provision("genwave-t551"); + var db = new Story467Database(project, composeFile, library, station); + await db.WaitForSchemaAsync(); + return db; + } } diff --git a/tests/GenWave.Host.Tests/Support/AdScriptCompletionsRouter.cs b/tests/GenWave.Host.Tests/Support/AdScriptCompletionsRouter.cs index a124f8d6..c441a58f 100644 --- a/tests/GenWave.Host.Tests/Support/AdScriptCompletionsRouter.cs +++ b/tests/GenWave.Host.Tests/Support/AdScriptCompletionsRouter.cs @@ -28,11 +28,15 @@ internal sealed class AdScriptCompletionsRouter /// A short two-voice script that passes the real AdScriptValidator for a 30s spot /// (the AdSpotWorkerHarness.WellFormedReply/Story412's own short ANNOUNCER/VOICE1 precedent) /// — proven end to end against the real RollingPatterDurationEstimator, never a fake - /// duration estimator. + /// duration estimator. Deliberately carries NO phone-shaped digit run (PLAN T550, SPEC F199.2): + /// Story423's own facts assert this reply survives onto the row byte for byte, and every sponsor + /// this router serves is phone-less, so a placeholder "555-…" number here would now be hygiened + /// (its clause dropped) by the real AdScriptWriter.ApplyPhoneHygiene the write pipeline + /// runs — breaking that exact-equality promise for a reason unrelated to what Story423 tests. public const string WellFormedReply = "ANNOUNCER: Cravin's Diner has a deal so good it's almost illegal.\n" + "VOICE1: Almost. Stop by and taste the difference tonight.\n" + - "ANNOUNCER: Call 555-0142 - that's 555-0142 - Cravin's Diner."; + "ANNOUNCER: That's Cravin's Diner, right on Main Street."; // A route is appended by RouteSponsor on whichever thread the Arc's own arrangement runs on, and // read by the handler below on the station's own consumer thread — a plain List<> read racing that diff --git a/tests/GenWave.Tts.Tests/Specs/Story390_AdScriptWriter.cs b/tests/GenWave.Tts.Tests/Specs/Story390_AdScriptWriter.cs index 2e776531..f84d64c5 100644 --- a/tests/GenWave.Tts.Tests/Specs/Story390_AdScriptWriter.cs +++ b/tests/GenWave.Tts.Tests/Specs/Story390_AdScriptWriter.cs @@ -243,6 +243,7 @@ public sealed class ScenarioRuleIdMapsHonestlyToItsOwnCause // miss is OverLength, a content-truth-shaped miss is TruthGateReject). [Theory] [InlineData("format", LlmCallCause.MalformedResponse)] + [InlineData("stage_direction", LlmCallCause.MalformedResponse)] [InlineData("duration", LlmCallCause.OverLength)] [InlineData("brand_collision", LlmCallCause.TruthGateReject)] [InlineData("phone_shape", LlmCallCause.TruthGateReject)] @@ -441,10 +442,14 @@ public void AColonInsideProseIsNotATag() } [Fact] - public void AStillEmptyTagStaysVisibleForTheValidator() + public void AStillEmptyTagIsDroppedAtTheFinalJoin() { - // The T400 posture holds where nothing fills the tag: the validator names it, never a silent drop. - Assert.Equal("ANNOUNCER: Hi.\nVOICE1:", AdScriptWriter.ApplyLineAwareHygiene("ANNOUNCER: Hi.\nVOICE1:")); + // STORY-468/F201.1 supersedes the T400 posture this fact used to pin (a bare tag staying + // visible for the validator to name): a line whose text is STILL empty once continuation- + // joining has had its own chance to fill it is now dropped WHOLE, never surfaced as a bare + // "TAG:" — the same drop AC5's hygiene-emptied "VOICE1: (laughs)" exercises, generalized to + // a line that arrived with no text to begin with. + Assert.Equal("ANNOUNCER: Hi.", AdScriptWriter.ApplyLineAwareHygiene("ANNOUNCER: Hi.\nVOICE1:")); } } diff --git a/tests/GenWave.Tts.Tests/Specs/Story466_TheExamplePhoneNeverAirs.cs b/tests/GenWave.Tts.Tests/Specs/Story466_TheExamplePhoneNeverAirs.cs index cc60c239..3a5bfa75 100644 --- a/tests/GenWave.Tts.Tests/Specs/Story466_TheExamplePhoneNeverAirs.cs +++ b/tests/GenWave.Tts.Tests/Specs/Story466_TheExamplePhoneNeverAirs.cs @@ -1,52 +1,133 @@ // STORY-466 — The example phone never airs (gh-#701 · SPEC F199 · PLAN T549 T550) // -// BDD specification — xUnit. RED at plan time: every fact is [Fact(Skip = Pending)] with a loud body — -// remove the Skip only in the task that makes it green. Each Given comment names the arrange the scenario needs. +// BDD specification — xUnit. AC1/AC2 (T549) and AC3–AC5 (T550, AdScriptWriter.ApplyPhoneHygiene) live +// here. AC6 needs the real GenWave.Ads.AdScriptValidator — GenWave.Tts.Tests does not (and must not) +// reference GenWave.Ads — so it lives in +// tests/GenWave.Ads.Tests/Specs/Story466_TheValidatorStillRefusesAStray555.cs instead. namespace GenWave.Tts.Tests.Specs; +using System.Text.RegularExpressions; +using GenWave.Core.Domain; + public static class FeatureTheexamplephoneneverairs { - const string Pending = "pending: T549 — The example phone never airs (STORY-466)"; + static readonly Regex ExamplePhonePattern = new(@"555-01\d{2}", RegexOptions.Compiled); + + static AdScriptWriteRequest Request(string sponsorName) => + new(sponsorName, null, null, 30, AudiencePosture.Everyone, 200, 0.4); public sealed class ScenarioThePromptForOneSponsor { // Given: AdScriptPromptBuilder for slug "acme", built twice - - /// AC1 — the same 555-01xx both times - [Fact(Skip = Pending)] - public void IsDeterministic() => Assert.Fail(Pending); + readonly string firstExample; + readonly string secondExample; + + public ScenarioThePromptForOneSponsor() + { + firstExample = ExamplePhonePattern.Match(AdScriptPromptBuilder.BuildSystemPrompt(Request("acme"))).Value; + secondExample = ExamplePhonePattern.Match(AdScriptPromptBuilder.BuildSystemPrompt(Request("acme"))).Value; + } + + /// AC1 — the same 555-01xx example both times + [Fact] + public void IsDeterministic() => Assert.Equal(("555-0115", "555-0115"), (firstExample, secondExample)); } public sealed class ScenarioThePromptsForTwoSponsors { // Given: slugs "acme" and "zenith" - - /// AC2 — different examples - [Fact(Skip = Pending)] - public void DiffersPerSponsor() => Assert.Fail(Pending); + readonly string acmeExample; + readonly string zenithExample; + + public ScenarioThePromptsForTwoSponsors() + { + acmeExample = AdScriptPromptBuilder.ExamplePhone("acme"); + zenithExample = AdScriptPromptBuilder.ExamplePhone("zenith"); + } + + /// AC2 — different examples (pinned pair) + [Fact] + public void DiffersPerSponsor() => Assert.Equal(("555-0115", "555-0129"), (acmeExample, zenithExample)); } public sealed class ScenarioAScriptWithTheExample { // Given: "555-0142" in the script, sponsor phone "812-555-0199", AdScriptWriter hygiene (T550) + const string Script = "ANNOUNCER: Call 555-0142 for a free quote."; + const string SponsorPhone = "812-555-0199"; + + readonly string result; + + public ScenarioAScriptWithTheExample() => result = AdScriptWriter.ApplyPhoneHygiene(Script, SponsorPhone); /// AC3 — the sponsor phone is present - [Fact(Skip = Pending)] - public void SaysTheSponsorPhone() => Assert.Fail(Pending); + [Fact] + public void SaysTheSponsorPhone() => Assert.Contains(SponsorPhone, result, StringComparison.Ordinal); /// AC3 — no 555-0142 remains - [Fact(Skip = Pending)] - public void DropsTheExample() => Assert.Fail(Pending); + [Fact] + public void DropsTheExample() => Assert.DoesNotContain("555-0142", result, StringComparison.Ordinal); } public sealed class ScenarioASponsorWhoseOwnPhoneIs555 { // Given: sponsor phone "555-0100", script says it + const string Script = "ANNOUNCER: Call 555-0100 today."; + + readonly string result; + + public ScenarioASponsorWhoseOwnPhoneIs555() => result = AdScriptWriter.ApplyPhoneHygiene(Script, "555-0100"); /// AC4 — unchanged - [Fact(Skip = Pending)] - public void KeepsTheSponsorsOwnFiveFiveFive() => Assert.Fail(Pending); + [Fact] + public void KeepsTheSponsorsOwnFiveFiveFive() => Assert.Equal(Script, result); + } + + public sealed class ScenarioAParenFormattedSponsorPhone + { + // Given: sponsor phone "(406) 222-0100", script already carrying it in the same paren format + const string Script = "ANNOUNCER: Call (406) 222-0100 today."; + + readonly string result; + + public ScenarioAParenFormattedSponsorPhone() => + result = AdScriptWriter.ApplyPhoneHygiene(Script, "(406) 222-0100"); + + /// AC4 — the paren-formatted sponsor phone is left exactly as written, never + /// misread as only its own trailing 7-digit tail + [Fact] + public void KeepsTheParenFormattedNumber() => Assert.Equal(Script, result); + } + + public sealed class ScenarioTheSponsorPhoneInADifferentFormat + { + // Given: sponsor phone "812-555-0199", script carries the SAME digits wrapped in parens instead + const string Script = "ANNOUNCER: Call (812) 555-0199 today."; + + readonly string result; + + public ScenarioTheSponsorPhoneInADifferentFormat() => + result = AdScriptWriter.ApplyPhoneHygiene(Script, "812-555-0199"); + + /// AC4 — digit equality clears the run even when the formatting differs + [Fact] + public void KeepsTheDifferentlyFormattedNumber() => Assert.Equal(Script, result); + } + + public sealed class ScenarioAStrayNumberIsReplacedAsOneWholeRun + { + // Given: sponsor phone "(406) 222-0100", a stray dashed number sharing none of its digits + const string Script = "ANNOUNCER: Call 812-555-0199."; + + readonly string result; + + public ScenarioAStrayNumberIsReplacedAsOneWholeRun() => + result = AdScriptWriter.ApplyPhoneHygiene(Script, "(406) 222-0100"); + + /// AC3 — the whole 10-digit run is replaced, never split into a 7-digit tail + [Fact] + public void ReplacesTheWholeRun() => Assert.Equal("ANNOUNCER: Call (406) 222-0100.", result); } // --------------------------------------------------------------------- @@ -55,24 +136,44 @@ public sealed class ScenarioASponsorWhoseOwnPhoneIs555 public sealed class ScenarioASponsorWithNoPhone { - // Given: no phone; line "Call 555-0142 today for a quote." + // Given: no phone; line "Call 555-0142 today for a quote." (F199.4 pinned) + readonly string result; + + public ScenarioASponsorWithNoPhone() => + result = AdScriptWriter.ApplyPhoneHygiene("ANNOUNCER: Call 555-0142 today for a quote.", sponsorPhone: null); /// AC5 — the clause is dropped - [Fact(Skip = Pending)] - public void DropsTheClause() => Assert.Fail(Pending); + [Fact] + public void DropsTheClause() => Assert.Equal("ANNOUNCER: Today for a quote.", result); /// AC5 — no digits remain - [Fact(Skip = Pending)] - public void LeavesNoDigits() => Assert.Fail(Pending); + [Fact] + public void LeavesNoDigits() => Assert.DoesNotContain(result, char.IsDigit); } - public sealed class ScenarioAStrayNumberAfterHygiene + public sealed class ScenarioTheWholeLineIsThePhoneClause { - // Given: post-hygiene script carrying "555-0199" ≠ sponsor phone, F160 validation + // Given: no phone; the line has nothing in it but the phone clause + readonly string result; + + public ScenarioTheWholeLineIsThePhoneClause() => + result = AdScriptWriter.ApplyPhoneHygiene("ANNOUNCER: Call us at 555-0142.", sponsorPhone: null); - /// AC6 — the phone rule fails - [Fact(Skip = Pending)] - public void StillFailsTheValidator() => Assert.Fail(Pending); + /// AC5 — the line is dropped entirely, never left as a bare orphaned "." + [Fact] + public void DropsTheWholeLine() => Assert.Equal(string.Empty, result); } + public sealed class ScenarioThePhoneClauseFollowsAComma + { + // Given: no phone; a comma-joined lead-in precedes the phone clause + readonly string result; + + public ScenarioThePhoneClauseFollowsAComma() => + result = AdScriptWriter.ApplyPhoneHygiene("ANNOUNCER: Cravin's Diner, 555-0142.", sponsorPhone: null); + + /// AC5 — the orphaned comma goes with the clause; the sentence's own period stays + [Fact] + public void LeavesNoOrphanPunctuation() => Assert.Equal("ANNOUNCER: Cravin's Diner.", result); + } } diff --git a/tests/GenWave.Tts.Tests/Specs/Story468_StageDirectionsNeverReachTheVoice.cs b/tests/GenWave.Tts.Tests/Specs/Story468_StageDirectionsNeverReachTheVoice.cs index 0104c51a..8e06ad76 100644 --- a/tests/GenWave.Tts.Tests/Specs/Story468_StageDirectionsNeverReachTheVoice.cs +++ b/tests/GenWave.Tts.Tests/Specs/Story468_StageDirectionsNeverReachTheVoice.cs @@ -1,60 +1,90 @@ // STORY-468 — Stage directions never reach the voice (gh-#706 · SPEC F201 · PLAN T552) -// -// BDD specification — xUnit. RED at plan time: every fact is [Fact(Skip = Pending)] with a loud body — -// remove the Skip only in the task that makes it green. Each Given comment names the arrange the scenario needs. namespace GenWave.Tts.Tests.Specs; public static class FeatureStagedirectionsneverreachthevoice { - const string Pending = "pending: T552 — Stage directions never reach the voice (STORY-468)"; - - public sealed class ScenarioOneShapePerLine + public sealed class ScenarioLineAwareHygieneStripsStageDirections { - // Given: "(warmly) Come on down today." · "Come on down [beat] today." · "Come on down *pause* today." through ApplyLineAwareHygiene + // Given: a parenthetical, a bracketed beat, and an asterisked aside — alone (AC1–AC3) and all + // three on one line (AC4, the SAME given/expected text SPEC F201.3's own AdScriptWriter spec + // table pins verbatim) — through ApplyLineAwareHygiene. + + /// AC1 parentheses stripped · AC2 brackets stripped · AC3 asterisks stripped · + /// AC4/F201.3 all three on one line strip together, with the whitespace they leave behind + /// collapsed and a stranded space before the trailing period tidied away. + [Theory] + [InlineData("ANNOUNCER: (warmly) Come on down today.", "ANNOUNCER: Come on down today.")] + [InlineData("ANNOUNCER: Come on down [beat] today.", "ANNOUNCER: Come on down today.")] + [InlineData("ANNOUNCER: Come on down *pause* today.", "ANNOUNCER: Come on down today.")] + [InlineData( + "ANNOUNCER: (warmly) Come on down *pause* today [beat].", + "ANNOUNCER: Come on down today.")] + public void StripsTheShapeAndCollapsesWhitespace(string raw, string expected) => + Assert.Equal(expected, AdScriptWriter.ApplyLineAwareHygiene(raw)); + } - /// AC1 — parentheses stripped - [Fact(Skip = Pending)] - public void StripsParentheses() => Assert.Fail(Pending); + public sealed class ScenarioALineWithNoShapeIsUntouched + { + // PLAN T552 review F1: SpaceBeforePunctuationPattern used to run on every line whether or not a + // shape was stripped, so it rewrote legitimate copy that never carried a stage direction at all. + // Given: lines whose own punctuation/spacing is deliberate, carrying no "(…)"/"[…]"/"*…*" shape. - /// AC2 — brackets stripped - [Fact(Skip = Pending)] - public void StripsBrackets() => Assert.Fail(Pending); + /// An untouched line comes back byte for byte — an ellipsis is never collapsed, and a + /// deliberately spaced colon/period is never re-tidied. + [Theory] + [InlineData("ANNOUNCER: wait ... then go")] + [InlineData("ANNOUNCER: Remember : call now")] + [InlineData("ANNOUNCER: 3 . 5 dollars")] + public void ReturnsTheLineByteForByte(string raw) => + Assert.Equal(raw, AdScriptWriter.ApplyLineAwareHygiene(raw)); - /// AC3 — asterisks stripped - [Fact(Skip = Pending)] - public void StripsAsterisks() => Assert.Fail(Pending); + /// AC4/F201.3 still yields exactly the same collapsed result once a real shape is + /// present, pinning that the T552 F1 fix only skips the tidy when NOTHING was stripped. + [Fact] + public void AC4StillCollapsesWhenAShapeIsPresent() => + Assert.Equal( + "ANNOUNCER: Come on down today.", + AdScriptWriter.ApplyLineAwareHygiene("ANNOUNCER: (warmly) Come on down *pause* today [beat].")); } - public sealed class ScenarioAllThreeOnOneLine + public sealed class ScenarioEmptiedLinesNeverVoteForLeadAnnouncer { - // Given: "(warmly) Come on down *pause* today [beat]." + // PLAN T552 review F2: a line emptied by hygiene used to still VOTE in the "nobody is ANNOUNCER + // -> lead voice" election before being dropped at the final join — so two stage-direction-only + // VOICE1 lines outvoted a single real VOICE2 line and elected the WRONG voice as ANNOUNCER. + // Given: two pure-direction VOICE1 lines and one real VOICE2 line — VOICE2 should become + // ANNOUNCER once the emptied VOICE1 lines are dropped before the election runs. - /// AC4 — "Come on down today." - [Fact(Skip = Pending)] - public void StripsEveryShape() => Assert.Fail(Pending); + /// The emptied VOICE1 lines never cast a vote; VOICE2 — the only voice left with any + /// text — becomes ANNOUNCER. + [Fact] + public void TheSurvivingVoiceBecomesAnnouncer() => + Assert.Equal( + "ANNOUNCER: Hi.", + AdScriptWriter.ApplyLineAwareHygiene("VOICE1: (laughs)\nVOICE1: (sighs)\nVOICE2: Hi.")); } public sealed class ScenarioALineThatIsOnlyADirection { - // Given: "VOICE: (laughs)" + // Given: "ANNOUNCER: Come on down today.\nVOICE1: (laughs)" — STORY-468's own AC5 writes + // "VOICE" as shorthand for whichever cast voice carries the line; ApplyLineAwareHygiene's + // known cast is ANNOUNCER/VOICE1/VOICE2 (SPEC F200.1), so this fact uses VOICE1. An ANNOUNCER + // line precedes it so the script stays otherwise valid (F160.3 still has its required tag) — + // proving the drop is scoped to the one emptied line, never the whole script. - /// AC5 — the VOICE line is gone - [Fact(Skip = Pending)] - public void DropsTheEmptiedLine() => Assert.Fail(Pending); + /// AC5 — the VOICE1 line is gone, not left behind as a bare "VOICE1:" + [Fact] + public void DropsTheEmptiedLine() => + Assert.Equal( + "ANNOUNCER: Come on down today.", + AdScriptWriter.ApplyLineAwareHygiene("ANNOUNCER: Come on down today.\nVOICE1: (laughs)")); } // --------------------------------------------------------------------- // SAD PATH // --------------------------------------------------------------------- - public sealed class ScenarioResidueAfterHygiene - { - // Given: post-hygiene text containing "(beat)" through F160 - - /// AC6 — rule "stage-direction" names the line - [Fact(Skip = Pending)] - public void FailsValidation() => Assert.Fail(Pending); - } - + // AC6 lives in tests/GenWave.Ads.Tests/Specs/Story468_ResidueFailsValidation.cs — it needs the REAL + // GenWave.Ads.AdScriptValidator, which this project cannot reference (L10). }