From 5c2449e9bea3fa38d6a94b71693338981617f065 Mon Sep 17 00:00:00 2001 From: Monster0506 Date: Sun, 20 Sep 2026 05:23:53 -0400 Subject: [PATCH] Drop implausibly small per-occupation salary estimates (e.g. 80 instead of 80000) Found via real browser testing: the Market Potential page occasionally showed nonsensical ranges like "$80 - $120" for a Software Engineer match. The LLM sometimes returns typicalMinUsd/typicalMaxUsd as an abbreviated figure (e.g. 80, presumably meaning "$80k") rather than a full annual-salary number - a valid, correctly-ordered positive range, so the existing negative/min>max validation didn't catch it. - Strengthened the prompt with an explicit example ("85000, never 85 or '85k'") to reduce how often this happens. - Added a plausibility floor (any value under $1,000/year is obviously not a real full-time salary) that nulls out both values rather than keeping them, consistent with the prompt's own "omit both if you cannot give a reasonable estimate" contract and the codebase's existing explicit-no-estimate-over-fabricated-data philosophy. --- .../AiSuggestedOccupationMatchingService.cs | 90 ++++++++++++++----- ...SuggestedOccupationMatchingServiceTests.cs | 55 ++++++++++-- 2 files changed, 114 insertions(+), 31 deletions(-) diff --git a/HumanCapital/AiSuggestedOccupationMatchingService.cs b/HumanCapital/AiSuggestedOccupationMatchingService.cs index 782f663..68ab681 100644 --- a/HumanCapital/AiSuggestedOccupationMatchingService.cs +++ b/HumanCapital/AiSuggestedOccupationMatchingService.cs @@ -22,9 +22,14 @@ public AiSuggestedOccupationMatchingService(ILlmService llmService) public async Task> MatchAsync( ProfessionalProfile profile, - CancellationToken cancellationToken = default) + CancellationToken cancellationToken = default + ) { - if (profile.Skills.Count == 0 && profile.Experience.Count == 0 && profile.Education.Count == 0) + if ( + profile.Skills.Count == 0 + && profile.Experience.Count == 0 + && profile.Education.Count == 0 + ) { return []; } @@ -37,7 +42,9 @@ public async Task> MatchAsync( catch (LlmServiceException ex) { throw new OccupationMatchingException( - "Failed to match occupations: the LLM provider call failed.", ex); + "Failed to match occupations: the LLM provider call failed.", + ex + ); } var json = StripCodeFence(completion); @@ -50,34 +57,45 @@ public async Task> MatchAsync( catch (JsonException ex) { throw new OccupationMatchingException( - "The LLM returned a response that could not be parsed as occupation matches.", ex); + "The LLM returned a response that could not be parsed as occupation matches.", + ex + ); } if (response?.Occupations is not { } occupations) { throw new OccupationMatchingException( - "The LLM returned an occupation response without a structured occupations list."); + "The LLM returned an occupation response without a structured occupations list." + ); } var matches = new List(occupations.Count); foreach (var occupation in occupations) { - if (occupation is null + if ( + occupation is null || string.IsNullOrWhiteSpace(occupation.Title) - || string.IsNullOrWhiteSpace(occupation.Explanation)) + || string.IsNullOrWhiteSpace(occupation.Explanation) + ) { throw new OccupationMatchingException( - "The LLM returned an occupation match without a title or explanation."); + "The LLM returned an occupation match without a title or explanation." + ); } - if (occupation.TypicalMinUsd is < 0 + if ( + occupation.TypicalMinUsd is < 0 || occupation.TypicalMaxUsd is < 0 - || (occupation.TypicalMinUsd is not null + || ( + occupation.TypicalMinUsd is not null && occupation.TypicalMaxUsd is not null - && occupation.TypicalMinUsd > occupation.TypicalMaxUsd)) + && occupation.TypicalMinUsd > occupation.TypicalMaxUsd + ) + ) { throw new OccupationMatchingException( - "The LLM returned an invalid compensation range for an occupation match."); + "The LLM returned an invalid compensation range for an occupation match." + ); } var keySkills = (occupation.KeySkills ?? []) @@ -86,18 +104,42 @@ public async Task> MatchAsync( .Distinct(StringComparer.OrdinalIgnoreCase) .ToList(); - matches.Add(new OccupationMatch( - occupation.Title.Trim(), - occupation.Explanation.Trim(), - IsAiEstimated: true, - keySkills, - occupation.TypicalMinUsd, - occupation.TypicalMaxUsd)); + // The LLM occasionally returns an abbreviated figure (e.g. 80, + // presumably meaning "$80k") instead of a full annual-salary + // number - a positive value, so the min<0/max<0/min>max checks + // above don't catch it, but clearly not a real US salary. Treat + // it the same as the prompt's own "omit both if you cannot give + // a reasonable estimate" instruction: drop it rather than show + // a nonsensical range like "$80 - $120". + var (typicalMinUsd, typicalMaxUsd) = + IsPlausibleAnnualSalary(occupation.TypicalMinUsd) + && IsPlausibleAnnualSalary(occupation.TypicalMaxUsd) + ? (occupation.TypicalMinUsd, occupation.TypicalMaxUsd) + : (null, null); + + matches.Add( + new OccupationMatch( + occupation.Title.Trim(), + occupation.Explanation.Trim(), + IsAiEstimated: true, + keySkills, + typicalMinUsd, + typicalMaxUsd + ) + ); } return matches; } + // No real full-time US annual salary is this low; a value below this + // is almost certainly the model returning an abbreviated figure (e.g. + // 80 instead of 80000) rather than a genuine estimate. + private const int MinPlausibleAnnualSalaryUsd = 1_000; + + private static bool IsPlausibleAnnualSalary(int? value) => + value is null || value >= MinPlausibleAnnualSalaryUsd; + private static string BuildPrompt(ProfessionalProfile profile) { var summary = new StringBuilder(); @@ -112,7 +154,9 @@ private static string BuildPrompt(ProfessionalProfile profile) summary.AppendLine("Experience:"); foreach (var exp in profile.Experience) { - var description = exp.Description is not null ? $": {exp.Description}" : string.Empty; + var description = exp.Description is not null + ? $": {exp.Description}" + : string.Empty; summary.AppendLine($"- {exp.Title ?? "Role"} at {exp.Organization}{description}"); } } @@ -150,7 +194,8 @@ the profile above that justify the match (one sentence). Only suggest occupation (not limited to skills already in the profile above). "typicalMinUsd"/"typicalMaxUsd" should be a rough typical US annual - salary range for that specific occupation (not the whole profile). + salary range for that specific occupation (not the whole profile), + as a full number in dollars - e.g. 85000, never 85 or "85k". Omit both if you cannot give a reasonable estimate. """; } @@ -185,5 +230,6 @@ private record OccupationEntry( string? Explanation, IReadOnlyList? KeySkills, int? TypicalMinUsd, - int? TypicalMaxUsd); + int? TypicalMaxUsd + ); } diff --git a/tests/MoneyMirror.Tests/HumanCapital/AiSuggestedOccupationMatchingServiceTests.cs b/tests/MoneyMirror.Tests/HumanCapital/AiSuggestedOccupationMatchingServiceTests.cs index 94e3aaa..5e636a3 100644 --- a/tests/MoneyMirror.Tests/HumanCapital/AiSuggestedOccupationMatchingServiceTests.cs +++ b/tests/MoneyMirror.Tests/HumanCapital/AiSuggestedOccupationMatchingServiceTests.cs @@ -7,14 +7,18 @@ public class AiSuggestedOccupationMatchingServiceTests { private class FakeLlmService(string response) : ILlmService { - public Task CompleteAsync(string prompt, CancellationToken cancellationToken = default) => - Task.FromResult(response); + public Task CompleteAsync( + string prompt, + CancellationToken cancellationToken = default + ) => Task.FromResult(response); } private class ThrowingLlmService : ILlmService { - public Task CompleteAsync(string prompt, CancellationToken cancellationToken = default) => - throw new LlmServiceException("provider unavailable"); + public Task CompleteAsync( + string prompt, + CancellationToken cancellationToken = default + ) => throw new LlmServiceException("provider unavailable"); } private static readonly ProfessionalProfile SampleProfile = ProfessionalProfile.Empty with @@ -80,7 +84,9 @@ public async Task MatchAsync_MissingOccupationsList_ThrowsOccupationMatchingExce { var service = new AiSuggestedOccupationMatchingService(new FakeLlmService("{}")); - await Assert.ThrowsAsync(() => service.MatchAsync(SampleProfile)); + await Assert.ThrowsAsync(() => + service.MatchAsync(SampleProfile) + ); } [Fact] @@ -89,7 +95,9 @@ public async Task MatchAsync_MatchMissingTitleOrExplanation_ThrowsOccupationMatc const string json = "{ \"occupations\": [{ \"title\": \"Software Engineer\" }] }"; var service = new AiSuggestedOccupationMatchingService(new FakeLlmService(json)); - await Assert.ThrowsAsync(() => service.MatchAsync(SampleProfile)); + await Assert.ThrowsAsync(() => + service.MatchAsync(SampleProfile) + ); } [Fact] @@ -104,7 +112,32 @@ public async Task MatchAsync_InvalidCompensationRange_ThrowsOccupationMatchingEx """; var service = new AiSuggestedOccupationMatchingService(new FakeLlmService(json)); - await Assert.ThrowsAsync(() => service.MatchAsync(SampleProfile)); + await Assert.ThrowsAsync(() => + service.MatchAsync(SampleProfile) + ); + } + + [Fact] + public async Task MatchAsync_ImplausiblySmallCompensationRange_NullsOutBothValuesRatherThanKeepingThem() + { + // The model sometimes returns an abbreviated figure (e.g. 80, + // presumably meaning "$80k") instead of a real annual-salary + // number. It's a valid, correctly-ordered positive range, so the + // negative/min>max checks don't catch it - it should still be + // dropped rather than shown as a nonsensical "$80 - $120". + const string json = """ + { + "occupations": [ + {"title": "Software Engineer", "explanation": "Strong C# background.", "typicalMinUsd": 80, "typicalMaxUsd": 120} + ] + } + """; + var service = new AiSuggestedOccupationMatchingService(new FakeLlmService(json)); + + var match = Assert.Single(await service.MatchAsync(SampleProfile)); + + Assert.Null(match.TypicalMinUsd); + Assert.Null(match.TypicalMaxUsd); } [Fact] @@ -150,7 +183,9 @@ public async Task MatchAsync_MalformedJson_ThrowsOccupationMatchingException() { var service = new AiSuggestedOccupationMatchingService(new FakeLlmService("not json")); - await Assert.ThrowsAsync(() => service.MatchAsync(SampleProfile)); + await Assert.ThrowsAsync(() => + service.MatchAsync(SampleProfile) + ); } [Fact] @@ -158,6 +193,8 @@ public async Task MatchAsync_LlmServiceFails_ThrowsOccupationMatchingException() { var service = new AiSuggestedOccupationMatchingService(new ThrowingLlmService()); - await Assert.ThrowsAsync(() => service.MatchAsync(SampleProfile)); + await Assert.ThrowsAsync(() => + service.MatchAsync(SampleProfile) + ); } }