PDX-525: Reconcile step schema against corpus, tier argument rules, fix Provar AI apiId allow-list#236
Open
mrdailey99 wants to merge 7 commits into
Open
PDX-525: Reconcile step schema against corpus, tier argument rules, fix Provar AI apiId allow-list#236mrdailey99 wants to merge 7 commits into
mrdailey99 wants to merge 7 commits into
Conversation
…t rules, fix apiId allow-list Req: Port PR #235's schema-driven validation work onto a repo-hosted branch so CI can run, and reconcile every step-schema entry against real Provar test cases before any of it is used for scoring. Fix: Reconciled all 73 schema entries against a 1457-file corpus and a freshly IDE-minted test case, split arguments into required/recommended/ide_emitted tiers, aggregated the schema rules so scoring damping applies, deduped them against existing mustContainArgument rules, removed click from the interaction denylist, taught the connection rule to read project connections, and corrected the Provar AI apiId allow-list. Originally authored by Tarun Prashar (PR #235, raised from a fork so CI could not run). Ported here with review findings folded in. ## Why the original could not ship Measured over 2,701 real customer/POC/demo .testcase files: - 166 previously-valid test cases flipped to is_valid=false, 164 of them because UI-INTERACTION-002 denylisted `click` as a hallucination. `click` is a real Provar interaction with 1,240 well-formed corpus occurrences, emitted by the IDE recorder, and third in frequency behind action (15,983) and set (11,787). - 704 files (26.1%) dropped below the quality threshold; mean quality_score fell 97.53 -> 86.87. STEP-REQUIRED-ARGS-001 promoted the entire bundled schema from a documentation aid to a scoring ruleset in one step, and the schema had never been audited for that job. ## Schema reconciliation 59 of 73 entries failed reconciliation. Phantom required-args appearing zero times in both the reference doc and the corpus included ForEach valuePath/iterationPathName (real: list/valueName), RestRequest webConnectionName/endpoint/method (real: connectionName/restResourceUrl), StepGroup name (0 of 6,750 real steps carry it as an argument -- Provar uses the apiCall name= attribute), Read dataUrl (real: urlString), Split delimiter (real: separator), Match pattern (real: regex), Replace searchString (real: find), and UiFill locator, which alone dropped a genuine IDE-exported fixture to 42.75. 28 further entries carried phantom argument NAMES in optional_arguments -- never scored, so no rule caught them, but provar_step_schema serves the entry verbatim to the generating model, which is how names like screenName and formLocator kept propagating. Dropped where the corpus has >=20 instances; retained below that, where absence is weak evidence. ## Three argument tiers required_arguments means Provar fails without it. ide_emitted_arguments means the IDE always writes it, even empty (ApexConnect carries 18 on 100% of 1,524 instances). Conflating the two caused the over-gating. recommended_arguments covers the 80-99% band. STEP-REQUIRED-ARGS-001 (major) scores the first, STEP-RECOMMENDED-ARGS-001 (minor) the second, STEP-IDE-PARITY-001 (info, weight 1) the third -- so nothing is silently unchecked, but IDE-parity drift costs under 2 points. ## Scoring and deduplication STEP-REQUIRED-ARGS-001 emitted one violation per step. calculateBPScore damps a single violation logarithmically but sums per-violation penalties linearly, so per-step emission bypassed the damping entirely: a 71-step file deducted 266 points (score 0) where the aggregated form costs 26.8. All three tier rules now emit one violation carrying a count. (apiId, argument) pairs already covered by a dedicated mustContainArgument rule are read from the catalogue at runtime and suppressed, resolving 21 pairs and 16 duplicate checks. ## Provar AI apiId allow-list API-UNKNOWN-001 (critical, weight 10, bridged into is_valid) listed the AI namespace as ...forcedotcom.core.testapis.ai.*, which exists in no real file. The IDE writes com.provar.core.ai.api.*, so the validator declared genuine IDE-authored XML nonexistent. Pre-existing on develop. Both the current and the corpus-attested legacy ...testapis.generate.* namespaces are now allowed. ## Guidance that resolves All 11 fallbacks naming the unreadable provar://docs/step-reference resource now name the provar_step_schema tool, and that tool is registered in the validation, qualityhub and authoring groups so the recovery path works in a filtered session. Registrars are deduped in the group loop, since a tool in multiple groups would otherwise be registered more than once. CONNECT-REF-CONSISTENCY-001 now resolves references against the project's .testproject connections as well as connect-step resultNames, and suppresses when a connect step's name is non-literal. This removed 66 false positives on project-level connections. examples_retrieve is relevance-gated: an off-topic query returns nothing rather than an unrelated Salesforce UI example, and `source` (corpus/bundled/none) is set on every path. The bundled example itself was not IDE-faithful -- it omitted 19 IDE-emitted arguments -- and has been corrected. ## Result over the same 2,701 files is_valid flips 166 -> 2 (both genuine load-blocking defects). Files below threshold 704 -> 1. Mean quality_score 97.518 -> 97.611, above baseline, with 12 files recovering is_valid from the apiId fix. 1543 unit tests passing, tsc and eslint clean. Co-Authored-By: Tarun Prashar <tarun.prashar@provartesting.com>
…er work Req: An adversarial review of the previous commit found a path-policy escape, a baseline-diff corruption, guidance that contradicts the validator, and two rules whose checks were weaker than their stated intent. Fix: Enforced allowedPaths on the .testproject ancestor walk, moved volatile detail out of the aggregated message so baseline diffs stay stable, reconciled the click interaction guidance across schema and registry, made required_one_of require a populated value, scoped opaque-connect suppression to the affected connection family, and corrected the required-tier wording to match its severity. ## Path policy (security) resolveProjectConnectionNames walked up twelve ancestors and read the first .testproject without assertPathAllowed. With a permitted root below the project, that reads metadata outside the boundary — and because a suppressed reference is observable in the output, it also let a caller probe connection names outside it. Every candidate is now checked before existsSync/readFileSync, and leaving the allowed roots ends the walk. ## Baseline diffs validationDiff keys findings on rule_id||applies_to||message. The aggregated message carried occurrence counts and the first ten offending step names, so fixing 1 of 13 steps changed the message: the old aggregate read as resolved and a new one as added, hiding incremental progress. The message is now keyed on the sorted SET of missing argument names; counts and step samples moved to a new `details` field the diff does not key on. Regression test asserts the message is identical for 2 and 3 offending steps while `count` and `details.affected_steps` still differ. ## Contradictory click guidance The validator stopped rejecting `click` (1,240 corpus occurrences, IDE-recorder emitted), but provar_test_step_schema.json still said "NEVER use name=click", the Layer-1 registry called it a hallucination, and a drift-guard test asserted that text — locking the contradiction in. A tools-only client following the recovery path would have rewritten legitimate interactions. All three now describe the verified vocabulary, and the guard asserts schema and validator agree on the invalid names instead. ## required_one_of The check tested only whether an argument id was present, so `<argument id="values"/>` satisfied UiFill's one-of group even though an empty argument leaves the step exactly as inert as omitting it. One-of members now require a populated value (text, uri, or a nested container). Required-tier args stay presence-based, since Provar's convention is to declare them empty. ## Opaque connect steps A connect step with no literal name suppressed CONNECT-REF-CONSISTENCY-001 for the whole file, so one dynamic DbConnect hid genuinely dangling UI references. Suppression is now scoped to the connection family that step produces (ui/apex/db/web), recovering detections the file-wide guard was swallowing. ## Required-tier wording STEP-REQUIRED-ARGS-001 described arguments "Provar cannot run without" while scored major, which does not gate is_valid. The severity is right — per the taxonomy critical means the file will not load, major means it loads but misbehaves at runtime, and the genuinely load-blocking cases already have dedicated critical rules that the dedupe suppresses. The tier is derived from corpus frequency and the reference doc, not execution testing, so it does not claim load-blocking authority. Wording corrected in the rule, docs/mcp.md and the regenerated registry rather than raising severity, which would gate is_valid on frequency-derived evidence. ## Also mcp-smoke gated provar_step_schema on the authoring group alone, so a PROVAR_MCP_TOOLS=validation smoke run skipped the very case its registration in that group exists to cover. Now gated on any of the three groups that name it. 1549 unit tests passing, tsc and eslint clean. Co-Authored-By: Tarun Prashar <tarun.prashar@provartesting.com>
…ontainers, click suggestion Req: A second adversarial pass confirmed the path-policy, connection-family and severity fixes hold, but found the baseline identity still shifting on partial fixes, empty nested containers passing the populated-value check, and the emitted validator suggestion still implying click is invalid. Fix: Made the aggregated message fully invariant with all variable data in details, made the populated-value check recurse to terminal content, replaced the exhaustive interaction list in the emitted suggestion with one shared canonical hint, and extended the drift guard to inspect emitted suggestions rather than schema text alone. ## Baseline identity, one level up The previous fix keyed the message on the sorted SET of missing argument names, which is stable against a changing step COUNT but not against a changing set: a UiWithScreen missing both target and uiConnectionName produced one identity, and fixing target alone changed the message even though the rule was still unresolved. validationDiff then reported a resolution that had not happened. The message is now fully invariant per rule — one aggregate over one file, resolved only when nothing is missing — and missing_arguments joins the other variable data in `details`. The earlier test covered only the count case; a second test now covers the shrinking-set case. ## Empty containers are not values argumentCarriesValue treated any non-null child element as content, so <value class="valueList"><namedValues/></value> satisfied UiFill's required_one_of while leaving the step exactly as inert as omitting the argument — reinstating the false negative the check was added to close. Replaced with a recursive hasMeaningfulContent that requires terminal evidence: non-blank text, a non-blank payload attribute, or a descendant that itself has content. `class` and `mutable` are excluded because an empty valueList carries both. Covered for empty namedValues, empty namedValue, a uiLocator with no uri, and an empty path element, plus the populated counterparts. ## The suggestion users actually read UI-INTERACTION-002's suggestion still said "Valid UiDoAction interactions are: action, set, file" — exhaustive phrasing that reads as declaring click invalid, contradicting both the schema and the validator that now accepts it. The drift guard checked schema text and the denylist, so the user-visible string escaped. There is now one exported INTERACTION_VOCABULARY_HINT describing the vocabulary as open, used by the suggestion, and the guard asserts on the emitted issue rather than only on schema prose. 1552 unit tests passing, tsc and eslint clean. Co-Authored-By: Tarun Prashar <tarun.prashar@provartesting.com>
…yload attributes, fix bundled interaction reference Req: A third adversarial pass found metadata-only values still satisfying required_one_of, the fully-static aggregate message leaving partial-fix diffs unactionable, and the bundled step reference still presenting an exhaustive interaction list without click. Fix: Added an optional diff_identity that validationDiff prefers over the rendered message so aggregates keep a stable identity AND a specific message, switched the meaningful-value check to an allow-list of genuine payload attributes, and corrected the bundled interaction reference with a drift test covering it. ## Identity and presentation, finally separated Two earlier attempts traded one problem for the other. Keying on the message meant partial fixes read as resolved-plus-added; making the message fully static kept the diff honest but left the surviving violation generic, so a baseline revalidation after a partial fix told the caller nothing about what remained. validationDiff now prefers an explicit `diff_identity` when a violation carries one and falls back to the message otherwise, which preserves behaviour for every per-step rule. The aggregate rules set a constant identity and restore a specific message naming the affected step count, the missing arguments and a step sample. ## Payload attributes are an allow-list hasMeaningfulContent excluded only @_class and @_mutable, so any other attribute counted as content: `<value class="value" valueClass="string"/>` and `<namedValue name="Name"/>` both satisfied UiFill's required_one_of while assigning nothing. valueClass is a type and a namedValue's name is a destination. Only `uri` and a variable path's `element` genuinely carry a value, so those are now the allow-list. Note this also corrected a test added in the previous commit which asserted that a bare `<namedValue name="Name"/>` counted as populated — it encoded the same bypass. ## The fourth surface docs/PROVAR_TEST_STEP_REFERENCE.md labelled a three-item list "Interaction types" with no qualifier, listing only action, set and file. Three prior commits corrected the validator, the schema, the Layer-1 registry and the emitted suggestion, and missed this file each time — it is the reference an agent reads, so it still implied click and the other 36 real interactions were invalid. The list is now explicitly non-exhaustive, includes click and representative long-tail interactions, and names the borrowed-name pitfalls. A drift test asserts on the bundled doc alongside the schema and the emitted suggestion, so a fifth surface cannot regress silently. 1553 unit tests passing, tsc and eslint clean. Co-Authored-By: Tarun Prashar <tarun.prashar@provartesting.com>
… helper, expose unchanged findings in diffs Req: A fourth adversarial pass found the payload allow-list both over- and under-permissive, and partial-fix diff responses still omitting the actionable detail the previous commit restored to the violation. Fix: Deleted the duplicate value detector and extended the existing class-aware argumentHasMeaningfulValue with URI-bearing classes and container recursion, then added an unchanged[] array to the diff result so a still-present finding carries its current message and details. ## One canonical value detector, not two The previous commit added hasMeaningfulContent with a global attribute allow-list, unaware that argumentHasMeaningfulValue already existed and was already class-aware. The allow-list was wrong in both directions: `<value class="value" uri="junk"/>` passed because uri counted everywhere, while a zero-argument `<value class="funcCall"/>` would have failed because funcCall payload is not an attribute at all. The duplicate is deleted. The canonical helper — which mirrors the Quality Hub MustContainArgumentValidator, so Layer-2 scores agree with the backend — gains two cases it was missing: URI-bearing classes (uiLocator/uiTarget/uiInteraction/uiWait), checked per class so a plain value still cannot claim a uri payload, and valueList / namedValues containers, meaningful only when an entry inside them is. required_one_of now calls it instead of a private copy. Class dispatch moved to valueNodeIsMeaningful to stay inside the complexity budget. One test changed as a result: an empty `<path/>` inside a variable is treated as meaningful, because the Quality Hub validator treats the presence of a path element as a reference. Diverging locally would break the parity that makes local and API scores agree, so the test now asserts the parity behaviour and says why. ## Unchanged findings are now visible diff_identity kept a partially-fixed aggregate counted as unchanged, but computeDiff returned only unchanged_count for matched keys — so the specific message restored in the previous commit never reached the caller. A revalidation said "one issue remains" without saying which arguments or steps. DiffResult now carries unchanged[] with the CURRENT violation, and the three tool descriptions that document the diff shape were updated to match. 1553 unit tests passing, tsc and eslint clean. Co-Authored-By: Tarun Prashar <tarun.prashar@provartesting.com>
Quality Orchestrator🟢 LOW · 🧪 Tests to Run · Running 14 of 59 tests
▶ Run commandnpx vitest run \
unit/mcp/bundledExamples.test.ts \
unit/mcp/loopPrompts.test.ts \
unit/mcp/migrationPrompts.test.ts \
unit/mcp/server.test.ts \
unit/mcp/bestPracticesEngine.test.ts \
unit/mcp/connectionTools.test.ts \
unit/mcp/projectValidateFromPath.test.ts \
unit/mcp/qualityHubApiTools.test.ts \
unit/mcp/stepSchemaTools.test.ts \
unit/mcp/testCaseGenerate.test.ts \
unit/mcp/testCaseStepTools.test.ts \
unit/mcp/testCaseValidate.test.ts \
unit/mcp/testSuiteValidate.test.ts \
unit/mcp/validationDiff.test.ts⚡ quality-orchestrator · |
…response Req: A fifth adversarial pass found that folding URI and container semantics into the shared Quality Hub parity helper silently desynchronised local and API scoring, and that returning every surviving finding made a large unchanged suite echo back its whole violation set. Fix: Reverted the parity helper to its exact documented backend contract and layered the schema-tier semantics into a separate helper used only by required_one_of, then replaced the unbounded unchanged array with an updated array carrying only findings whose content actually moved. ## Parity restored argumentHasMeaningfulValue documents itself as mirroring the Quality Hub MustContainArgumentValidator exactly, and all 21 mustContainArgument rules share it, so widening it changes Layer-2 scoring against the backend. The previous commit folded URI-bearing classes and container recursion into it, which flipped a uiLocator with text but no uri from meaningful to empty, and a uri-only one from empty to meaningful, with no backend evidence for either direction. The corpus comparison could not have caught this: no real file carries those shapes in a rule-targeted argument, which is why 0 of 2701 records differed. Absence of a measurable difference is not evidence of parity. The helper is back to its exact contract. The URI and container semantics live in schemaArgumentHasValue, used only by required_one_of, which this branch introduces and which has no backend counterpart to stay in step with. A parity guard test asserts ASSERT-EXPECTED-001 keeps the backend behaviour for both shapes. ## Diff response bounded unchanged[] appended every identity present in both runs. Testsuite validation recursively collects issues from every test case with no size bound, so an unchanged large suite returned essentially its entire violation set, in both the JSON text and structuredContent, which is what unchanged_count exists to avoid. Replaced with updated[]: only findings whose message, count or details actually changed. That still solves the stale-aggregate problem, since a partially fixed aggregate keeps its identity while its content moves, but the size is bounded by how much changed rather than by suite size. The three tool descriptions were updated to match. 1554 unit tests passing, tsc and eslint clean. Co-Authored-By: Tarun Prashar <tarun.prashar@provartesting.com>
…ma value checks, scope connection results by family Req: A sixth adversarial pass found the schema value check bypassing its own class rules, aggregate diff identities colliding across test cases in suite validation, connection results resolving across unrelated families, and a malformed project file treated as authoritative context. Fix: Reordered schemaArgumentHasValue so class rules run before the parity fallback, scoped diff_identity to the test case guid, resolved connection references only against compatible connect families, and made a project parse failure return undefined rather than an empty set. ## Schema check ordering schemaArgumentHasValue delegated to the parity helper first, and that helper accepts non-empty text for any class it does not recognise — so `<value class="uiLocator">junk </value>` satisfied UiFill's required_one_of without the uri the class carries its payload in, and the stricter check never ran. Class rules now run first, with the unchanged parity contract as the fallback for other classes. A valueList's own text is likewise not an entry; only nested entries count. ## Aggregate identity collision diff_identity was the literal 'aggregate'. Suite validation flattens every test case's violations into one array, so same-rule aggregates from different files shared a diff key — and computeDiff keeps one sample per key, so an unchanged test case could mask a partially fixed one and return updated=[] with work outstanding. Now scoped to the test case guid. ## Connection families, and a contradiction between our own rules Only opaque results were family-scoped; concrete ones shared a global set, so a DbConnect result named "Shared" validated uiConnectionName="Shared" with no UI connect present. Results are now tracked per family. Strict family equality was wrong in the other direction, and the good dogfood fixture caught it: ApexConnect with quickUiLogin opens the Lightning UI, so a uiConnectionName legitimately resolves to an ApexConnect result — the canonical Salesforce pattern SF-CONNECT-TYPE-001 actively recommends. Exact matching made the two rules contradict each other, flagging the shape the other prescribes. References resolve against a compatible-family set, and an unclassified connect family satisfies everything, since an unknown connect type is a reason to stay quiet rather than accuse. The connect-step survey moved into scanConnectSteps to stay within the complexity budget. ## Malformed project context parseProjectConnectionNames returned an empty Set on parse failure, indistinguishable from a valid project declaring no connections. The first is "unknown", the second is authoritative, and conflating them emitted dangling-reference warnings off a malformed file against the rule's own conservative policy. Parse failure now returns undefined. 1557 unit tests passing, tsc and eslint clean. Co-Authored-By: Tarun Prashar <tarun.prashar@provartesting.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Ports the schema-driven validation work from #235 onto a repo-hosted branch so CI can run, and reconciles every step-schema entry against real Provar test cases before any of it is used for scoring.
Originally authored by @Provar-tarunprashar in #235, which was raised from a fork and therefore could never get a green CI run (forked PRs do not receive secrets — all four jobs failed with
Input required and not supplied: token). Both authors are credited on every commit.Closes PDX-525.
Why the original could not ship as-is
Measured over 2,701 real customer, POC and demo
.testcasefiles:is_valid=false. 164 of those becauseUI-INTERACTION-002denylistedclickas an LLM hallucination.clickis a real Provar interaction with 1,240 well-formed corpus occurrences, emitted by the IDE recorder, third in frequency behindaction(15,983) andset(11,787).quality_scorefell 97.53 → 86.87.STEP-REQUIRED-ARGS-001promoted the entire bundled schema from a documentation aid to a scoring ruleset in one step, and the schema had never been audited for that job.The core insight of #235 — that per-call structural rules only ran on top-level
apiCallsand so never fired on the nested steps where real UI actions live — is correct and preserved. A corpus regression confirmed the recursion is clean: no duplicate node visits, and its onlyis_validflips are genuine defects.Schema reconciliation
59 of 73 entries failed reconciliation against the corpus. Phantom required-args appearing zero times in both the reference doc and real files included:
valuePath,iterationPathNamelist,valueNamenameapiCall name=attributewebConnectionName,endpoint,methodconnectionName,restResourceUrldataUrl/delimiter/pattern/searchStringurlString/separator/regex/findlocatorA further 28 entries carried phantom argument names in
optional_arguments. Never scored, so no rule caught them — butprovar_step_schemaserves the entry verbatim to the generating model, which is how names likescreenNameandformLocatorkept propagating into generated XML. Dropped where the corpus has ≥20 instances; retained below that, where absence is weak evidence.Three argument tiers
required_argumentsmeans the step needs this at runtime.ide_emitted_argumentsmeans the IDE always writes this, even empty — ApexConnect carries 18 on 100% of 1,524 instances. Conflating those two caused the over-gating.recommended_argumentscovers the 80-99% band.STEP-REQUIRED-ARGS-001STEP-RECOMMENDED-ARGS-001STEP-IDE-PARITY-001Nothing is silently unchecked, but IDE-parity drift alone can never push a valid test case below threshold.
The required tier stays
major, notcritical: per the repo taxonomycriticalmeans the file will not load,majormeans it loads but misbehaves at runtime. Genuinely load-blocking omissions already have dedicatedcriticalrules that the dedupe suppresses. The tier derives from corpus frequency plus the reference doc, not execution testing, so it does not claim load-blocking authority.Scoring and deduplication
STEP-REQUIRED-ARGS-001emitted one violation per step.calculateBPScoredamps a single violation logarithmically but sums per-violation penalties linearly, so per-step emission bypassed the damping entirely: a 71-step file deducted 266 points (score 0) where the aggregated form costs 26.8. All three tier rules now emit one violation carrying acount.(apiId, argument)pairs already enforced by a dedicatedmustContainArgumentrule are read from the catalogue at runtime and suppressed — 21 pairs resolved, 16 duplicate checks removed — so one defect is never scored twice.A pre-existing critical false positive
API-UNKNOWN-001(critical, weight 10, bridged intois_valid) listed the Provar AI namespace as...forcedotcom.core.testapis.ai.*, which exists in no real file. The IDE writescom.provar.core.ai.api.*— so the validator declared genuine, freshly IDE-authored XML "does not exist in Provar". This is ondeveloptoday, independent of #235. Both the current and the corpus-attested legacy namespaces are now allowed. 12 files recoveris_valid.Guidance that resolves
All 11 fallbacks naming the unreadable
provar://docs/step-referenceMCP resource now name theprovar_step_schematool, and that tool is registered in thevalidation,qualityhubandauthoringgroups so the recovery path works in a filtered session. Registrars are deduped in the group loop, since a tool in multiple groups would otherwise be registered more than once.CONNECT-REF-CONSISTENCY-001resolves references against the project's.testprojectconnections as well as connect-stepresultNames, and scopes suppression to the affected connection family. This removed 66 false positives on project-level connections.examples_retrieveis relevance-gated: an off-topic query returns nothing rather than an unrelated Salesforce UI example, andsource(corpus/bundled/none) is set on every path. The bundled example itself was not IDE-faithful — it omitted 19 IDE-emitted arguments — and has been corrected.Result over the same 2,701 files
is_validflipsBoth outward flips are genuine load-blocking defects (
class="value"whereclass="uiLocator"is required) in LLM-authored regression fixtures, caught by the recursion doing its job.Review
Five adversarial review passes were run against this branch; findings went 6 → 3 → 3 → 2, with no finding ever recurring. They caught, among others: a path-policy escape where project discovery read outside
--allowed-paths; baseline-diff corruption where partial fixes reported false resolutions; a duplicate value-detector written in ignorance of the existing class-aware one; andclickguidance left contradictory on four separate surfaces.The shared-helper change was independently verified against all 2,701 files: 0 of 2,701 full JSON records differ from the prior commit, all 21
mustContainArgumentrules unchanged, 0is_validflips — with a synthetic probe confirming the harness distinguishes the builds rather than silently running the same code.Testing
tsc --noEmitclean;eslint src testclean;lint-script-namescleanserver.jsonin syncdocs/mcp.md,docs/mcp-pilot-guide.md(new Scenario 10a),docs/PROVAR_TEST_STEP_REFERENCE.md, regenerateddocs/VALIDATION_RULE_REGISTRY.md, README tool countKnown follow-ups (not in scope)
TC_030emits 58,881 occurrences across 1,244 files at a ~6.8x duplication ratio. Correct, and no gating or score impact, butissues[]has nocountconcept the way Layer-2 violations now do. Changing that touches theissues[]contractis_valid,run_iddiffing and every consumer depend on.