Skip to content

Add PAT (Pay A Tax) technique for LLM evaluation with gate-check structured output#65

Merged
oshorefueled merged 5 commits intomainfrom
feat/eval-gate-checks
Mar 2, 2026
Merged

Add PAT (Pay A Tax) technique for LLM evaluation with gate-check structured output#65
oshorefueled merged 5 commits intomainfrom
feat/eval-gate-checks

Conversation

@oshorefueled
Copy link
Contributor

@oshorefueled oshorefueled commented Mar 2, 2026

Why

Previous evaluations used a reasoning field to improve accuracy, but reasoning tokens are generated after the model has already pattern-matched to a conclusion — making the reasoning cosmetic and post-hoc. This PR replaces that with a "tax" approach: the model is forced to emit specific boolean gate checks per violation candidate before it can proceed. Serializing through these gates constrains what the model can plausibly output next, producing more accurate evaluations without a second model pass.

What

  • Requires line, rule_quote, confidence, reasoning, and gate check fields (rule_supports_claim, evidence_exact, context_supports_violation, plausible_non_violation, fix_is_drop_in, fix_preserves_meaning) in LLM structured output for both check and judge modes
  • Adds deterministic violation-filter.ts — computes surface/hide decisions and reason strings from gate check booleans; no extra model call
  • Adds --debug-json CLI flag that writes per-run artifacts to .vectorlint/runs/ containing raw model output, filter decisions, and surfaced violations
  • Updates the built-in directive to reflect the raw-candidates + gate-checks approach
  • Adds tests/evaluations/ with test rules and config for manual evaluation runs

Scope

In scope

  • Structured output schema contract (both check and judge)
  • Deterministic gate filtering logic
  • --debug-json instrumentation
  • Directive wording update

Behavior impact

  • Violation output will change. The gate check fields are part of the structured output the model generates on every run — not just when --debug-json is passed. Because the model has to commit to boolean gate checks per violation candidate during generation, the set of violations it produces will differ from the old approach. Expect changes in what gets surfaced across all output formats (line, json, vale-json, rdjson), even though the output structure is unchanged.
  • The standard output formats (--output json etc.) do not include the new gate fields — they are consumed internally and written to debug artifacts only
  • Models that fail to populate the required gate fields will throw a structured output error
  • --debug-json writes to .vectorlint/runs/ (gitignored); no impact on default runs
  • Directive rewrite changes the system prompt on every evaluation, compounding the change in violation output

Risk

  • Medium — the new required fields change the LLM output contract; any provider or model that doesn't support structured output reliably may fail where it previously succeeded
  • Gate checks are computed on every run but only used when --debug-json is passed — adds token cost even when not debugging

How to test / verify

Checks run

  • npm run test:run — passed
  • npm run lint — passed

Manual verification

# Smoke test with debug output
npm run dev -- tests/evaluations/TEST_FILE.md \
  --config tests/evaluations/.vectorlint.ini \
  --debug-json --verbose

# Expected: normal findings + a line like:
# [vectorlint] Debug JSON written: .vectorlint/runs/<uuid>.json
# Verify the artifact contains raw_model_output.violations[*].line and gate fields
ls .vectorlint/runs/

Expected artifact output

When --debug-json is passed, a file is written to .vectorlint/runs/<model-tag>/<run_id>.json. Example (one violation, truncated):

