Skip to content

Commit 377f7dc

Browse files
johnml1135johnml1135
committed
Bound fallback candidates for unresolved sound references.
1 parent c46a03b commit 377f7dc

4 files changed

Lines changed: 148 additions & 13 deletions

File tree

‎src/SIL.Motif.Commands/ProposalCommands.SoundSystem.cs‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -350,17 +350,17 @@ private static string ResolveEnvironmentReference(LcmCache cache, string referen
350350

351351
var candidates = matches.Length > 0 ? matches : environments.Where(environment =>
352352
ContainsReferenceName(environment.Name.BestAnalysisAlternative.Text, reference)).ToArray();
353-
if (candidates.Length == 0) candidates = environments;
354-
var displayedCandidates = matches.Length == 0 ? candidates.Take(12) : candidates;
355-
var candidateText = string.Join(", ", displayedCandidates
356-
.OrderBy(environment => EnvironmentDisplayName(environment), StringComparer.Ordinal)
357-
.ThenBy(environment => environment.Guid)
353+
var fallback = candidates.Length == 0;
354+
if (fallback) candidates = environments;
355+
var ordered = candidates.OrderBy(environment => EnvironmentDisplayName(environment), StringComparer.Ordinal)
356+
.ThenBy(environment => environment.Guid);
357+
var candidateText = string.Join(", ", (fallback ? ordered.Take(12) : ordered)
358358
.Select(environment => $"{EnvironmentDisplayName(environment)} [{CanonicalId.FromGuid(environment.Guid).Value}]"));
359-
if (matches.Length == 0 && candidates.Length > 12) candidateText += $", and {candidates.Length - 12} more";
360359
if (candidateText.Length == 0) candidateText = "none";
361360
var status = matches.Length == 0 ? "was not found" : "is ambiguous";
361+
var guidance = fallback ? $" Total: {candidates.Length}. Use motif_grammar(kind=environments) for the full list." : "";
362362
throw new InvalidOperationException(
363-
$"Environment reference '{reference}' {status}. Candidates: {candidateText}.");
363+
$"Environment reference '{reference}' {status}. Candidates: {candidateText}.{guidance}");
364364
}
365365

366366
private static string EnvironmentDisplayName(IPhEnvironment environment)

‎src/SIL.Motif.Runner/Composers/EditNaturalClassComposer.cs‎

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -112,16 +112,17 @@ public static CanonicalId ResolveNaturalClassReference(LcmCache cache, string re
112112

113113
var candidates = matches.Length > 0 ? matches : naturalClasses.Where(naturalClass => NaturalClassNames(naturalClass)
114114
.Any(name => ContainsName(name, reference))).ToArray();
115-
if (candidates.Length == 0) candidates = naturalClasses;
116-
var displayedCandidates = matches.Length == 0 ? candidates.Take(12) : candidates;
117-
var candidateText = string.Join(", ", displayedCandidates.OrderBy(NaturalClassDisplayName, StringComparer.Ordinal)
118-
.ThenBy(naturalClass => naturalClass.Guid)
115+
var fallback = candidates.Length == 0;
116+
if (fallback) candidates = naturalClasses;
117+
var ordered = candidates.OrderBy(NaturalClassDisplayName, StringComparer.Ordinal)
118+
.ThenBy(naturalClass => naturalClass.Guid);
119+
var candidateText = string.Join(", ", (fallback ? ordered.Take(12) : ordered)
119120
.Select(naturalClass => $"{NaturalClassDisplayName(naturalClass)} [{SoundSystemAuthoring.Id(naturalClass).Value}]"));
120-
if (matches.Length == 0 && candidates.Length > 12) candidateText += $", and {candidates.Length - 12} more";
121121
if (candidateText.Length == 0) candidateText = "none";
122122
var status = matches.Length == 0 ? "was not found" : "is ambiguous";
123+
var guidance = fallback ? $" Total: {candidates.Length}. Use motif_grammar(kind=natural_classes) for the full list." : "";
123124
throw new InvalidOperationException(
124-
$"'{ConstructName}': natural class reference '{reference}' {status}. Candidates: {candidateText}.");
125+
$"'{ConstructName}': natural class reference '{reference}' {status}. Candidates: {candidateText}.{guidance}");
125126
}
126127

127128
private static IPhNCSegments ResolveSegmentClass(LcmCache cache, CanonicalId id)

