From 6a266efc4805ae35f2b5de1e6415c53704665ef2 Mon Sep 17 00:00:00 2001 From: Pavlo Sushkov Date: Tue, 11 Aug 2026 01:28:11 +0300 Subject: [PATCH] fix: harden requirement extraction and rescore visibility Extraction hygiene, all deterministic and scoring-neutral by construction: - Technology extraction order is now stable. Map.copyOf salts its iteration order per JVM run, so identical text produced differently ordered technology lists and ExtractedRequirements record equality was unstable across runs. Nothing depended on that while rescore membership compared scores only. - REST is extracted only from qualified evidence: the API/service/endpoint/ controller forms, RESTful, JAX-RS, Spring REST, the technical shorthands HTTP/REST, REST/JSON and APIs (REST), after an explicit technical introducer, and as a complete item in a delimited technology list. The list delimiter is never a bare space, which is what keeps "Rest Assured" and "the rest of the stack" negative. - Go is extracted only from unambiguous spellings or from a delimited language list with a recognised neighbour that terminates as a list item, so ordinary English "go" no longer counts as evidence. - SpringBoot and Spring-Boot canonicalise to Spring Boot; JVM becomes its own canonical term, in no skill list and not a programming language. - Postgres -> PostgreSQL stays deliberately deferred to the scoring redesign, pinned by a regression test, because PostgreSQL is an equally weighted backend skill today. Rescore preview visibility: - Plan membership follows either persisted projection, not the score alone. The write transaction already re-persisted requirements whenever they differed, so requirement-only drift was being written without ever appearing in a preview. - The preview reports scoreChanged, requirementsChanged, requirementsOnly, changedPlan and unchanged counts, and names every differing ExtractedRequirements field with sanitized, bounded old and new values. Raw values are compared before any display conversion, so two values that share a truncated prefix cannot be reported as equal. - ScoreRescorePlan asserts its entry count against the report, so a write can never touch a row the preview did not account for. Fingerprint, locking and the existing guards are unchanged. Focused regression tests cover every REST and Go case, the aliases, the deferral, the 15-field requirement diff and the plan/report invariants. No scoring weights, bands, blockers, screening, workflow, Mini App ranking, schema or configuration were changed. Co-Authored-By: Claude Opus 5 --- .../DeterministicRequirementExtractor.java | 107 +++++++- .../preview/ScoreRescorePreviewReport.java | 82 +++++- .../ScoreRescorePreviewReportRenderer.java | 31 ++- .../preview/ScoreRescorePreviewService.java | 124 ++++++++- .../matching/rescore/ScoreRescorePlan.java | 7 + .../rescore/ScoreRescorePlanEntry.java | 15 ++ ...DeterministicRequirementExtractorTest.java | 238 +++++++++++++++++ .../matching/preview/RequirementDiffTest.java | 244 ++++++++++++++++++ ...ScoreRescorePreviewReportRendererTest.java | 76 ++++++ .../ScoreRescorePreviewServiceTest.java | 96 ++++++- .../ScoreRescorePlanEntryChangeTest.java | 70 +++++ .../ScoreRescoreWriteCoordinatorTest.java | 12 +- .../ScoreRescoreWriteTransactionTest.java | 5 +- 13 files changed, 1088 insertions(+), 19 deletions(-) create mode 100644 src/test/java/com/jobpilot/matching/preview/RequirementDiffTest.java create mode 100644 src/test/java/com/jobpilot/matching/preview/ScoreRescorePreviewReportRendererTest.java create mode 100644 src/test/java/com/jobpilot/matching/rescore/ScoreRescorePlanEntryChangeTest.java diff --git a/src/main/java/com/jobpilot/extraction/DeterministicRequirementExtractor.java b/src/main/java/com/jobpilot/extraction/DeterministicRequirementExtractor.java index 25fa63e..d5c4928 100644 --- a/src/main/java/com/jobpilot/extraction/DeterministicRequirementExtractor.java +++ b/src/main/java/com/jobpilot/extraction/DeterministicRequirementExtractor.java @@ -3,6 +3,7 @@ import com.jobpilot.jobs.domain.ExtractedRequirements; import com.jobpilot.jobs.domain.Job; import java.util.ArrayList; +import java.util.Collections; import java.util.LinkedHashMap; import java.util.List; import java.util.Locale; @@ -15,6 +16,75 @@ public class DeterministicRequirementExtractor { private static final Pattern YEARS = Pattern.compile( "(?i)(?:minimum|min\\.?|at least|requires?|with)?\\s*(\\d+(?:\\.\\d+)?)\\s*\\+?\\s*(?:years?|yrs?)(?:\\s+of)?(?:\\s+(?:commercial|professional|industry))?\\s+experience"); + /** Technical tokens REST is credibly listed beside; longer alternatives come first. */ + private static final String REST_TECH_TOKEN = "(?:OOP|OO|MVVM|MVC|MVP|SOAP|JSON|XML|YAML" + + "|HTTPS|HTTP|gRPC|GraphQL|SOA|microservices|NoSQL|PostgreSQL|MySQL|MongoDB|SQL" + + "|Redis|Kafka|RabbitMQ|Spring Boot|Spring|Hibernate|JPA|JDBC|Maven|Gradle|JUnit" + + "|Mockito|Selenium|Cucumber|BDD|TDD|Docker|Kubernetes|CI/CD|Jenkins|Linux|AWS" + + "|Azure|GCP|HTML|CSS|React|Angular|Node|Django|Flask|\\.NET|APIs|API|JavaScript" + + "|TypeScript|Java|Kotlin|Scala|Python|C#|C\\+\\+|Git)"; + /** Never a bare space: that is what keeps "Rest Assured" out of every list form. */ + private static final String REST_LIST_DELIMITER = "(?:\\s*[,;/]\\s*|\\s+(?:and|or)\\s+)"; + private static final String REST_LIST_END = "(?=\\s*[.)\\];:]|\\s*$)"; + private static final String REST_INTRODUCER = "(?:experience\\s+(?:with|in)|knowledge\\s+of" + + "|understanding\\s+of|familiarity\\s+with|proficiency\\s+(?:in|with)" + + "|proficient\\s+(?:in|with)|skills?\\s+(?:in|with))"; + + /** + * Qualified REST. Bare "rest" is an ordinary English noun and "Rest Assured" is a test + * library, so neither is API evidence on its own, but production showed the token also + * carries genuine evidence as technical shorthand ("HTTP/REST", "REST/JSON", "APIs (REST)"), + * directly after a technical introducer, and as an item in a delimited technology list + * ("OO, MVC, REST", "Java, Spring Boot, REST, SQL"). + * + *

The list forms require REST to be a complete item — a delimiter on both sides, + * or a delimiter and the end of the list — and the delimiter never matches a bare space. That + * is what keeps "Selenium, Cucumber, BDD, Rest Assured and CI/CD" negative: "Rest" there is + * followed by a word, not by a delimiter. A posting carrying both dirty and genuine text + * still matches, because the scan reaches the genuine occurrence. + */ + private static final String QUALIFIED_REST = "(?:" + + "RESTful" + + "|REST[\\s-]?(?:API|service|endpoint|controller)s?" + + "|JAX-?RS" + + "|Spring\\s+REST" + + "|HTTP\\s*/\\s*REST" + + "|REST\\s*/\\s*JSON" + + "|APIs?\\s*\\(\\s*REST\\s*\\)" + + "|" + REST_INTRODUCER + "\\s+REST" + + "|" + REST_TECH_TOKEN + REST_LIST_DELIMITER + "REST" + + REST_LIST_DELIMITER + REST_TECH_TOKEN + + "|" + REST_TECH_TOKEN + REST_LIST_DELIMITER + REST_TECH_TOKEN + + REST_LIST_DELIMITER + "REST" + REST_LIST_END + + "|REST" + REST_LIST_DELIMITER + REST_TECH_TOKEN + + REST_LIST_DELIMITER + REST_TECH_TOKEN + + ")"; + + private static final String LANGUAGE_TOKEN = "(?:Java|Python|Rust|Kotlin|Scala|Ruby|PHP" + + "|C\\+\\+|C#|TypeScript|JavaScript|Elixir|Erlang|Swift|Haskell)"; + private static final String LANGUAGE_DELIMITER = "(?:\\s*[,/]\\s*|\\s+(?:and|or)\\s+)"; + private static final String TECHNICAL_INTRODUCER = "(?:programming\\s+languages?|languages?" + + "|technologies|tech\\s+stack|stack|experience\\s+(?:with|in)" + + "|proficient\\s+(?:with|in)|written\\s+in|skills\\s+(?:with|in))"; + /** Go has to end as a list item; without this "experience with Java and go to market" matches. */ + private static final String GO_LIST_TAIL = + "(?=\\s*[,;./)\\]]|\\s+(?:and|or)\\s+" + LANGUAGE_TOKEN + "|\\s*$)"; + + /** + * "Go" is an ordinary English verb, so a bare token is never evidence. Either it is spelled + * unambiguously, or it sits between two recognised languages in a delimited list, or + * it follows an explicit technical introducer and terminates as a list item. Simple + * two-item adjacency is deliberately rejected: "ready to go, Java developers should apply" + * would otherwise qualify. Conservative false negatives are preferred here — Go carries no + * scoring weight, while a false Go pollutes the evidence diagnostics depend on. + */ + private static final String QUALIFIED_GO = "(?:Golang|goroutines?" + + "|Go[\\s-]?(?:developer|engineer|programming|language|module|routine)s?" + + "|" + LANGUAGE_TOKEN + LANGUAGE_DELIMITER + "Go" + LANGUAGE_DELIMITER + LANGUAGE_TOKEN + + "|" + TECHNICAL_INTRODUCER + "\\s*:?\\s*(?:" + LANGUAGE_TOKEN + LANGUAGE_DELIMITER + + "){0,3}Go" + GO_LIST_TAIL + + ")"; + private static final Map TECHNOLOGIES = technologies(); private static final List PROGRAMMING = List.of("Java", "TypeScript", "JavaScript", "Python", "Kotlin", "C#", "C++", "Go"); // Whole words only: "internal" or "international" must not count as an internship. @@ -96,17 +166,40 @@ public ExtractedRequirements extract(Job job) { salary(text), job.getDeadline(), "DETERMINISTIC"); } + /** + * Insertion-ordered on purpose. {@link Map#copyOf} salts its iteration order per JVM run, so + * the extracted technology list — and therefore {@link ExtractedRequirements} record equality — + * used to differ between runs for identical text. Nothing depended on that while the rescore + * plan compared scores only; requirement-aware plan membership does. + */ private static Map technologies() { Map map = new LinkedHashMap<>(); - for (String tech : List.of("Java", "Spring Boot", "Spring MVC", "Spring Security", "REST", "SQL", - "PostgreSQL", "JPA", "Hibernate", "Maven", "JUnit", "Mockito", "React", "React Native", - "TypeScript", "JavaScript", "HTML", "CSS", "Git", "GitHub Actions", "CI/CD", "Docker", - "Kubernetes", "Python", "Kotlin", "C#", "C++", "Go", "AWS", "Azure", "GCP")) { - String token = "CI/CD".equals(tech) ? "CI(?:/|\\s*)CD" : Pattern.quote(tech); - String expression = "(?i)(? "CI(?:/|\\s*)CD"; + case "REST" -> QUALIFIED_REST; + case "Go" -> QUALIFIED_GO; + case "Spring Boot" -> "Spring[\\s-]?Boot"; + default -> Pattern.quote(tech); + }; } private List extractTechnologies(String text) { diff --git a/src/main/java/com/jobpilot/matching/preview/ScoreRescorePreviewReport.java b/src/main/java/com/jobpilot/matching/preview/ScoreRescorePreviewReport.java index 9934e91..829a944 100644 --- a/src/main/java/com/jobpilot/matching/preview/ScoreRescorePreviewReport.java +++ b/src/main/java/com/jobpilot/matching/preview/ScoreRescorePreviewReport.java @@ -5,12 +5,13 @@ import com.jobpilot.matching.ScoreBand; import java.util.List; import java.util.Map; +import java.util.Objects; /** Immutable, deterministic result; it contains no description, URL, or candidate data. */ public record ScoreRescorePreviewReport( int inspectedJobs, int exactMatches, - int changedScoreCount, + int scoreDeltaChangedCount, int changedBandCount, int blockerRemovedCount, int blockerAddedCount, @@ -24,7 +25,84 @@ public record ScoreRescorePreviewReport( QueueProjection matchesQueue, QueueProjection reviewQueue, List changedJobs, - JobPreview juniorJavaDeveloper) { + JobPreview juniorJavaDeveloper, + ChangeCounts changeCounts, + List requirementsChangedJobs) { + + /** + * Plan membership, decomposed. {@code scoreDeltaChangedCount} above counts a numeric score + * move; {@code scoreChangedCount} here counts a {@code ScoreCard} identity change, which is + * what actually decides whether a row is written. + */ + public record ChangeCounts( + int scoreChangedCount, + int requirementsChangedCount, + int requirementsOnlyChangedCount, + int changedPlanCount, + int unchangedCount) { + } + + /** Every persisted component of {@code ExtractedRequirements}, in record order. */ + public enum RequirementField { + SENIORITY, + INTERNSHIP_OR_TRAINEE, + REQUIRED_EXPERIENCE_YEARS, + REQUIRED_EDUCATION, + FINAL_YEAR_MANDATORY, + TECHNOLOGIES, + PROGRAMMING_LANGUAGES, + SPOKEN_LANGUAGES, + LOCATION, + REMOTE_ELIGIBILITY, + MENTORSHIP_SIGNALS, + WORK_AUTHORIZATION, + SALARY, + APPLICATION_DEADLINE, + EXTRACTION_METHOD + } + + /** One differing persisted field. Both values arrive already sanitized and bounded. */ + public record RequirementFieldChange( + RequirementField field, + String storedValue, + String computedValue) { + + public static final int MAX_VALUE_LENGTH = 80; + + public RequirementFieldChange { + Objects.requireNonNull(field, "Requirement field is required"); + requireBounded(storedValue, "Stored"); + requireBounded(computedValue, "Computed"); + } + + private static void requireBounded(String value, String side) { + if (value == null || value.length() > MAX_VALUE_LENGTH) { + throw new IllegalArgumentException( + side + " requirement value must be sanitized and bounded"); + } + } + } + + /** + * Why one row entered the plan. Carries no description, URL, or candidate data, and cannot be + * constructed without naming at least one field, so a planned row can never be invisible. + */ + public record RequirementsPreview( + long jobId, + String title, + boolean scoreChanged, + int storedScore, + int computedScore, + List changedFields) { + + public RequirementsPreview { + changedFields = List.copyOf(changedFields); + if (changedFields.isEmpty()) { + throw new IllegalArgumentException( + "A requirements preview must name at least one changed field"); + } + } + } public record BoundaryCrossings( List unsuitable, diff --git a/src/main/java/com/jobpilot/matching/preview/ScoreRescorePreviewReportRenderer.java b/src/main/java/com/jobpilot/matching/preview/ScoreRescorePreviewReportRenderer.java index 48cf160..74ffd1c 100644 --- a/src/main/java/com/jobpilot/matching/preview/ScoreRescorePreviewReportRenderer.java +++ b/src/main/java/com/jobpilot/matching/preview/ScoreRescorePreviewReportRenderer.java @@ -4,6 +4,8 @@ import com.jobpilot.matching.preview.ScoreRescorePreviewReport.JobPreview; import com.jobpilot.matching.preview.ScoreRescorePreviewReport.QueueEntry; import com.jobpilot.matching.preview.ScoreRescorePreviewReport.QueueProjection; +import com.jobpilot.matching.preview.ScoreRescorePreviewReport.RequirementFieldChange; +import com.jobpilot.matching.preview.ScoreRescorePreviewReport.RequirementsPreview; import java.util.ArrayList; import java.util.List; import org.springframework.stereotype.Component; @@ -24,7 +26,7 @@ public List render(ScoreRescorePreviewResult result) { List lines = new ArrayList<>(); lines.add("SCORE_RESCORE_PREVIEW status=SUCCESS inspected=" + report.inspectedJobs() + " exact=" + report.exactMatches() - + " changedScores=" + report.changedScoreCount() + + " scoreDeltaChanged=" + report.scoreDeltaChangedCount() + " changedBands=" + report.changedBandCount() + " blockerRemoved=" + report.blockerRemovedCount() + " blockerAdded=" + report.blockerAddedCount() @@ -32,6 +34,13 @@ public List render(ScoreRescorePreviewResult result) { + " decreases=" + report.scoreDecreaseCount() + " zeroToPositive=" + report.storedZeroToPositiveCount() + " positiveToZero=" + report.storedPositiveToZeroCount()); + lines.add("SCORE_RESCORE_PREVIEW_PLAN scoreChanged=" + + report.changeCounts().scoreChangedCount() + + " requirementsChanged=" + report.changeCounts().requirementsChangedCount() + + " requirementsOnlyChanged=" + + report.changeCounts().requirementsOnlyChangedCount() + + " changedPlan=" + report.changeCounts().changedPlanCount() + + " unchanged=" + report.changeCounts().unchangedCount()); lines.add(bound("SCORE_RESCORE_PREVIEW_DELTA distribution=" + report.scoreDeltaDistribution())); lines.add(bound("SCORE_RESCORE_PREVIEW_BOUNDARIES unsuitable=" @@ -43,6 +52,9 @@ public List render(ScoreRescorePreviewResult result) { for (JobPreview job : report.changedJobs()) { lines.add(renderJob("SCORE_RESCORE_PREVIEW_CHANGED", job)); } + for (RequirementsPreview requirements : report.requirementsChangedJobs()) { + lines.add(renderRequirements(requirements)); + } renderQueue(lines, "matches", report.matchesQueue()); renderQueue(lines, "review", report.reviewQueue()); if (report.juniorJavaDeveloper() == null) { @@ -81,6 +93,23 @@ private String renderJob(String prefix, JobPreview job) { + "->" + position(job.newQueuePosition())); } + /** Names every persisted field the write would change, so no planned row is opaque. */ + private String renderRequirements(RequirementsPreview requirements) { + StringBuilder line = new StringBuilder("SCORE_RESCORE_PREVIEW_REQUIREMENTS jobId=") + .append(requirements.jobId()) + .append(" title=").append(quote(requirements.title())) + .append(" scoreChanged=").append(requirements.scoreChanged()) + .append(" score=").append(requirements.storedScore()) + .append("->").append(requirements.computedScore()) + .append(" changedFields=").append(requirements.changedFields().size()); + for (RequirementFieldChange change : requirements.changedFields()) { + line.append(' ').append(change.field()).append('=') + .append(quote(change.storedValue())) + .append("->").append(quote(change.computedValue())); + } + return bound(line.toString()); + } + private void renderQueue(List lines, String name, QueueProjection queue) { lines.add("SCORE_RESCORE_PREVIEW_QUEUE queue=/" + name + " phase=BEFORE total=" + queue.before().size()); diff --git a/src/main/java/com/jobpilot/matching/preview/ScoreRescorePreviewService.java b/src/main/java/com/jobpilot/matching/preview/ScoreRescorePreviewService.java index 3ca2652..a4cddde 100644 --- a/src/main/java/com/jobpilot/matching/preview/ScoreRescorePreviewService.java +++ b/src/main/java/com/jobpilot/matching/preview/ScoreRescorePreviewService.java @@ -18,13 +18,19 @@ import com.jobpilot.matching.rescore.ScoreRescorePlan; import com.jobpilot.matching.rescore.ScoreRescorePlanEntry; import com.jobpilot.matching.rescore.ScoreRescorePlanResult; +import com.jobpilot.common.Utf16; import com.jobpilot.matching.preview.ScoreRescorePreviewReport.BoundaryCrossings; +import com.jobpilot.matching.preview.ScoreRescorePreviewReport.ChangeCounts; import com.jobpilot.matching.preview.ScoreRescorePreviewReport.JobPreview; import com.jobpilot.matching.preview.ScoreRescorePreviewReport.QueueEntry; import com.jobpilot.matching.preview.ScoreRescorePreviewReport.QueueProjection; +import com.jobpilot.matching.preview.ScoreRescorePreviewReport.RequirementField; +import com.jobpilot.matching.preview.ScoreRescorePreviewReport.RequirementFieldChange; +import com.jobpilot.matching.preview.ScoreRescorePreviewReport.RequirementsPreview; import com.jobpilot.matching.preview.ScoreRescorePreviewReport.ScoreSnapshot; import com.jobpilot.matching.preview.ScoreRescorePreviewResult.ErrorCategory; import java.time.Instant; +import java.time.format.DateTimeFormatter; import java.util.ArrayList; import java.util.Collections; import java.util.Comparator; @@ -48,6 +54,10 @@ @Service public class ScoreRescorePreviewService { static final String TARGET_TITLE = "Code First Girls Programme - Junior Java Developer"; + private static final int MAX_LIST_ELEMENTS = 12; + private static final int MAX_LIST_ELEMENT_LENGTH = 24; + /** What {@link PreviewTextSanitizer} returns once a value normalises to nothing. */ + private static final String BLANK_SANITIZER_MARKER = ""; private final JobScoreRepository scores; private final JobRequirementRepository requirements; @@ -225,11 +235,31 @@ private ScoreRescorePreviewReport buildReport(List jobs) { List possibleCrossings = new ArrayList<>(); List strongCrossings = new ArrayList<>(); List changes = new ArrayList<>(); + List requirementChanges = new ArrayList<>(); + int scoreChanged = 0; + int requirementsChanged = 0; + int requirementsOnlyChanged = 0; + int unchanged = 0; JobPreview named = null; for (EvaluatedJob evaluated : jobs) { ScoreCard oldScore = evaluated.storedScore(); ScoreCard newScore = evaluated.computed().score(); + ExtractedRequirements oldRequirements = evaluated.storedRequirements(); + ExtractedRequirements newRequirements = evaluated.computed().requirements(); + // Identical to the equality that decides plan membership, so the two can never diverge. + boolean scoreDiffers = !oldScore.equals(newScore); + boolean requirementsDiffer = !oldRequirements.equals(newRequirements); + if (scoreDiffers) scoreChanged++; + if (requirementsDiffer) requirementsChanged++; + if (requirementsDiffer && !scoreDiffers) requirementsOnlyChanged++; + if (!scoreDiffers && !requirementsDiffer) unchanged++; + if (requirementsDiffer) { + requirementChanges.add(new RequirementsPreview(evaluated.job().getId(), + PreviewTextSanitizer.text(evaluated.job().getTitle(), 120), scoreDiffers, + oldScore.score(), newScore.score(), + requirementDiff(oldRequirements, newRequirements))); + } int delta = newScore.score() - oldScore.score(); deltas.merge(delta, 1, Integer::sum); if (oldScore.equals(newScore)) exact++; @@ -263,7 +293,96 @@ private ScoreRescorePreviewReport buildReport(List jobs) { Collections.unmodifiableMap(orderedDeltas), zeroToPositive, positiveToZero, new BoundaryCrossings(List.copyOf(unsuitableCrossings), List.copyOf(possibleCrossings), List.copyOf(strongCrossings)), - staleMatchInversions(jobs), matches, review, List.copyOf(changes), named); + staleMatchInversions(jobs), matches, review, List.copyOf(changes), named, + new ChangeCounts(scoreChanged, requirementsChanged, requirementsOnlyChanged, + scoreChanged + requirementsOnlyChanged, unchanged), + List.copyOf(requirementChanges)); + } + + /** + * Every persisted requirement field that differs, and only those. + * + *

Raw values are compared first and converted to display text only after a difference is + * established: comparing rendered text would hide two long values that share a truncated + * prefix. Equality mirrors {@code ExtractedRequirements} record equality exactly — including + * order-sensitive list comparison — because that is what decides plan membership, so a planned + * row can never produce an empty diff. + */ + private List requirementDiff(ExtractedRequirements stored, + ExtractedRequirements computed) { + List changes = new ArrayList<>(); + for (RequirementField field : RequirementField.values()) { + Object storedValue = raw(stored, field); + Object computedValue = raw(computed, field); + if (Objects.equals(storedValue, computedValue)) continue; + changes.add(new RequirementFieldChange(field, + display(storedValue), display(computedValue))); + } + return List.copyOf(changes); + } + + /** Exhaustive by construction: a new record component cannot compile without a branch here. */ + private Object raw(ExtractedRequirements value, RequirementField field) { + return switch (field) { + case SENIORITY -> value.seniority(); + case INTERNSHIP_OR_TRAINEE -> value.internshipOrTrainee(); + case REQUIRED_EXPERIENCE_YEARS -> value.requiredExperienceYears(); + case REQUIRED_EDUCATION -> value.requiredEducation(); + case FINAL_YEAR_MANDATORY -> value.finalYearMandatory(); + case TECHNOLOGIES -> value.technologies(); + case PROGRAMMING_LANGUAGES -> value.programmingLanguages(); + case SPOKEN_LANGUAGES -> value.spokenLanguages(); + case LOCATION -> value.location(); + case REMOTE_ELIGIBILITY -> value.remoteEligibility(); + case MENTORSHIP_SIGNALS -> value.mentorshipSignals(); + case WORK_AUTHORIZATION -> value.workAuthorization(); + case SALARY -> value.salary(); + case APPLICATION_DEADLINE -> value.applicationDeadline(); + case EXTRACTION_METHOD -> value.extractionMethod(); + }; + } + + private String display(Object value) { + if (value == null) return "(null)"; + if (value instanceof List list) return displayList(list); + if (value instanceof Instant instant) { + return DateTimeFormatter.ISO_INSTANT.format(instant); + } + if (value instanceof String text) return boundedText(text); + return bounded(String.valueOf(value)); + } + + /** + * Normalise first, then decide emptiness, so "", " " and control-only text all read the + * same while staying distinct from a genuinely absent "(null)". Raw equality has already been + * decided by the caller, so two different raw values that both normalise to nothing are still + * reported as a change. + */ + private String boundedText(String value) { + String safe = bounded(value); + return safe.isEmpty() || BLANK_SANITIZER_MARKER.equals(safe) ? "(empty)" : safe; + } + + private String displayList(List values) { + if (values.isEmpty()) return "[]"; + StringBuilder rendered = new StringBuilder("["); + for (int index = 0; index < Math.min(values.size(), MAX_LIST_ELEMENTS); index++) { + if (index > 0) rendered.append(", "); + rendered.append(PreviewTextSanitizer.text( + String.valueOf(values.get(index)), MAX_LIST_ELEMENT_LENGTH)); + } + if (values.size() > MAX_LIST_ELEMENTS) { + rendered.append(", +").append(values.size() - MAX_LIST_ELEMENTS).append(" more"); + } + return bounded(rendered.append(']').toString()); + } + + /** Visibly truncated, so an operator can tell a cut value from a short one. */ + private String bounded(String value) { + String safe = PreviewTextSanitizer.text(value, RequirementFieldChange.MAX_VALUE_LENGTH + 1); + return safe.length() > RequirementFieldChange.MAX_VALUE_LENGTH + ? Utf16.truncate(safe, RequirementFieldChange.MAX_VALUE_LENGTH - 1) + "…" + : safe; } private JobPreview preview(EvaluatedJob evaluated, Map oldPositions, @@ -468,7 +587,8 @@ private ScoreRescorePreviewReport emptyReport() { QueueProjection emptyQueue = new QueueProjection(List.of(), List.of()); return new ScoreRescorePreviewReport(0, 0, 0, 0, 0, 0, 0, 0, Map.of(), 0, 0, new BoundaryCrossings(List.of(), List.of(), List.of()), List.of(), - emptyQueue, emptyQueue, List.of(), null); + emptyQueue, emptyQueue, List.of(), null, + new ChangeCounts(0, 0, 0, 0, 0), List.of()); } private PreviewAbort abort(ErrorCategory category, String safeMessage) { diff --git a/src/main/java/com/jobpilot/matching/rescore/ScoreRescorePlan.java b/src/main/java/com/jobpilot/matching/rescore/ScoreRescorePlan.java index 9ffb26b..1d13733 100644 --- a/src/main/java/com/jobpilot/matching/rescore/ScoreRescorePlan.java +++ b/src/main/java/com/jobpilot/matching/rescore/ScoreRescorePlan.java @@ -20,6 +20,13 @@ public ScoreRescorePlan(ScoreRescorePreviewReport report, public ScoreRescorePlan { Objects.requireNonNull(report, "Preview report is required"); entries = normalized(entries); + // The write may only touch rows the preview accounted for, so the two are tied here + // rather than left to agree by convention. + if (report.changeCounts() == null + || report.changeCounts().changedPlanCount() != entries.size()) { + throw new IllegalArgumentException( + "Plan entry count does not match the preview report changed plan count"); + } if (!ScoreRescorePlanFingerprint.fingerprint(entries).equals(fingerprint)) { throw new IllegalArgumentException("Plan fingerprint does not match its entries"); } diff --git a/src/main/java/com/jobpilot/matching/rescore/ScoreRescorePlanEntry.java b/src/main/java/com/jobpilot/matching/rescore/ScoreRescorePlanEntry.java index 9330151..4ab6fb2 100644 --- a/src/main/java/com/jobpilot/matching/rescore/ScoreRescorePlanEntry.java +++ b/src/main/java/com/jobpilot/matching/rescore/ScoreRescorePlanEntry.java @@ -63,10 +63,25 @@ public static ScoreRescorePlanEntry snapshot(JobScore scoreRow, computed.score(), computed.requirements()); } + /** + * A row is planned when either persisted projection would move. The write transaction already + * re-persists requirements whenever they differ, so a score-only test let it change rows the + * preview never showed, while requirement drift carrying no score change — a stale seniority, + * a withdrawn dirty technology token — stayed permanently unreachable. + */ public boolean changed() { + return !storedScore.equals(computedScore) + || !storedRequirements.equals(computedRequirements); + } + + public boolean scoreChanged() { return !storedScore.equals(computedScore); } + public boolean requirementsChanged() { + return !storedRequirements.equals(computedRequirements); + } + private static long requireId(Long id, String type) { if (id == null || id < 1) { throw new IllegalArgumentException("Persisted " + type + " identity is required"); diff --git a/src/test/java/com/jobpilot/extraction/DeterministicRequirementExtractorTest.java b/src/test/java/com/jobpilot/extraction/DeterministicRequirementExtractorTest.java index 3627fa3..55b877c 100644 --- a/src/test/java/com/jobpilot/extraction/DeterministicRequirementExtractorTest.java +++ b/src/test/java/com/jobpilot/extraction/DeterministicRequirementExtractorTest.java @@ -11,7 +11,10 @@ import java.time.Clock; import java.time.Instant; import java.time.ZoneOffset; +import java.util.List; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; class DeterministicRequirementExtractorTest { private final Clock clock = Clock.fixed(Instant.parse("2026-07-17T12:00:00Z"), ZoneOffset.UTC); @@ -280,6 +283,241 @@ void productionRegressionFixturesForJobs86And98() { .isEqualTo("UNKNOWN"); } + // ---------------------------------------------------------------- extractor hygiene + + private List technologiesOf(String description) { + return extractor.extract(jobWithDescription("Software Engineer", description)) + .technologies(); + } + + @ParameterizedTest + @ValueSource(strings = { + "We build REST API services", + "Design REST APIs for partners", + "Experience with REST service design", + "Owns several REST services", + "Implements a REST endpoint", + "Documented REST endpoints", + "Writes a REST controller", + "Several REST controllers in Spring", + "Strong RESTful experience", + "Builds RESTful API layers", + "Uses JAX-RS in production", + "Spring REST is used throughout" + }) + void extractsQualifiedRestEvidence(String description) { + assertThat(technologiesOf(description)).contains("REST"); + } + + @ParameterizedTest + @ValueSource(strings = { + "HTTP/REST is used across services", + "Networking protocols (TCP/IP, DHCP, HTTP / REST)", + "Experience with REST/JSON and Git", + "Payloads use REST / JSON everywhere", + "Integrating public APIs (REST)", + "Exposes an API (REST) for partners", + "Exposes an API ( REST ) for partners" + }) + void extractsTechnicalRestShorthand(String description) { + assertThat(technologiesOf(description)).contains("REST"); + } + + @ParameterizedTest + @ValueSource(strings = { + "Experience with REST is required", + "Experience in REST is required", + "Knowledge of REST is required", + "Understanding of REST is required", + "Familiarity with REST is required", + "Proficiency in REST is required", + "Skills with REST are required" + }) + void extractsRestAfterATechnicalIntroducer(String description) { + assertThat(technologiesOf(description)).contains("REST"); + } + + @ParameterizedTest + @ValueSource(strings = { + "Experience and understanding of OO, MVC, REST.", + "Solid grasp of OOP, MVC, REST", + "Java, Spring Boot, REST, SQL, PostgreSQL", + "technologies: Java, REST, SQL", + "stack: Java, REST, SQL" + }) + void extractsRestAsATechnologyListItem(String description) { + assertThat(technologiesOf(description)).contains("REST"); + } + + @ParameterizedTest + @ValueSource(strings = { + "Integration with the rest of the stack", + "ready for integration with the rest of the stack", + "Automation with Rest Assured", + "Automation with RestAssured", + "learn tools such as Selenium, Cucumber, BDD, Rest Assured and CI/CD", + "Consumes the interest API daily", + "Calls the forest API nightly", + "Integrates a restaurant API", + "leave the rest for later", + "introduce you to the rest of the team", + "rest of your responsibilities are listed below", + "experience with the rest of the stack" + }) + void rejectsUnqualifiedRestEvidence(String description) { + assertThat(technologiesOf(description)).doesNotContain("REST"); + } + + @Test + void extractsRestWhenDirtyAndGenuineEvidenceBothOccur() { + assertThat(technologiesOf("Automation with Rest Assured over the rest of the stack, " + + "plus a genuine REST API layer")).contains("REST"); + assertThat(technologiesOf("Rest Assured for testing; services expose REST APIs")) + .contains("REST"); + } + + /** Verbatim contexts from the nine production rows the audit surfaced. */ + @ParameterizedTest + @ValueSource(strings = { + "Experience and understanding of OO, MVC, REST.", + "Networking protocols and technologies (TCP/IP, DHCP, HTTP/REST)", + "Experience with REST/JSON and Git", + "Experience in exposing and integrating public APIs (REST)", + "Java, Spring Boot, REST, SQL, PostgreSQL" + }) + void keepsGenuineProductionRestEvidence(String description) { + assertThat(technologiesOf(description)).contains("REST"); + } + + @ParameterizedTest + @ValueSource(strings = { + "learn tools such as Selenium, Cucumber, BDD, Rest Assured and CI/CD", + "ready for integration with the rest of the stack" + }) + void dropsDirtyProductionRestEvidence(String description) { + assertThat(technologiesOf(description)).doesNotContain("REST"); + } + + @ParameterizedTest + @ValueSource(strings = { + "Golang experience preferred", + "Understands goroutine scheduling", + "Tuning goroutines at scale", + "Hiring a Go developer", + "Hiring a Go engineer", + "Go programming is a plus", + "The Go language is used here", + "Uses Go modules", + "Knows Go routines", + "Python, Go, Java", + "languages: Go, Rust", + "programming languages: Go, Rust", + "technologies: Go, Rust", + "tech stack: Go, Rust", + "stack: Go, Rust", + "experience with Go and Rust", + "experience in Go and Rust", + "proficient with Go, Rust", + "proficient in Go, Rust", + "written in Go, mostly", + "skills with Go and Rust", + "skills in Go and Rust" + }) + void extractsGoOnlyWithTechnicalContext(String description) { + assertThat(technologiesOf(description)).contains("Go"); + } + + @ParameterizedTest + @ValueSource(strings = { + "ready to go, Java developers should apply", + "go and Java developers will review it", + "Python developers go and Java developers follow", + "ready to go / Java experience preferred", + "go, Java is installed separately", + "experience with Java and go to market", + "please go ahead and apply", + "willing to go the extra mile", + "supports go-live weekends", + "experience with go-to-market strategy" + }) + void rejectsOrdinaryEnglishGo(String description) { + assertThat(technologiesOf(description)).doesNotContain("Go"); + } + + @ParameterizedTest + @ValueSource(strings = { + "Experience with Spring Boot 3", + "Experience with SpringBoot microservices", + "We use Spring-Boot in production" + }) + void canonicalisesSpringBootAliases(String description) { + assertThat(technologiesOf(description)).contains("Spring Boot"); + } + + @Test + void extractsJvmAsItsOwnCanonicalTerm() { + assertThat(technologiesOf("Services run on the JVM")) + .contains("JVM").doesNotContain("Java"); + } + + @Test + void jvmIsNotAProgrammingLanguage() { + var requirements = extractor.extract( + jobWithDescription("Software Engineer", "Runs on the JVM, written in Go, mostly")); + + assertThat(requirements.technologies()).contains("JVM", "Go"); + assertThat(requirements.programmingLanguages()).contains("Go").doesNotContain("JVM"); + } + + /** Deliberate deferral: the Postgres alias waits for the scoring redesign, not this patch. */ + @Test + void extractsExactPostgresqlButNotThePostgresAlias() { + assertThat(technologiesOf("We run PostgreSQL 16 here")).contains("PostgreSQL"); + assertThat(technologiesOf("Data stored in Postgres")).doesNotContain("PostgreSQL"); + } + + @Test + void keepsExistingWordBoundaryBehaviour() { + assertThat(technologiesOf("Strong JavaScript skills only")) + .contains("JavaScript").doesNotContain("Java"); + assertThat(technologiesOf("Core Java expertise")).contains("Java"); + assertThat(technologiesOf("Pipelines in GitHub Actions")) + .contains("GitHub Actions").doesNotContain("Git"); + assertThat(technologiesOf("Version control with Git")).contains("Git"); + assertThat(technologiesOf("MySQL and NoSQL stores")).doesNotContain("SQL"); + assertThat(technologiesOf("Strong SQL skills")).contains("SQL"); + assertThat(technologiesOf("Pipelines use CI/CD")).contains("CI/CD"); + assertThat(technologiesOf("Pipelines use CI CD")).contains("CI/CD"); + assertThat(technologiesOf("Pipelines use CICD")).contains("CI/CD"); + assertThat(technologiesOf("Backends in C# and C++")).contains("C#", "C++"); + } + + /** The terms this patch does not touch must keep matching exactly as before. */ + @Test + void keepsEveryUntouchedTechnologyWorking() { + assertThat(technologiesOf("Java, Spring MVC, Spring Security, SQL, PostgreSQL, JPA, " + + "Hibernate, Maven, JUnit, Mockito, React, React Native, TypeScript, " + + "JavaScript, HTML, CSS, Git, GitHub Actions, CI/CD, Docker, Kubernetes, " + + "Python, Kotlin, C#, C++, AWS, Azure and GCP")) + .contains("Java", "Spring MVC", "Spring Security", "SQL", "PostgreSQL", "JPA", + "Hibernate", "Maven", "JUnit", "Mockito", "React", "React Native", + "TypeScript", "JavaScript", "HTML", "CSS", "Git", "GitHub Actions", + "CI/CD", "Docker", "Kubernetes", "Python", "Kotlin", "C#", "C++", + "AWS", "Azure", "GCP"); + } + + /** + * Record equality is order-sensitive and now decides rescore plan membership, so extraction + * order must be stable rather than salted per JVM run. + */ + @Test + void extractsTechnologiesInAStableDeclarationOrder() { + String description = "Kubernetes, Docker, React, PostgreSQL and Java"; + + assertThat(technologiesOf(description)) + .containsExactly("Java", "PostgreSQL", "React", "Docker", "Kubernetes"); + } + private Job jobWithDescription(String title, String description) { return normalizer.normalize(new RawJob("fixture", "text-1", "https://example.com/jobs/text-1", title, "Example", "Bucharest, Romania", description, null, diff --git a/src/test/java/com/jobpilot/matching/preview/RequirementDiffTest.java b/src/test/java/com/jobpilot/matching/preview/RequirementDiffTest.java new file mode 100644 index 0000000..8d6b28e --- /dev/null +++ b/src/test/java/com/jobpilot/matching/preview/RequirementDiffTest.java @@ -0,0 +1,244 @@ +package com.jobpilot.matching.preview; + +import static org.assertj.core.api.Assertions.assertThat; + +import com.jobpilot.jobs.domain.ExtractedRequirements; +import com.jobpilot.matching.preview.ScoreRescorePreviewReport.RequirementField; +import com.jobpilot.matching.preview.ScoreRescorePreviewReport.RequirementFieldChange; +import java.lang.reflect.Method; +import java.time.Instant; +import java.util.List; +import java.util.Locale; +import org.junit.jupiter.api.Test; + +/** + * The requirement diff is what makes a planned row explainable before a guarded write, so it is + * exercised directly rather than only through the preview service. + */ +class RequirementDiffTest { + private final ScoreRescorePreviewService service = + new ScoreRescorePreviewService(null, null, null, null); + + private List diff(ExtractedRequirements stored, + ExtractedRequirements computed) { + return ReflectionTestSupport.requirementDiff(service, stored, computed); + } + + @Test + void reportsTechnologiesOnly() { + var stored = base(); + var computed = withTechnologies(base(), List.of("Java", "REST")); + + assertThat(diff(stored, computed)).singleElement().satisfies(change -> { + assertThat(change.field()).isEqualTo(RequirementField.TECHNOLOGIES); + assertThat(change.storedValue()).isEqualTo("[Java]"); + assertThat(change.computedValue()).isEqualTo("[Java, REST]"); + }); + } + + @Test + void reportsProgrammingLanguagesOnly() { + var computed = new ExtractedRequirements("JUNIOR", false, null, null, false, + List.of("Java"), List.of("Java", "Go"), List.of(), "Bucharest", + "Romania eligible", List.of(), null, null, null, "DETERMINISTIC"); + + assertThat(diff(base(), computed)).singleElement() + .satisfies(change -> assertThat(change.field()) + .isEqualTo(RequirementField.PROGRAMMING_LANGUAGES)); + } + + /** The documented pre-existing drift: stored JUNIOR, freshly UNKNOWN. */ + @Test + void reportsSeniorityOnly() { + var computed = withSeniority(base(), "UNKNOWN"); + + assertThat(diff(base(), computed)).singleElement().satisfies(change -> { + assertThat(change.field()).isEqualTo(RequirementField.SENIORITY); + assertThat(change.storedValue()).isEqualTo("JUNIOR"); + assertThat(change.computedValue()).isEqualTo("UNKNOWN"); + }); + } + + @Test + void reportsEveryDifferingFieldInRecordOrder() { + var computed = new ExtractedRequirements("UNKNOWN", true, 2.0, "BSc", true, + List.of("Java", "Go"), List.of("Java", "Go"), List.of("English (B2)"), "Cluj", + "Remote from Romania allowed", List.of("mentor"), "visa", "1000 EUR", + Instant.parse("2026-09-01T00:00:00Z"), "DETERMINISTIC"); + + assertThat(diff(base(), computed)).extracting(RequirementFieldChange::field) + .containsExactly(RequirementField.SENIORITY, + RequirementField.INTERNSHIP_OR_TRAINEE, + RequirementField.REQUIRED_EXPERIENCE_YEARS, + RequirementField.REQUIRED_EDUCATION, + RequirementField.FINAL_YEAR_MANDATORY, + RequirementField.TECHNOLOGIES, + RequirementField.PROGRAMMING_LANGUAGES, + RequirementField.SPOKEN_LANGUAGES, + RequirementField.LOCATION, + RequirementField.REMOTE_ELIGIBILITY, + RequirementField.MENTORSHIP_SIGNALS, + RequirementField.WORK_AUTHORIZATION, + RequirementField.SALARY, + RequirementField.APPLICATION_DEADLINE); + } + + @Test + void reportsNothingWhenRequirementsAreIdentical() { + assertThat(diff(base(), base())).isEmpty(); + } + + /** Record equality is order-sensitive, so a reorder is a real change and must be shown. */ + @Test + void reportsListReorderAsAChange() { + var computed = withTechnologies(base(), List.of("REST", "Java")); + var stored = withTechnologies(base(), List.of("Java", "REST")); + + assertThat(diff(stored, computed)).singleElement() + .satisfies(change -> assertThat(change.field()) + .isEqualTo(RequirementField.TECHNOLOGIES)); + } + + /** Comparison happens before truncation, so a late difference cannot be hidden. */ + @Test + void reportsValuesDifferingOnlyBeyondTheDisplayLimit() { + String prefix = "x".repeat(RequirementFieldChange.MAX_VALUE_LENGTH + 20); + var stored = withEducation(base(), prefix + "AAA"); + var computed = withEducation(base(), prefix + "BBB"); + + assertThat(diff(stored, computed)).singleElement().satisfies(change -> { + assertThat(change.field()).isEqualTo(RequirementField.REQUIRED_EDUCATION); + assertThat(change.storedValue()).hasSizeLessThanOrEqualTo( + RequirementFieldChange.MAX_VALUE_LENGTH); + assertThat(change.storedValue()).endsWith("…"); + }); + } + + @Test + void distinguishesNullFromEmpty() { + var stored = withEducation(base(), null); + var computed = withEducation(base(), ""); + + assertThat(diff(stored, computed)).singleElement().satisfies(change -> { + assertThat(change.storedValue()).isEqualTo("(null)"); + assertThat(change.computedValue()).isEqualTo("(empty)"); + }); + } + + @Test + void rendersWhitespaceOnlyValuesAsEmpty() { + assertThat(diff(base(), withEducation(base(), " "))).singleElement() + .satisfies(change -> { + assertThat(change.storedValue()).isEqualTo("(null)"); + assertThat(change.computedValue()).isEqualTo("(empty)"); + }); + assertThat(diff(base(), withEducation(base(), "\n\t "))).singleElement() + .satisfies(change -> assertThat(change.computedValue()).isEqualTo("(empty)")); + } + + @Test + void collapsesAndStripsWhitespaceAroundRealText() { + assertThat(diff(base(), withEducation(base(), " BSc Computer\n Science "))) + .singleElement() + .satisfies(change -> assertThat(change.computedValue()) + .isEqualTo("BSc Computer Science")); + } + + /** Raw equality decides the change; rendering only explains it afterwards. */ + @Test + void detectsAChangeBetweenTwoValuesThatBothRenderAsEmpty() { + var stored = withEducation(base(), ""); + var computed = withEducation(base(), " "); + + assertThat(diff(stored, computed)).singleElement().satisfies(change -> { + assertThat(change.field()).isEqualTo(RequirementField.REQUIRED_EDUCATION); + assertThat(change.storedValue()).isEqualTo("(empty)"); + assertThat(change.computedValue()).isEqualTo("(empty)"); + }); + } + + @Test + void boundsLongListsWithAVisibleRemainder() { + List many = List.of("a", "b", "c", "d", "e", "f", "g", "h", "i", "j", "k", "l", + "m", "n"); + + assertThat(diff(base(), withTechnologies(base(), many))).singleElement() + .satisfies(change -> { + assertThat(change.computedValue()).contains("+2 more"); + assertThat(change.computedValue()).hasSizeLessThanOrEqualTo( + RequirementFieldChange.MAX_VALUE_LENGTH); + }); + } + + /** + * A new persisted component must not be able to slip past the diff unnoticed, so the enum is + * pinned to the record's own components. + */ + @Test + void requirementFieldCoversEveryPersistedRecordComponent() { + List components = java.util.Arrays.stream( + ExtractedRequirements.class.getRecordComponents()) + .map(java.lang.reflect.RecordComponent::getName) + .map(RequirementDiffTest::screamingSnake) + .toList(); + + assertThat(java.util.Arrays.stream(RequirementField.values()).map(Enum::name).toList()) + .containsExactlyElementsOf(components); + } + + private static String screamingSnake(String camelCase) { + return camelCase.replaceAll("([a-z0-9])([A-Z])", "$1_$2").toUpperCase(Locale.ROOT); + } + + private ExtractedRequirements base() { + return new ExtractedRequirements("JUNIOR", false, null, null, false, + List.of("Java"), List.of("Java"), List.of(), "Bucharest", + "Romania eligible", List.of(), null, null, null, "DETERMINISTIC"); + } + + private ExtractedRequirements withSeniority(ExtractedRequirements value, String seniority) { + return new ExtractedRequirements(seniority, value.internshipOrTrainee(), + value.requiredExperienceYears(), value.requiredEducation(), + value.finalYearMandatory(), value.technologies(), value.programmingLanguages(), + value.spokenLanguages(), value.location(), value.remoteEligibility(), + value.mentorshipSignals(), value.workAuthorization(), value.salary(), + value.applicationDeadline(), value.extractionMethod()); + } + + private ExtractedRequirements withTechnologies(ExtractedRequirements value, + List technologies) { + return new ExtractedRequirements(value.seniority(), value.internshipOrTrainee(), + value.requiredExperienceYears(), value.requiredEducation(), + value.finalYearMandatory(), technologies, value.programmingLanguages(), + value.spokenLanguages(), value.location(), value.remoteEligibility(), + value.mentorshipSignals(), value.workAuthorization(), value.salary(), + value.applicationDeadline(), value.extractionMethod()); + } + + private ExtractedRequirements withEducation(ExtractedRequirements value, String education) { + return new ExtractedRequirements(value.seniority(), value.internshipOrTrainee(), + value.requiredExperienceYears(), education, + value.finalYearMandatory(), value.technologies(), value.programmingLanguages(), + value.spokenLanguages(), value.location(), value.remoteEligibility(), + value.mentorshipSignals(), value.workAuthorization(), value.salary(), + value.applicationDeadline(), value.extractionMethod()); + } + + /** The diff is a private implementation detail; the test reaches it without widening it. */ + private static final class ReflectionTestSupport { + @SuppressWarnings("unchecked") + static List requirementDiff(ScoreRescorePreviewService service, + ExtractedRequirements stored, + ExtractedRequirements computed) { + try { + Method method = ScoreRescorePreviewService.class.getDeclaredMethod( + "requirementDiff", ExtractedRequirements.class, + ExtractedRequirements.class); + method.setAccessible(true); + return (List) method.invoke(service, stored, computed); + } catch (ReflectiveOperationException failure) { + throw new IllegalStateException("Could not invoke requirementDiff", failure); + } + } + } +} diff --git a/src/test/java/com/jobpilot/matching/preview/ScoreRescorePreviewReportRendererTest.java b/src/test/java/com/jobpilot/matching/preview/ScoreRescorePreviewReportRendererTest.java new file mode 100644 index 0000000..2c5d860 --- /dev/null +++ b/src/test/java/com/jobpilot/matching/preview/ScoreRescorePreviewReportRendererTest.java @@ -0,0 +1,76 @@ +package com.jobpilot.matching.preview; + +import static org.assertj.core.api.Assertions.assertThat; + +import com.jobpilot.matching.preview.ScoreRescorePreviewReport.BoundaryCrossings; +import com.jobpilot.matching.preview.ScoreRescorePreviewReport.ChangeCounts; +import com.jobpilot.matching.preview.ScoreRescorePreviewReport.QueueProjection; +import com.jobpilot.matching.preview.ScoreRescorePreviewReport.RequirementField; +import com.jobpilot.matching.preview.ScoreRescorePreviewReport.RequirementFieldChange; +import com.jobpilot.matching.preview.ScoreRescorePreviewReport.RequirementsPreview; +import java.util.List; +import java.util.Map; +import org.junit.jupiter.api.Test; + +/** The rendered preview is what an operator approves a write against. */ +class ScoreRescorePreviewReportRendererTest { + private static final String DESCRIPTION = + "Confidential posting body that must never reach an operational log"; + private static final String URL = "https://example.com/jobs/secret-posting"; + + private final ScoreRescorePreviewReportRenderer renderer = + new ScoreRescorePreviewReportRenderer(); + + @Test + void namesEveryChangedRequirementFieldWithBothValues() { + List lines = renderer.render(ScoreRescorePreviewResult.success(report())); + + String requirementsLine = lines.stream() + .filter(line -> line.startsWith("SCORE_RESCORE_PREVIEW_REQUIREMENTS")) + .findFirst().orElseThrow(); + + assertThat(requirementsLine).contains("jobId=42", "scoreChanged=false", "score=56->56", + "changedFields=2", + "SENIORITY=\"JUNIOR\"->\"UNKNOWN\"", + "TECHNOLOGIES=\"[Java, Go]\"->\"[Java]\""); + } + + @Test + void exposesThePlanCountsInTheHeader() { + List lines = renderer.render(ScoreRescorePreviewResult.success(report())); + + assertThat(lines).anySatisfy(line -> assertThat(line).contains( + "SCORE_RESCORE_PREVIEW_PLAN scoreChanged=0", "requirementsChanged=1", + "requirementsOnlyChanged=1", "changedPlan=1", "unchanged=2")); + assertThat(lines).anySatisfy(line -> assertThat(line).contains("scoreDeltaChanged=0")); + } + + @Test + void neverPrintsDescriptionsOrUrls() { + List lines = renderer.render(ScoreRescorePreviewResult.success(report())); + + assertThat(lines).noneSatisfy(line -> assertThat(line).contains(DESCRIPTION)); + assertThat(lines).noneSatisfy(line -> assertThat(line).contains(URL)); + } + + @Test + void keepsEveryLineWithinTheLineBound() { + List lines = renderer.render(ScoreRescorePreviewResult.success(report())); + + assertThat(lines).allSatisfy(line -> assertThat(line.length()) + .isLessThanOrEqualTo(ScoreRescorePreviewReportRenderer.MAX_LINE_LENGTH)); + } + + private ScoreRescorePreviewReport report() { + QueueProjection queue = new QueueProjection(List.of(), List.of()); + RequirementsPreview requirements = new RequirementsPreview(42L, "Java Developer", false, + 56, 56, List.of( + new RequirementFieldChange(RequirementField.SENIORITY, "JUNIOR", "UNKNOWN"), + new RequirementFieldChange(RequirementField.TECHNOLOGIES, + "[Java, Go]", "[Java]"))); + return new ScoreRescorePreviewReport(3, 3, 0, 0, 0, 0, 0, 0, Map.of(), 0, 0, + new BoundaryCrossings(List.of(), List.of(), List.of()), List.of(), + queue, queue, List.of(), null, + new ChangeCounts(0, 1, 1, 1, 2), List.of(requirements)); + } +} diff --git a/src/test/java/com/jobpilot/matching/preview/ScoreRescorePreviewServiceTest.java b/src/test/java/com/jobpilot/matching/preview/ScoreRescorePreviewServiceTest.java index ee47a61..7a14537 100644 --- a/src/test/java/com/jobpilot/matching/preview/ScoreRescorePreviewServiceTest.java +++ b/src/test/java/com/jobpilot/matching/preview/ScoreRescorePreviewServiceTest.java @@ -123,12 +123,16 @@ void genuineSeniorEngineerRemainsBlockedAndUnchanged() { assertThat(result.report().exactMatches()).isEqualTo(1); assertThat(result.report().changedJobs()).isEmpty(); - assertThat(result.report().changedScoreCount()).isZero(); + assertThat(result.report().scoreDeltaChangedCount()).isZero(); assertNoRepositoryWrites(); } + /** + * The write transaction re-persists requirements whenever they differ, so requirements-only + * drift must enter the plan and be visible in the preview instead of being written unseen. + */ @Test - void requirementsOnlyDifferenceDoesNotEnterScoreWritePlan() { + void requirementsOnlyDifferenceEntersThePlanAndIsVisibleInThePreview() { Job target = ordinaryMatch(10L); ScoreCalculation current = calculator.calculate(target); ExtractedRequirements stored = withSalary(current.requirements(), "legacy salary text"); @@ -138,12 +142,87 @@ void requirementsOnlyDifferenceDoesNotEnterScoreWritePlan() { assertThat(result.status()) .isEqualTo(ScoreRescorePlanResult.Status.SUCCESS); - assertThat(result.plan().report().exactMatches()).isEqualTo(1); - assertThat(result.plan().changedCount()).isZero(); + var report = result.plan().report(); + assertThat(report.changeCounts().scoreChangedCount()).isZero(); + assertThat(report.changeCounts().requirementsChangedCount()).isEqualTo(1); + assertThat(report.changeCounts().requirementsOnlyChangedCount()).isEqualTo(1); + assertThat(report.changeCounts().changedPlanCount()).isEqualTo(1); + assertThat(report.changeCounts().unchangedCount()).isZero(); + assertThat(result.plan().changedCount()).isEqualTo(1); + assertThat(report.changedJobs()).isEmpty(); + assertThat(report.requirementsChangedJobs()).singleElement().satisfies(preview -> { + assertThat(preview.jobId()).isEqualTo(10L); + assertThat(preview.scoreChanged()).isFalse(); + assertThat(preview.storedScore()).isEqualTo(preview.computedScore()); + assertThat(preview.changedFields()).singleElement().satisfies(change -> { + assertThat(change.field()) + .isEqualTo(ScoreRescorePreviewReport.RequirementField.SALARY); + assertThat(change.storedValue()).isEqualTo("legacy salary text"); + assertThat(change.computedValue()).isEqualTo("(null)"); + }); + }); + assertNoRepositoryWrites(); + } + + @Test + void unchangedRowEntersNeitherPlanNorPreviewLists() { + Job target = ordinaryMatch(11L); + ScoreCalculation current = calculator.calculate(target); + stub(List.of(row(target, current.score())), + List.of(requirement(target, current.requirements()))); + + var result = service.plan(250); + + var report = result.plan().report(); + assertThat(report.changeCounts().changedPlanCount()).isZero(); + assertThat(report.changeCounts().unchangedCount()).isEqualTo(1); + assertThat(report.changedJobs()).isEmpty(); + assertThat(report.requirementsChangedJobs()).isEmpty(); assertThat(result.plan().entries()).isEmpty(); assertNoRepositoryWrites(); } + /** Every invariant the guarded write relies on, asserted together over one mixed corpus. */ + @Test + void previewCountsAndPlanStayConsistentAcrossMixedChanges() { + Job scoreOnly = juniorProgramme(20L); + ScoreCalculation scoreOnlyCurrent = calculator.calculate(scoreOnly); + Job requirementsOnly = ordinaryMatch(21L); + ScoreCalculation requirementsOnlyCurrent = calculator.calculate(requirementsOnly); + Job unchanged = ordinaryMatch(22L); + ScoreCalculation unchangedCurrent = calculator.calculate(unchanged); + + stub(List.of(row(scoreOnly, staleScore(scoreOnlyCurrent.score())), + row(requirementsOnly, requirementsOnlyCurrent.score()), + row(unchanged, unchangedCurrent.score())), + List.of(requirement(scoreOnly, scoreOnlyCurrent.requirements()), + requirement(requirementsOnly, + withSalary(requirementsOnlyCurrent.requirements(), "legacy")), + requirement(unchanged, unchangedCurrent.requirements()))); + + var plan = service.plan(250).plan(); + var counts = plan.report().changeCounts(); + + assertThat(counts.changedPlanCount()).isEqualTo(plan.changedCount()); + assertThat(counts.scoreChangedCount() + counts.requirementsOnlyChangedCount()) + .isEqualTo(counts.changedPlanCount()); + assertThat(plan.report().inspectedJobs()) + .isEqualTo(counts.changedPlanCount() + counts.unchangedCount()); + assertThat(plan.report().changedJobs()).hasSize(counts.scoreChangedCount()); + assertThat(plan.report().requirementsChangedJobs()) + .hasSize(counts.requirementsChangedCount()); + assertThat(plan.report().scoreDeltaChangedCount()) + .isLessThanOrEqualTo(counts.scoreChangedCount()); + + Set represented = new java.util.HashSet<>(); + plan.report().changedJobs().forEach(job -> represented.add(job.jobId())); + plan.report().requirementsChangedJobs().forEach(job -> represented.add(job.jobId())); + assertThat(represented).containsAll(plan.changedJobIds()); + plan.report().requirementsChangedJobs() + .forEach(job -> assertThat(job.changedFields()).isNotEmpty()); + assertNoRepositoryWrites(); + } + @Test void missingRequirementsRowFailsClosed() { Job target = juniorProgramme(3L); @@ -299,6 +378,15 @@ private ExtractedRequirements withSeniority(ExtractedRequirements value, String value.applicationDeadline(), value.extractionMethod()); } + /** A stored card that differs from the freshly computed one by score alone. */ + private ScoreCard staleScore(ScoreCard fresh) { + return new ScoreCard(fresh.score() - 5, fresh.band(), fresh.suitable(), + fresh.formalEligibility(), fresh.javaBackend(), fresh.traineeQuality(), + fresh.supportingTechnology(), fresh.locationFormat(), + fresh.experienceCompatibility(), fresh.freshness(), fresh.penalties(), + fresh.strengths(), fresh.risks(), fresh.hardBlockers()); + } + private ExtractedRequirements withSalary(ExtractedRequirements value, String salary) { return new ExtractedRequirements(value.seniority(), value.internshipOrTrainee(), value.requiredExperienceYears(), value.requiredEducation(), diff --git a/src/test/java/com/jobpilot/matching/rescore/ScoreRescorePlanEntryChangeTest.java b/src/test/java/com/jobpilot/matching/rescore/ScoreRescorePlanEntryChangeTest.java new file mode 100644 index 0000000..2d95c33 --- /dev/null +++ b/src/test/java/com/jobpilot/matching/rescore/ScoreRescorePlanEntryChangeTest.java @@ -0,0 +1,70 @@ +package com.jobpilot.matching.rescore; + +import static org.assertj.core.api.Assertions.assertThat; + +import com.jobpilot.jobs.domain.ExtractedRequirements; +import com.jobpilot.jobs.domain.ScreeningDisposition; +import com.jobpilot.matching.ScoreBand; +import com.jobpilot.matching.ScoreCard; +import java.time.Instant; +import java.util.List; +import org.junit.jupiter.api.Test; + +/** Plan membership must follow either persisted projection, not the score alone. */ +class ScoreRescorePlanEntryChangeTest { + + @Test + void scoreOnlyChangeIsPlanned() { + var entry = entry(card(50), card(56), requirements("JUNIOR"), requirements("JUNIOR")); + + assertThat(entry.changed()).isTrue(); + assertThat(entry.scoreChanged()).isTrue(); + assertThat(entry.requirementsChanged()).isFalse(); + } + + @Test + void requirementsOnlyChangeIsPlanned() { + var entry = entry(card(56), card(56), requirements("JUNIOR"), requirements("UNKNOWN")); + + assertThat(entry.changed()).isTrue(); + assertThat(entry.scoreChanged()).isFalse(); + assertThat(entry.requirementsChanged()).isTrue(); + } + + @Test + void bothChangedIsPlanned() { + var entry = entry(card(50), card(56), requirements("JUNIOR"), requirements("UNKNOWN")); + + assertThat(entry.changed()).isTrue(); + assertThat(entry.scoreChanged()).isTrue(); + assertThat(entry.requirementsChanged()).isTrue(); + } + + @Test + void unchangedRowIsNotPlanned() { + var entry = entry(card(56), card(56), requirements("JUNIOR"), requirements("JUNIOR")); + + assertThat(entry.changed()).isFalse(); + assertThat(entry.scoreChanged()).isFalse(); + assertThat(entry.requirementsChanged()).isFalse(); + } + + private ScoreRescorePlanEntry entry(ScoreCard storedScore, ScoreCard computedScore, + ExtractedRequirements storedRequirements, + ExtractedRequirements computedRequirements) { + return new ScoreRescorePlanEntry(5, 10, 20, ScreeningDisposition.REVIEW, + "description", "content", Instant.EPOCH, "json", + storedScore, storedRequirements, computedScore, computedRequirements); + } + + private ScoreCard card(int score) { + return new ScoreCard(score, ScoreBand.POSSIBLE_MATCH, true, 25, 9, 3, 0, 8, 10, 1, 0, + List.of(), List.of(), List.of()); + } + + private ExtractedRequirements requirements(String seniority) { + return new ExtractedRequirements(seniority, false, null, null, false, + List.of("Java"), List.of("Java"), List.of(), "Bucharest", + "Romania eligible", List.of(), null, null, null, "DETERMINISTIC"); + } +} diff --git a/src/test/java/com/jobpilot/matching/rescore/ScoreRescoreWriteCoordinatorTest.java b/src/test/java/com/jobpilot/matching/rescore/ScoreRescoreWriteCoordinatorTest.java index 6049bbf..8fa3068 100644 --- a/src/test/java/com/jobpilot/matching/rescore/ScoreRescoreWriteCoordinatorTest.java +++ b/src/test/java/com/jobpilot/matching/rescore/ScoreRescoreWriteCoordinatorTest.java @@ -83,7 +83,7 @@ void everyMatchingGuardInvokesTransactionExactlyOnce() { @Test void oldFingerprintCannotBeAppliedToThePostWriteEmptyPlan() { - ScoreRescorePlan empty = new ScoreRescorePlan(report(1), List.of()); + ScoreRescorePlan empty = new ScoreRescorePlan(report(1, 0), List.of()); ScoreRescoreWriteResult result = coordinator.execute(empty, properties(true, "1", plan.fingerprint(), "250", ScoreRescoreCommandGuards.CONFIRMATION)); @@ -118,11 +118,19 @@ private ScoreRescorePlan plan() { } private ScoreRescorePreviewReport report(int inspected) { + return report(inspected, 1); + } + + /** The plan now asserts its entry count against the report, so fixtures must agree. */ + private ScoreRescorePreviewReport report(int inspected, int changedPlan) { var queue = new ScoreRescorePreviewReport.QueueProjection(List.of(), List.of()); return new ScoreRescorePreviewReport(inspected, inspected - 1, 1, 1, 1, 0, 1, 0, Map.of(56, 1), 1, 0, new ScoreRescorePreviewReport.BoundaryCrossings(List.of(5L), List.of(5L), - List.of()), List.of(), queue, queue, List.of(), null); + List.of()), List.of(), queue, queue, List.of(), null, + new ScoreRescorePreviewReport.ChangeCounts(changedPlan, changedPlan, 0, + changedPlan, inspected - changedPlan), + List.of()); } private ExtractedRequirements requirements(String seniority) { diff --git a/src/test/java/com/jobpilot/matching/rescore/ScoreRescoreWriteTransactionTest.java b/src/test/java/com/jobpilot/matching/rescore/ScoreRescoreWriteTransactionTest.java index dd2447b..22dffee 100644 --- a/src/test/java/com/jobpilot/matching/rescore/ScoreRescoreWriteTransactionTest.java +++ b/src/test/java/com/jobpilot/matching/rescore/ScoreRescoreWriteTransactionTest.java @@ -173,7 +173,10 @@ private ScoreRescorePlan plan(Target... targets) { var report = new ScoreRescorePreviewReport(targets.length, 0, targets.length, 0, 0, 0, 0, 0, Map.of(), 0, 0, new ScoreRescorePreviewReport.BoundaryCrossings(List.of(), List.of(), List.of()), - List.of(), queue, queue, List.of(), null); + List.of(), queue, queue, List.of(), null, + new ScoreRescorePreviewReport.ChangeCounts(entries.size(), entries.size(), 0, + entries.size(), targets.length - entries.size()), + List.of()); return new ScoreRescorePlan(report, entries); }