{
  "run_id": "908b766e-b80a-4caf-a3f9-76a1a80aab8d",
  "timestamp": "2026-02-28T16:24:15.188Z",
  "file": "TEST_FILE.md",
  "model": {
    "provider": "openai",
    "name": "gpt-5.2-2025-12-11",
    "tag": "openai-gpt-5.2-2025-12-11"
  },
  "prompt": {
    "pack": "Test",
    "id": "Consistency",
    "filename": "consistency.md",
    "evaluation_type": "check"
  },
  "raw_model_output": {
    "reasoning": "Reviewed the input for terminology consistency...",
    "violations": [
      {
        "line": 43,
        "quoted_text": "AI-Powered Search",
        "context_before": "* **",
        "context_after": "**\n  Enable AI Search to help developers ask questions...",
        "description": "Potential terminology inconsistency: \"AI-Powered Search\" vs \"AI Search\" for the same feature.",
        "analysis": "The bullet heading names the feature \"AI-Powered Search\", but the description calls it \"AI Search\".",
        "suggestion": "Use one term consistently.",
        "fix": "  Enable AI-Powered Search to help developers ask questions about your product and instantly receive an answer.",
        "rule_quote": "Flag when the same concept is referred to by different terms",
        "checks": {
          "rule_supports_claim": true,
          "evidence_exact": true,
          "context_supports_violation": true,
          "plausible_non_violation": true,
          "fix_is_drop_in": true,
          "fix_preserves_meaning": true
        },
        "check_notes": {
          "rule_supports_claim": "The rule explicitly requires flagging the same concept referred to by different terms.",
          "evidence_exact": "Quoted text exactly matches line 43.",
          "context_supports_violation": "The very next line uses a different name (\"AI Search\") for the likely same feature.",
          "plausible_non_violation": "\"AI-Powered Search\" might be a category label while \"AI Search\" is the product name.",
          "fix_is_drop_in": "Direct substitution without structural changes.",
          "fix_preserves_meaning": "Keeps the intended meaning while aligning terminology."
        },
        "confidence": 0.8
      }
    ]
  },
  "filter_decisions": [
    {
      "index": 0,
      "surface": false,
      "reasons": ["plausible_non_violation=true"]
    }
  ],
  "surfaced_violations": []
}

Key things to verify:

  • raw_model_output.violations[*] contains line, rule_quote, checks, check_notes, and confidence
  • filter_decisions[*].reasons lists why each candidate was hidden or surfaced
  • surfaced_violations contains only candidates that passed all gates

Rollback

  • Revert to main — no DB/config/env changes
  • Delete .vectorlint/runs/ locally (optional, gitignored)

Summary by CodeRabbit

  • New Features

    • Added a CLI flag to emit structured debug artifacts and include raw model outputs and per-violation filter decisions in runs.
    • Evaluation results now expose raw_model_output, per-violation metadata (lines, quotes, confidence) and aggregated reasoning.
  • Documentation

    • Added an evaluation workflow guide and several test rule examples (clarity, consistency, passive voice, readability, wordiness).
  • Chores

    • Updated ignore list to exclude debug artifact directories and refined evaluator prompt directive.

…tation

- Add gate checks (rule_supports_claim, evidence_exact,
  context_supports_violation, plausible_non_violation, fix_is_drop_in,
  fix_preserves_meaning) to both check and judge LLM output schemas
- Require reasoning, confidence, rule_quote, and line fields in
  structured output for improved inspectability
- Add --debug-json CLI flag that writes per-run artifacts to
  .vectorlint/runs/ with raw model output and filter decisions
- Add src/debug/run-artifact.ts and violation-filter.ts for
  deterministic gating and artifact serialization
- Attach raw_model_output to evaluator results for debug access
- Update directive to reflect raw-candidates + gate-checks approach
- Add tests/evaluations/ with test rules and config for manual evals
- Ignore .vectorlint/runs/ in git
- Add tests/evaluations/README.md with instructions for running
  manual evaluations, switching models, and inspecting debug artifacts
- Add docs/pr-description.md for the gate-check PR
- Fix RulesPath in tests/evaluations/.vectorlint.ini (./builtin →
  ./test-rules) so manual evaluation runs don't abort on missing path
- Guard writeDebugRunArtifact calls in orchestrator with try/catch;
  filesystem errors now warn instead of interrupting result handling
- Fix artifact path examples in pr-description.md to consistently
  use .vectorlint/runs/<model-tag>/<run_id>.json
@oshorefueled oshorefueled changed the title Implement PAT (Pay A Tax) technique for LLM evaluation with gate-check structured output Add PAT (Pay A Tax) technique for LLM evaluation with gate-check structured output Mar 2, 2026
@coderabbitai
Copy link

coderabbitai bot commented Mar 2, 2026

📝 Walkthrough

Walkthrough

Adds opt-in debug-artifact generation and plumbing (CLI flag, types, orchestrator) plus debug utilities, expands prompt/result schemas with raw_model_output and reasoning, updates evaluator outputs, and adds test evaluation rules and docs. Files written under .vectorlint/runs/ are gitignored.

Changes