‎tests/SIL.Motif.Tests.LibLcm/Composers/EditNaturalClassComposerTests.cs‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,10 +40,45 @@ public void ReferenceNamesUseCanonicalDecompositionAndOrdinalCaseWithoutGuessing
4040
Assert.Contains("ambiguous", ambiguous.Message);
4141
Assert.Contains(naturalClass.Value, ambiguous.Message);
4242
Assert.Contains(duplicate.Value, ambiguous.Message);
43+
Assert.Contains("vo\u0301wels", ambiguous.Message.Normalize(System.Text.NormalizationForm.FormD),
44+
StringComparison.OrdinalIgnoreCase);
4345
Assert.Throws<ContractParseException>(() => EditNaturalClassIntentParser.Parse(JsonSerializer.SerializeToElement(
4446
new { target = "vówels", expectedMembers = new[] { a.Value }, members = new[] { a.Value } })));
4547
}
4648

49+
[Fact]
50+
public void MissingReferenceBoundsFallbackButNotSuggestionsOrAmbiguousMatches()
51+
{
52+
using var cache = pristine.NewScratch();
53+
var authored = new List<OperationEnvelope>();
54+
var phoneme = CreatePhoneme(cache, authored, "a");
55+
var classes = Enumerable.Range(0, 13)
56+
.Select(index => CreateSegmentClass(cache, authored, $"class {index}", $"C{index}", [phoneme]))
57+
.ToArray();
58+
59+
var refusal = Assert.Throws<InvalidOperationException>(() =>
60+
EditNaturalClassComposer.ResolveNaturalClassReference(cache, "absent"));
61+
62+
Assert.Equal(12, classes.Count(id => refusal.Message.Contains(id.Value, StringComparison.Ordinal)));
63+
Assert.Contains("Total: 13", refusal.Message);
64+
Assert.Contains("motif_grammar(kind=natural_classes)", refusal.Message);
65+
var suggestions = Assert.Throws<InvalidOperationException>(() =>
66+
EditNaturalClassComposer.ResolveNaturalClassReference(cache, "class"));
67+
Assert.All(classes, id => Assert.Contains(id.Value, suggestions.Message));
68+
Assert.DoesNotContain("Total:", suggestions.Message);
69+
70+
NonUndoableUnitOfWorkHelper.Do(cache.ActionHandlerAccessor, () =>
71+
{
72+
foreach (var naturalClass in cache.LangProject.PhonologicalDataOA.NaturalClassesOS)
73+
naturalClass.Name.set_String(cache.DefaultAnalWs, "shared name");
74+
});
75+
var ambiguous = Assert.Throws<InvalidOperationException>(() =>
76+
EditNaturalClassComposer.ResolveNaturalClassReference(cache, "shared name"));
77+
Assert.Contains("ambiguous", ambiguous.Message);
78+
Assert.All(classes, id => Assert.Contains(id.Value, ambiguous.Message));
79+
Assert.DoesNotContain("Total:", ambiguous.Message);
80+
}
81+
4782
[Fact]
4883
public void EditsOnlyTheRequestedSegmentIdentities()
4984
{

‎tests/SIL.Motif.Tests.Mcp/ToolSurfaceTests.cs‎

Lines changed: 99 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
using System.Text.Json.Nodes;
44
using System.Text.RegularExpressions;
55
using SIL.LCModel;
6+
using SIL.LCModel.Core.Text;
67
using SIL.LCModel.Infrastructure;
78
using SIL.Motif.Commands;
89
using SIL.Motif.Commands.Baselines;
@@ -457,6 +458,11 @@ public async Task DetailedMemberIdsAreAcceptedAsExpectedMembersByNaturalClassEdi
457458
Guid phonemeGuid = default;
458459
Guid otherPhonemeGuid = default;
459460
Guid naturalClassGuid = default;
461+
Guid duplicateClassGuid = default;
462+
Guid environmentGuid = default;
463+
Guid duplicateEnvironmentGuid = default;
464+
var suggestionGuids = new List<Guid>();
465+
var baselineEnvironmentCount = 0;
460466
using var project = new WalkthroughProject(pristine, configure: (cache, _, _) =>
461467
{
462468
NonUndoableUnitOfWorkHelper.Do(cache.ActionHandlerAccessor, () =>
@@ -488,9 +494,38 @@ public async Task DetailedMemberIdsAreAcceptedAsExpectedMembersByNaturalClassEdi
488494
naturalClass.Abbreviation.set_String(cache.DefaultAnalWs, "N");
489495
naturalClass.SegmentsRC.Add(phoneme);
490496
naturalClass.SegmentsRC.Add(otherPhoneme);
497+
var duplicateClass = services.GetInstance<IPhNCSegmentsFactory>().Create();
498+
phonology.NaturalClassesOS.Add(duplicateClass);
499+
duplicateClass.Name.set_String(cache.DefaultAnalWs, "nasals");
500+
duplicateClass.Abbreviation.set_String(cache.DefaultAnalWs, "N2");
501+
duplicateClass.SegmentsRC.Add(phoneme);
502+
var environment = services.GetInstance<IPhEnvironmentFactory>().Create();
503+
phonology.EnvironmentsOS.Add(environment);
504+
environment.Name.set_String(cache.DefaultAnalWs, "after nasal");
505+
environment.StringRepresentation = TsStringUtils.MakeString("/ [N] _", cache.DefaultVernWs);
506+
var duplicateEnvironment = services.GetInstance<IPhEnvironmentFactory>().Create();
507+
phonology.EnvironmentsOS.Add(duplicateEnvironment);
508+
duplicateEnvironment.Name.set_String(cache.DefaultAnalWs, "after nasal");
509+
duplicateEnvironment.StringRepresentation = TsStringUtils.MakeString("/ [N] _", cache.DefaultVernWs);
510+
var namedEnvironment = services.GetInstance<IPhEnvironmentFactory>().Create();
511+
phonology.EnvironmentsOS.Add(namedEnvironment);
512+
namedEnvironment.Name.set_String(cache.DefaultAnalWs, "after a nasal");
513+
namedEnvironment.StringRepresentation = TsStringUtils.MakeString("/ [N] _", cache.DefaultVernWs);
514+
for (var index = 0; index < 13; index++)
515+
{
516+
var suggestion = services.GetInstance<IPhEnvironmentFactory>().Create();
517+
phonology.EnvironmentsOS.Add(suggestion);
518+
suggestion.Name.set_String(cache.DefaultAnalWs, "zzz candidate");
519+
suggestion.StringRepresentation = TsStringUtils.MakeString("/ [N] _", cache.DefaultVernWs);
520+
suggestionGuids.Add(suggestion.Guid);
521+
}
522+
baselineEnvironmentCount = phonology.EnvironmentsOS.Count;
491523
phonemeGuid = phoneme.Guid;
492524
otherPhonemeGuid = otherPhoneme.Guid;
493525
naturalClassGuid = naturalClass.Guid;
526+
duplicateClassGuid = duplicateClass.Guid;
527+
environmentGuid = environment.Guid;
528+
duplicateEnvironmentGuid = duplicateEnvironment.Guid;
494529
});
495530
});
496531
var managedRoot = RunnerOptions.ResolveRoot();
@@ -571,6 +606,70 @@ public async Task DetailedMemberIdsAreAcceptedAsExpectedMembersByNaturalClassEdi
571606
("expectedMembers", memberIds), ("members", new[] { memberId })), CancellationToken.None);
572607
Assert.False(namedEdit.IsError ?? false, namedEdit.StructuredContent?.GetRawText());
573608
Assert.Equal(1, namedEdit.StructuredContent!.Value.GetProperty("result").GetProperty("operationCount").GetInt32());
609+
610+
var start = await MotifMcpServer.CallAsync(editContext, experimentalTools, "motif_start_proposal",
611+
Arguments(("project", project.FwDataPath), ("draft", "named-references"), ("label", "Use named references"),
612+
("rationale", "Check explicit sound references.")), CancellationToken.None);
613+
Assert.False(start.IsError ?? false, start.StructuredContent?.GetRawText());
614+
async Task<ModelContextProtocol.Protocol.CallToolResult> AddEnvironment(string reference, string name) =>
615+
await MotifMcpServer.CallAsync(context, grammarTools, "motif_add_environment",
616+
Arguments(("project", project.FwDataPath), ("draft", "named-references"), ("name", name),
617+
("left", new[] { new { naturalClass = reference } }), ("right", Array.Empty<object>())), CancellationToken.None);
618+
var missingClass = await AddEnvironment("absent", "missing class");
619+
Assert.True(missingClass.IsError);
620+
Assert.Equal("draft.invalid", missingClass.StructuredContent!.Value.GetProperty("code").GetString());
621+
Assert.Contains(CanonicalId.FromGuid(naturalClassGuid).Value, missingClass.StructuredContent.Value.GetProperty("message").GetString());
622+
var ambiguousClass = await AddEnvironment("nasals", "ambiguous class");
623+
Assert.True(ambiguousClass.IsError);
624+
var classMessage = ambiguousClass.StructuredContent!.Value.GetProperty("message").GetString()!;
625+
Assert.Contains("ambiguous", classMessage);
626+
Assert.Contains(CanonicalId.FromGuid(naturalClassGuid).Value, classMessage);
627+
Assert.Contains(CanonicalId.FromGuid(duplicateClassGuid).Value, classMessage);
628+
Assert.False((await AddEnvironment("N", "named class")).IsError ?? false);
629+
Assert.False((await AddEnvironment(CanonicalId.FromGuid(naturalClassGuid).Value, "identified class")).IsError ?? false);
630+
631+
var replacement = await MotifMcpServer.CallAsync(context, grammarTools, "motif_add_natural_class",
632+
Arguments(("project", project.FwDataPath), ("draft", "named-references"), ("name", "replacement nasals"),
633+
("abbreviation", "RN"), ("members", new[] { memberId })), CancellationToken.None);
634+
Assert.False(replacement.IsError ?? false, replacement.StructuredContent?.GetRawText());
635+
var creation = replacement.StructuredContent!.Value.GetProperty("result").GetProperty("operations")
636+
.EnumerateArray().Single(operation => operation.GetProperty("kind").GetString()!.EndsWith("/createNaturalClasses"));
637+
var namedClass = await AddEnvironment("replacement nasals", "after replacement");
638+
Assert.False(namedClass.IsError ?? false, namedClass.StructuredContent?.GetRawText());
639+
async Task<ModelContextProtocol.Protocol.CallToolResult> Relink(string target) =>
640+
await MotifMcpServer.CallAsync(editContext, experimentalTools, "motif_relink_natural_class",
641+
Arguments(("project", project.FwDataPath), ("draft", "named-references"), ("source", "N"),
642+
("replacement", "replacement nasals"), ("replacementCreationOperation", creation.GetProperty("operationId").GetString()),
643+
("environments", new[] { new { target, expectedLeft = new[] { new { naturalClass = "N" } }, expectedRight = Array.Empty<object>() } }),
644+
("ruleContexts", Array.Empty<string>())), CancellationToken.None);
645+
var missingEnvironment = await Relink("absent");
646+
Assert.True(missingEnvironment.IsError);
647+
Assert.Equal("draft.invalid", missingEnvironment.StructuredContent!.Value.GetProperty("code").GetString());
648+
var missingMessage = missingEnvironment.StructuredContent.Value.GetProperty("message").GetString()!;
649+
Assert.Contains(CanonicalId.FromGuid(environmentGuid).Value, missingMessage);
650+
Assert.Equal(12, Regex.Matches(missingMessage, @"\[[A-Za-z0-9_-]{22}\]").Count);
651+
Assert.Contains($"Total: {baselineEnvironmentCount + 3}", missingMessage);
652+
Assert.Contains("motif_grammar(kind=environments)", missingMessage);
653+
var suggestions = await Relink("zzz");
654+
Assert.True(suggestions.IsError);
655+
var suggestionMessage = suggestions.StructuredContent!.Value.GetProperty("message").GetString()!;
656+
Assert.All(suggestionGuids, guid => Assert.Contains(CanonicalId.FromGuid(guid).Value, suggestionMessage));
657+
Assert.DoesNotContain("Total:", suggestionMessage);
658+
var manyAmbiguous = await Relink("zzz candidate");
659+
Assert.True(manyAmbiguous.IsError);
660+
var manyMessage = manyAmbiguous.StructuredContent!.Value.GetProperty("message").GetString()!;
661+
Assert.Contains("ambiguous", manyMessage);
662+
Assert.All(suggestionGuids, guid => Assert.Contains(CanonicalId.FromGuid(guid).Value, manyMessage));
663+
var ambiguousEnvironment = await Relink("after nasal");
664+
Assert.True(ambiguousEnvironment.IsError);
665+
var environmentMessage = ambiguousEnvironment.StructuredContent!.Value.GetProperty("message").GetString()!;
666+
Assert.Contains("ambiguous", environmentMessage);
667+
Assert.Contains(CanonicalId.FromGuid(environmentGuid).Value, environmentMessage);
668+
Assert.Contains(CanonicalId.FromGuid(duplicateEnvironmentGuid).Value, environmentMessage);
669+
var namedRelink = await Relink("AFTER A NASAL");
670+
Assert.False(namedRelink.IsError ?? false, namedRelink.StructuredContent?.GetRawText());
671+
var identifiedRelink = await Relink(CanonicalId.FromGuid(environmentGuid).Value);
672+
Assert.False(identifiedRelink.IsError ?? false, identifiedRelink.StructuredContent?.GetRawText());
574673
}
575674

576675
private static Dictionary<string, JsonElement> Arguments(params (string Name, object? Value)[] values) =>

0 commit comments

Comments
 (0)