Cohort / File(s) Summary
CLI & schemas
./.gitignore, src/cli/commands.ts, src/cli/types.ts, src/schemas/cli-schemas.ts
Introduced --debug-json CLI option, propagated debugJson into evaluation types/context, and added .vectorlint/runs/ and docs to .gitignore.
Orchestrator & flow
src/cli/orchestrator.ts
Threaded debugJson through evaluation, added per-run UUID/timestamps, model metadata extraction, per-violation decision computation, and writes debug artifacts during check and judge paths.
Debug utilities
src/debug/run-artifact.ts, src/debug/violation-filter.ts
New debug artifact writer (path sanitization, dir creation, JSON write) and filter decision computation (surface flag + reasons) for gate violations.
Evaluator outputs
src/evaluators/base-evaluator.ts
Collects per-chunk LLM outputs into raw_model_output (single value or array) and aggregates per-chunk reasoning into result objects for Check and Judge flows.
Prompt schemas & directive
src/prompts/schema.ts, src/prompts/directive-loader.ts
Expanded public types (violations include line, description, rule_quote, checks, check_notes, confidence), added GateChecks/GateCheckNotes, added raw_model_output and reasoning fields, and rewrote DEFAULT_DIRECTIVE to enforce structured JSON outputs.
Scoring
src/scoring/scorer.ts
Preserved and propagated raw violation objects (with new metadata) when aggregating scores instead of sanitizing fields.
Tests / eval rules / docs
tests/evaluations/.vectorlint.ini, tests/evaluations/README.md, tests/evaluations/TEST_FILE.md, tests/evaluations/test-rules/Test/*
Added test configuration, README explaining debug artifacts and workflows, a sample TEST_FILE, and five rule definition files (clarity, consistency, passive-voice, readability, wordiness).

Sequence Diagram(s)

sequenceDiagram
    actor User
    participant CLI as CLI
    participant Orch as Orchestrator
    participant Eval as BaseEvaluator
    participant LLM as LLM/Prompt Schema
    participant Debug as DebugWriter

    User->>CLI: run with --debug-json
    CLI->>Orch: evaluateFiles(options{debugJson:true})
    Orch->>Eval: runCheckEvaluation / runJudgeEvaluation
    Eval->>LLM: send prompts
    LLM-->>Eval: raw_model_output (+ per-chunk reasoning)
    Eval-->>Orch: result (violations, raw_model_output, reasoning)
    Orch->>Orch: compute filter_decisions & surfaced_violations
    Orch->>Orch: getModelInfoFromEnv()
    Orch->>Debug: writeDebugRunArtifact(artifact)
    Debug-->>Orch: artifact path
    Orch-->>User: log artifact location
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

Suggested reviewers

  • ayo6706

Poem

🐇 I hopped through code to catch the trace,
Raw outputs tucked in a quiet place,
Decisions and reasons all neatly spun,
Runs saved in folders for races run—
A little rabbit, debugging done.

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 41.67% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely summarizes the main change: adding the PAT (Pay A Tax) technique for LLM evaluation with gate-check structured output, which is the primary feature across all modified and new files.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feat/eval-gate-checks

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
src/prompts/directive-loader.ts (1)

49-51: ⚠️ Potential issue | 🟡 Minor

Use catch (e: unknown) to match repo TypeScript error-handling standards.

The bare catch {} block violates the project's consistent pattern. All other catch blocks across the codebase bind the error as unknown.

🛠️ Suggested change
-  } catch {
+  } catch (e: unknown) {
     // Ignore errors when reading override file
   }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/prompts/directive-loader.ts` around lines 49 - 51, Replace the bare catch
block in directive-loader.ts (the catch { ... } that currently ignores errors
when reading the override file) with a typed catch binding using the repository
convention (bind the thrown value as e: unknown), keep the existing comment
about ignoring errors, and ensure no runtime behavior changes beyond using the
bound variable name (so the block becomes a catch that accepts e: unknown and
still ignores/handles the error as before).
src/prompts/schema.ts (1)

50-50: ⚠️ Potential issue | 🟡 Minor

Constrain line to positive integers in structured output schema.

Line 50 and Line 142 currently accept any number (including decimals/negatives). For deterministic line mapping, enforce integer + minimum 1.

✅ Proposed fix
-                    line: { type: "number" },
+                    line: { type: "integer", minimum: 1 },
@@
-              line: { type: "number" },
+              line: { type: "integer", minimum: 1 },

Based on learnings: Validate all external data (files, CLI, env, APIs) at system boundaries using Zod schemas.

Also applies to: 142-142

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/prompts/schema.ts` at line 50, The "line" field in the structured output
schema currently allows any number; update both occurrences of the "line"
property to enforce positive integers by replacing the type with
z.number().int().min(1) (e.g., change line: { type: "number" } to use Zod: line:
z.number().int().min(1)). Locate the "line" properties in the schema definitions
(both places currently using a numeric type) and apply this Zod constraint so
external data is validated as integers >= 1.
🧹 Nitpick comments (4)
.gitignore (1)

17-17: Consider anchoring docs ignore rule to repo root.

Using docs can unintentionally ignore nested paths. If intent is only root local docs, prefer /docs/.

🔧 Suggested tweak
-docs
+/docs/
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.gitignore at line 17, The .gitignore contains a broad "docs" rule that may
unintentionally ignore nested directories; change that entry to "/docs" so it is
anchored to the repository root. Locate the "docs" line in the .gitignore file
and replace it with "/docs" to ensure only the top-level docs directory is
ignored.
src/prompts/directive-loader.ts (1)

35-38: Remove duplicated hard-constraint wording.

Lines 36-37 repeat the same evidence-copy requirement. Keeping one canonical statement reduces prompt bloat and ambiguity.

♻️ Suggested cleanup
 ## Hard constraints
 - Do NOT invent evidence. Every quoted span must be copied exactly from the Input.
-- Every quoted span must be copied exactly from the Input.
 - Use the provided line numbers.
 - Exclude the line number prefix from quoted spans.
 - If you cannot provide a valid drop-in fix, set fix="" and mark fix_is_drop_in=false.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/prompts/directive-loader.ts` around lines 35 - 38, Remove the duplicated
hard-constraint wording in the directive text: keep a single canonical statement
"Every quoted span must be copied exactly from the Input." and delete the
redundant duplicate that follows it (the repeated line shown in the prompt
block). Update the prompt constant or template in
src/prompts/directive-loader.ts (the hard-constraints block) so only one
occurrence of that exact sentence remains, preserving surrounding formatting and
line-numbering guidance.
src/cli/orchestrator.ts (1)

32-53: Prefer runtime/provider model metadata over env-only lookup for artifact subdirs.

getModelInfoFromEnv() works when env vars are set, but runs configured via ~/.vectorlint/config.toml can end up without a model.tag, so artifacts won’t consistently land under .vectorlint/runs/<model-tag>/... as documented.

Also applies to: 680-681, 778-779

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/cli/orchestrator.ts` around lines 32 - 53, getModelInfoFromEnv currently
only reads env vars, so model.tag can be missing for runs configured via
~/.vectorlint/config.toml; change getModelInfoFromEnv to prefer runtime/provider
metadata and fall back to env: accept optional runtimeModel or providerClient
input (or call a new helper like getRuntimeModelInfo) and merge that metadata
with env values so tag is computed from runtime.provider/name first, then env,
and finally omitted only if neither exists; update callers that rely on
model.tag (the places referenced as 680-681 and 778-779) to pass the runtime
model-info or call the updated function so artifacts consistently land under
.vectorlint/runs/<model-tag>/...
src/prompts/schema.ts (1)

59-99: Deduplicate gate schema fragments to prevent contract drift.

The checks and check_notes JSON-schema objects are duplicated; extracting shared constants will keep judge/check contracts in sync.

♻️ Refactor sketch
+const gateChecksSchema = {
+  type: "object",
+  additionalProperties: false,
+  properties: {
+    rule_supports_claim: { type: "boolean" },
+    evidence_exact: { type: "boolean" },
+    context_supports_violation: { type: "boolean" },
+    plausible_non_violation: { type: "boolean" },
+    fix_is_drop_in: { type: "boolean" },
+    fix_preserves_meaning: { type: "boolean" },
+  },
+  required: [
+    "rule_supports_claim",
+    "evidence_exact",
+    "context_supports_violation",
+    "plausible_non_violation",
+    "fix_is_drop_in",
+    "fix_preserves_meaning",
+  ],
+} as const;
+
+const gateCheckNotesSchema = {
+  type: "object",
+  additionalProperties: false,
+  properties: {
+    rule_supports_claim: { type: "string" },
+    evidence_exact: { type: "string" },
+    context_supports_violation: { type: "string" },
+    plausible_non_violation: { type: "string" },
+    fix_is_drop_in: { type: "string" },
+    fix_preserves_meaning: { type: "string" },
+  },
+  required: [
+    "rule_supports_claim",
+    "evidence_exact",
+    "context_supports_violation",
+    "plausible_non_violation",
+    "fix_is_drop_in",
+    "fix_preserves_meaning",
+  ],
+} as const;
@@
-                    checks: { ... },
-                    check_notes: { ... },
+                    checks: gateChecksSchema,
+                    check_notes: gateCheckNotesSchema,
@@
-              checks: { ... },
-              check_notes: { ... },
+              checks: gateChecksSchema,
+              check_notes: gateCheckNotesSchema,

Also applies to: 151-191

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/prompts/schema.ts` around lines 59 - 99, The checks and check_notes
JSON-schema fragments are duplicated which risks contract drift; extract a
single shared definition of the keys (e.g., a constant like CHECK_KEYS or
CHECK_FIELD_NAMES) and use it to build both schema objects so properties and
required lists stay in sync—update the existing checks and check_notes
definitions (referencing the checks, check_notes, properties, and required
symbols) to derive their property names from that shared constant while keeping
types different (boolean for checks, string for check_notes); ensure confidence
remains unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/debug/run-artifact.ts`:
- Around line 24-31: sanitizePathSegment currently allows "." which lets ".."
survive and runId is interpolated into filenames, enabling path traversal;
update sanitizePathSegment to explicitly reject "." and ".." (return "unknown")
and strip or replace any path separator characters (e.g., "/" and "\") so only
alphanumeric, ".", "_", "-" remain, and keep the 80-char cap. Additionally
ensure wherever runId or other segments are injected into filenames (the code
that constructs the debug artifact path that currently interpolates runId) you
first pass them through sanitizePathSegment and then build paths using a safe
join to the fixed ".vectorlint/runs" directory (do not concatenate strings), so
untrusted values cannot escape the intended directory.

In `@src/debug/violation-filter.ts`:
- Around line 30-31: The code computes fixEmpty via const fixEmpty = (v.fix ??
"").trim() === "" but never records that in the reasons array, causing hidden
candidates without an explanation; update the logic around fixEmpty (and the
block that builds reasons for each violation—look for the code that aggregates
reasons between lines where fixEmpty is defined and where reasons are returned)
to push an explicit reason string such as "empty_fix" (or similar descriptive
label) into the reasons collection whenever fixEmpty is true so every
hidden/filtered candidate has a recorded hide reason.

---

Outside diff comments:
In `@src/prompts/directive-loader.ts`:
- Around line 49-51: Replace the bare catch block in directive-loader.ts (the
catch { ... } that currently ignores errors when reading the override file) with
a typed catch binding using the repository convention (bind the thrown value as
e: unknown), keep the existing comment about ignoring errors, and ensure no
runtime behavior changes beyond using the bound variable name (so the block
becomes a catch that accepts e: unknown and still ignores/handles the error as
before).

In `@src/prompts/schema.ts`:
- Line 50: The "line" field in the structured output schema currently allows any
number; update both occurrences of the "line" property to enforce positive
integers by replacing the type with z.number().int().min(1) (e.g., change line:
{ type: "number" } to use Zod: line: z.number().int().min(1)). Locate the "line"
properties in the schema definitions (both places currently using a numeric
type) and apply this Zod constraint so external data is validated as integers >=
1.

---

Nitpick comments:
In @.gitignore:
- Line 17: The .gitignore contains a broad "docs" rule that may unintentionally
ignore nested directories; change that entry to "/docs" so it is anchored to the
repository root. Locate the "docs" line in the .gitignore file and replace it
with "/docs" to ensure only the top-level docs directory is ignored.

In `@src/cli/orchestrator.ts`:
- Around line 32-53: getModelInfoFromEnv currently only reads env vars, so
model.tag can be missing for runs configured via ~/.vectorlint/config.toml;
change getModelInfoFromEnv to prefer runtime/provider metadata and fall back to
env: accept optional runtimeModel or providerClient input (or call a new helper
like getRuntimeModelInfo) and merge that metadata with env values so tag is
computed from runtime.provider/name first, then env, and finally omitted only if
neither exists; update callers that rely on model.tag (the places referenced as
680-681 and 778-779) to pass the runtime model-info or call the updated function
so artifacts consistently land under .vectorlint/runs/<model-tag>/...

In `@src/prompts/directive-loader.ts`:
- Around line 35-38: Remove the duplicated hard-constraint wording in the
directive text: keep a single canonical statement "Every quoted span must be
copied exactly from the Input." and delete the redundant duplicate that follows
it (the repeated line shown in the prompt block). Update the prompt constant or
template in src/prompts/directive-loader.ts (the hard-constraints block) so only
one occurrence of that exact sentence remains, preserving surrounding formatting
and line-numbering guidance.

In `@src/prompts/schema.ts`:
- Around line 59-99: The checks and check_notes JSON-schema fragments are
duplicated which risks contract drift; extract a single shared definition of the
keys (e.g., a constant like CHECK_KEYS or CHECK_FIELD_NAMES) and use it to build
both schema objects so properties and required lists stay in sync—update the
existing checks and check_notes definitions (referencing the checks,
check_notes, properties, and required symbols) to derive their property names
from that shared constant while keeping types different (boolean for checks,
string for check_notes); ensure confidence remains unchanged.

ℹ️ Review info

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f4c55ab and f18a3c4.

📒 Files selected for processing (19)
  • .gitignore
  • src/cli/commands.ts
  • src/cli/orchestrator.ts
  • src/cli/types.ts
  • src/debug/run-artifact.ts
  • src/debug/violation-filter.ts
  • src/evaluators/base-evaluator.ts
  • src/prompts/directive-loader.ts
  • src/prompts/schema.ts
  • src/schemas/cli-schemas.ts
  • src/scoring/scorer.ts
  • tests/evaluations/.vectorlint.ini
  • tests/evaluations/README.md
  • tests/evaluations/TEST_FILE.md
  • tests/evaluations/test-rules/Test/clarity.md
  • tests/evaluations/test-rules/Test/consistency.md
  • tests/evaluations/test-rules/Test/passive-voice.md
  • tests/evaluations/test-rules/Test/readability.md
  • tests/evaluations/test-rules/Test/wordiness.md

- Strip '.' from sanitizePathSegment allowlist to prevent '..'
  traversal via path.join in debug artifact paths
- Pass runId through sanitizePathSegment before use as filename
- Replace 10-field conditional spread in calculateCheckScore with
  a single object spread + criterionName alias
- Simplify getModelInfoFromEnv return to use && spread shorthand
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (3)
src/debug/run-artifact.ts (1)

4-22: Narrow DebugRunArtifact field types to match produced payloads.

filter_decisions and surfaced_violations are internal outputs with known structure; keeping them unknown reduces type-safety across writer/reader boundaries.

🧩 Suggested type tightening
 export type DebugRunArtifact = {
@@
-  prompt: {
+  prompt: {
     pack?: string;
     id?: string;
     filename?: string;
-    evaluation_type?: string;
+    evaluation_type?: "check" | "judge";
   };
   raw_model_output: unknown;
-  filter_decisions: unknown;
-  surfaced_violations: unknown;
+  filter_decisions: Array<{
+    index: number;
+    criterion?: string;
+    surface: boolean;
+    reasons: string[];
+  }>;
+  surfaced_violations: unknown[];
 };

As per coding guidelines: "Prefer explicit imports and narrow types in TypeScript ESM".

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/debug/run-artifact.ts` around lines 4 - 22, The DebugRunArtifact type
uses overly broad unknowns for filter_decisions and surfaced_violations; replace
them with explicit, exported types (e.g., declare and export FilterDecision and
SurfacedViolation interfaces and use filter_decisions: FilterDecision[] and
surfaced_violations: SurfacedViolation[]) that reflect the actual payload shape
(fields like decision, rule_id, reason, timestamp for FilterDecision and
violation_type, severity, details, locations for SurfacedViolation as
appropriate), and update any writer/reader sites that construct or consume
DebugRunArtifact to import and use those new types so type-safety is preserved
across boundaries.
src/cli/orchestrator.ts (2)

670-700: Extract debug artifact emission into one helper to prevent branch drift.

The check and judge blocks duplicate run-id/model lookup/write/try-catch flow. A shared helper will reduce future divergence risk.

♻️ Suggested consolidation
+function emitDebugArtifact(params: {
+  relFile: string;
+  promptFile: PromptFile;
+  promptId: string;
+  evaluationType: "check" | "judge";
+  rawModelOutput: unknown;
+  filterDecisions: unknown;
+  surfacedViolations: unknown;
+}): void {
+  const runId = randomUUID();
+  const model = getModelInfoFromEnv();
+  try {
+    const filePath = writeDebugRunArtifact(process.cwd(), runId, {
+      file: params.relFile,
+      ...(Object.keys(model).length > 0 ? { model } : {}),
+      subdir: model.tag,
+      prompt: {
+        pack: params.promptFile.pack,
+        id: params.promptId,
+        filename: params.promptFile.filename,
+        evaluation_type: params.evaluationType,
+      },
+      raw_model_output: params.rawModelOutput ?? null,
+      filter_decisions: params.filterDecisions,
+      surfaced_violations: params.surfacedViolations,
+    });
+    console.warn(`[vectorlint] Debug JSON written: ${filePath}`);
+  } catch (err: unknown) {
+    const message = err instanceof Error ? err.message : String(err);
+    console.warn(`[vectorlint] Debug JSON write failed: ${message}`);
+  }
+}

Also applies to: 762-802

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/cli/orchestrator.ts` around lines 670 - 700, Extract the duplicated
debug-artifact emission into a single helper (e.g., emitDebugJson or
writeDebugRunDebugArtifact) and call it from both the "check" and "judge"
branches; this helper should generate runId via randomUUID, call
getModelInfoFromEnv, compute decisions with
computeFilterDecision(result.violations), build the same payload (file: relFile,
optional model, subdir: model.tag, prompt details from promptFile and promptId,
raw_model_output from result, filter_decisions from decisions, and
surfaced_violations), invoke writeDebugRunArtifact(process.cwd(), runId,
payload) inside a try/catch, and replicate the current console warnings on
success or failure; replace the duplicated blocks with a single call to this
helper so behavior and logging remain identical.

687-687: Avoid ad-hoc casts for raw_model_output; encode it in the result type contract.

Line 687 and Line 785 currently rely on local assertions. This weakens the structured-output guarantee and makes schema drift easier to miss at compile time.

As per coding guidelines: "Prefer explicit imports and narrow types in TypeScript ESM" and "Use strict TypeScript with no any; use unknown + schema validation for external data".

Also applies to: 785-785

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@src/cli/orchestrator.ts`:
- Around line 670-700: Extract the duplicated debug-artifact emission into a
single helper (e.g., emitDebugJson or writeDebugRunDebugArtifact) and call it
from both the "check" and "judge" branches; this helper should generate runId
via randomUUID, call getModelInfoFromEnv, compute decisions with
computeFilterDecision(result.violations), build the same payload (file: relFile,
optional model, subdir: model.tag, prompt details from promptFile and promptId,
raw_model_output from result, filter_decisions from decisions, and
surfaced_violations), invoke writeDebugRunArtifact(process.cwd(), runId,
payload) inside a try/catch, and replicate the current console warnings on
success or failure; replace the duplicated blocks with a single call to this
helper so behavior and logging remain identical.

In `@src/debug/run-artifact.ts`:
- Around line 4-22: The DebugRunArtifact type uses overly broad unknowns for
filter_decisions and surfaced_violations; replace them with explicit, exported
types (e.g., declare and export FilterDecision and SurfacedViolation interfaces
and use filter_decisions: FilterDecision[] and surfaced_violations:
SurfacedViolation[]) that reflect the actual payload shape (fields like
decision, rule_id, reason, timestamp for FilterDecision and violation_type,
severity, details, locations for SurfacedViolation as appropriate), and update
any writer/reader sites that construct or consume DebugRunArtifact to import and
use those new types so type-safety is preserved across boundaries.

ℹ️ Review info

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f18a3c4 and b423eac.

📒 Files selected for processing (3)
  • src/cli/orchestrator.ts
  • src/debug/run-artifact.ts
  • src/scoring/scorer.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/scoring/scorer.ts

@oshorefueled oshorefueled merged commit 461c38e into main Mar 2, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant