Repository navigation
Arm Phase-2 evidence: rules/red-flag eval, live extraction baseline, and the citation-resolution fix it surfaced - #2
Conversation
The first live extraction eval surfaced a defect keyless fakes had hidden:
citedValue.citations validated strictly as clauseIdSchema, so when the small
model didn't echo the "{uuid}:{page}:{clause_path}" id verbatim the whole-object
safeParse failed -> allFailed -> every field extraction_failed. Two of four demo
doc types extracted zero fields despite the model returning correct values.
- field.ts: citations accept any string. They are model HINTS resolved
structurally, never a validation gate (aligns with ADR-0006's own intent).
- citation-resolver.ts: resolve anchor-first — the clause whose frozen text
contains the exact verbatim_anchor is the citation; the model's ids only
disambiguate a repeated anchor; an ambiguous anchor with no hint stays
unresolved (never fuzzy-match, ADR-0005).
Extraction accuracy 0.64 -> 0.95 and not_found precision 0.21 -> 1.00 on the
demo corpus (Haiku 4.5).
Co-Authored-By: Claude Opus 4.8 <[email protected]>
Corpus-level precision/recall/F1 of the rules engine vs human-labeled expected flags, over each demo doc's ground-truth extraction. Because the engine is pure (no LLM, no DB) the eval needs no key and no Postgres — it runs on every PR, never fail-soft, and version-pins to rules.yaml (a ruleset bump forces a deliberate re-baseline). - rules-eval.ts / rules-metrics.ts / rules-gold.ts, with gold/flags.jsonl (24 expected flags across 5 docs, per-severity, plus notExpected guards for near-miss boundaries like capped-vs-uncapped preference). - pnpm eval:rules (--gate / --write-baseline); always-on rules job in eval.yml. Rules gold reconstructs each doc's extraction from gold/extraction.jsonl exactly as the rules unit tests do (mkExtraction), so the eval isolates rule correctness from extraction accuracy. F1 = 1.00 on the demo corpus. Co-Authored-By: Claude Opus 4.8 <[email protected]>
First deliberate live baseline (ADR-0006 governance): - extraction: anthropic:claude-haiku-4-5-20251001 — accuracy 0.955, not_found precision 1.00, hallucination 0.00, citation recall 0.43 (n=48), with a committed content-addressed LLM-response cache so gated runs are cheap and reproducible offline. - rules: F1 1.00 (24 flags / 5 docs), deterministic and keyless. README shows the real numbers. Retrieval baseline still pending JINA_API_KEY; the CI extraction gate arms once ANTHROPIC_API_KEY is set as an Actions secret. Co-Authored-By: Claude Opus 4.8 <[email protected]>
Reorder the README to open with value, not architecture: a "Who it's for" section (the three concrete personas), a "What it catches" table sampling the real 31-rule engine with severity and statutory sources, and a "One engine, many document types" note on where the extract -> deterministic-rules -> cited report spine generalizes (NDAs, DPAs, EU-regulation gap analysis). Status and the eval numbers stay, now below the value framing. Co-Authored-By: Claude Opus 4.8 <[email protected]>
|
Warning Review limit reached
Next review available in: 54 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughThe PR changes citation resolution to use verbatim anchors with clause-id hints, adds deterministic red-flag evaluation and metrics, records evaluation artifacts, documents the workflow, and runs the rules gate in pull-request CI. ChangesCitation Resolution and Rules Evaluation
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant PullRequest
participant GitHubActions
participant RulesEval
participant RulesEngine
participant Baseline
PullRequest->>GitHubActions: open or update pull request
GitHubActions->>RulesEval: run pnpm eval:rules -- --gate
RulesEval->>RulesEngine: evaluate reconstructed extraction
RulesEngine-->>RulesEval: fired rules and severities
RulesEval->>Baseline: compare precision, recall, and F1
Baseline-->>GitHubActions: pass or fail gate
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
CI's checks job runs `format:check` (prettier --check) after lint; the four files added or edited on this branch needed reformatting. Purely cosmetic — line reflows and whitespace, no logic change (lint/typecheck/test still green). Co-Authored-By: Claude Opus 4.8 <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/eval.yml:
- Line 145: Update the actions/checkout step in the rules job to set
persist-credentials to false, ensuring subsequent evaluation steps cannot access
the checkout token.
In `@packages/api/src/extraction/citation-resolver.ts`:
- Line 47: Update the anchor handling in the citation resolver to preserve the
raw verbatim_anchor for matching and slicing; use only a trimmed length check to
detect an empty anchor, ensuring downstream operations slice canonical text
without transforming the anchor.
- Around line 77-81: Update the chosen-clause selection in the citation resolver
so an anchor resolves only when exactly one containing clause is also present in
hintIds. Do not use containing.find() to select among multiple hinted clauses;
leave multiply-hinted anchors undefined while preserving the existing
single-containing-clause fallback.
In `@packages/eval/src/rules-eval.ts`:
- Around line 137-140: In the evaluation flow after computeRulesMetrics and
before writeResults or appendHistory, check whether violations are present and
exit immediately for an invalid run. Keep printSummary available before the
guard, but ensure neither artifact-writing function executes when violations is
nonzero.
In `@packages/eval/src/rules-gold.ts`:
- Around line 22-28: Update goldFlagSchema to use Zod’s strict object schema so
unknown top-level fields such as expectd are rejected instead of stripped and
replaced by defaults. Preserve the existing doc, expected, and notExpected
validations and defaults.
In `@README.md`:
- Line 39: Update the Extraction description in README.md to avoid promising
citations for every field: state that fields carry structural citations when
grounded, and explicitly surface unresolved citations when grounding is
unavailable or ambiguous while preserving extracted values.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: e2cfffdf-c33a-4964-a619-3079a3f7d592
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (21)
.github/workflows/eval.ymlCLAUDE.mdREADME.mddocs/adr/0007-anchor-first-citation-resolution.mddocs/architecture.mdpackage.jsonpackages/api/src/extraction/citation-resolver.test.tspackages/api/src/extraction/citation-resolver.tspackages/eval/baselines/extraction.jsonpackages/eval/baselines/rules.jsonpackages/eval/cache/llm.jsonlpackages/eval/gold/flags.jsonlpackages/eval/history/extraction.jsonlpackages/eval/history/rules.jsonlpackages/eval/package.jsonpackages/eval/src/rules-eval.tspackages/eval/src/rules-gold.tspackages/eval/src/rules-metrics.test.tspackages/eval/src/rules-metrics.tspackages/shared/src/schemas/extraction/extraction.test.tspackages/shared/src/schemas/extraction/field.ts
| rules: | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - uses: actions/checkout@v7 |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== workflow excerpt =="
if [ -f .github/workflows/eval.yml ]; then
nl -ba .github/workflows/eval.yml | sed -n '110,190p'
else
echo "missing .github/workflows/eval.yml"
fi
echo
echo "== checkout occurrences in workflow =="
rg -n "actions/checkout|persist-credentials|git (push|fetch|clone|add|commit|tag|config|push)" .github/workflows/eval.yml || true
echo
echo "== changed files/stat =="
git diff --stat HEAD~1..HEAD 2>/dev/null || git diff --stat || true
echo
echo "== checkout docs pinned version availability =="
python3 - <<'PY'
import urllib.request, urllib.error
print("actions/checkout@v7 exists?", end=" ")
try:
req = urllib.request.urlopen("https://github.com/actions/checkout/releases/tag/v7", timeout=10)
print("yes", req.getcode())
except urllib.error.HTTPError as e:
print("no", e.code)
except Exception as e:
print("error", type(e).__name__, e)
PYRepository: just-nilux/contractix
Length of output: 222
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== workflow excerpt =="
if [ -f .github/workforms/eval.yml ]; then
awk '{printf "%6d %s\n", NR, $0}' .github/workflows/eval.yml | sed -n '110,190p'
else
echo "missing .github/workflows/eval.yml"
fi
echo
echo "== checkout occurrences in workflow =="
grep -n "actions/checkout\|persist-credentials\|git \(push\|fetch\|clone\|add\|commit\|tag\|config\|push\)" .github/workflows/eval.yml || true
echo
echo "== changed files/stat =="
git diff --stat HEAD~1..HEAD 2>/dev/null || git diff --stat || true
echo
echo "== file existence/details =="
ls -la .github/workflows/eval.yml || trueRepository: just-nilux/contractix
Length of output: 601
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== .github/workflows/eval.yml lines 130-155 =="
awk '{printf "%6d %s\n", NR, $0}' .github/workflows/eval.yml | sed -n '130,155p'
echo
echo "== git operations across eval workflow =="
grep -InE '\bgits?|git (push|fetch|clone|add|commit|tag|config|push)|persist-credentials|actions/checkout' .github/workflows/eval.yml || true
echo
echo "== checkout@v7 persist-credentials documentation from release files =="
python3 - <<'PY'
import urllib.request, json
url = "https://api.github.com/repos/actions/checkout/git/ref/tags/v7"
try:
req = urllib.request.urlopen(url, timeout=10)
obj = json.load(req)
print("ref:", obj.get("ref"))
print("object:", obj.get("object") or {})
except Exception as e:
print("API error:", type(e).__name__, e)
PYRepository: just-nilux/contractix
Length of output: 1877
🌐 Web query:
actions/checkout persist-credentials default true README
💡 Result:
In the actions/checkout GitHub Action, the persist-credentials input determines whether the authentication token or SSH key is configured in the local git configuration [1][2]. By default, this setting is true [1][3]. When enabled, it allows your workflow scripts to run authenticated git commands within the workspace [1][4]. The action automatically removes these credentials during the post-job cleanup [1][5]. If you wish to opt out of this behavior—for example, to prevent the credentials from being available to subsequent steps or to enhance security—you should explicitly set persist-credentials to false [1][3][5]. Technical context: - Mechanism: Since version v6.0.0, the action stores these credentials under the $RUNNER_TEMP directory rather than directly in the repository's.git/config, which improves security and compatibility with environments like Docker container actions [2][6]. - Security considerations: While the default is true to ensure ease of use for git operations, some users prefer setting it to false to minimize the risk of credential exposure [7][5].
Citations:
- 1: https://github.com/actions/checkout?tab=readme-ov-file
- 2: https://github.com/actions/checkout/tree/v6.0.0
- 3: https://github.com/marketplace/actions/checkout?version=v3.6.0
- 4: https://github.com/actions/checkout/tree/v6.0.2?tab=readme-ov-file
- 5: https://spacelift.io/blog/github-actions-checkout
- 6: actions/checkout@v5.0.1...v6.0.0
- 7: Change the default value of persist-credentials to false actions/checkout#1687
Disable persisted checkout credentials.
The rules job only runs repository-controlled evaluation scripts and does not perform authenticated Git operations after checkout. Set persist-credentials: false to avoid making the workflow token available to later steps.
🧰 Tools
🪛 zizmor (1.26.1)
[warning] 145-145: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/eval.yml at line 145, Update the actions/checkout step in
the rules job to set persist-credentials to false, ensuring subsequent
evaluation steps cannot access the checkout token.
Source: Linters/SAST tools
| ): ResolvedField { | ||
| const citations: ResolvedCitation[] = []; | ||
| const unresolved: string[] = []; | ||
| const anchor = field.verbatim_anchor.trim(); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Keep the verbatim anchor unchanged.
trim() changes the anchor before matching and slicing, so leading/trailing whitespace can produce a different cited span. Retain the raw anchor; only use anchor.trim().length === 0 to detect an empty anchor.
As per coding guidelines, downstream code must only slice canonical text and never transform it.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/api/src/extraction/citation-resolver.ts` at line 47, Update the
anchor handling in the citation resolver to preserve the raw verbatim_anchor for
matching and slicing; use only a trimmed length check to detect an empty anchor,
ensuring downstream operations slice canonical text without transforming the
anchor.
Source: Coding guidelines
| const containing = [...clausesByRef.values()].filter((c) => c.text.includes(anchor)); | ||
| const hintIds = new Set(hints.map((c) => c.id)); | ||
| const chosen = | ||
| containing.find((c) => hintIds.has(c.id)) ?? | ||
| (containing.length === 1 ? containing[0] : undefined); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Leave multiply-hinted anchors unresolved.
When two valid hints both contain the anchor, find() selects whichever clause appears first in the map. That is still ambiguous and violates the no-guess rule; resolve only if exactly one hinted containing clause exists.
Proposed fix
- const chosen =
- containing.find((c) => hintIds.has(c.id)) ??
- (containing.length === 1 ? containing[0] : undefined);
+ const hintedContaining = containing.filter((c) => hintIds.has(c.id));
+ const chosen =
+ hintedContaining.length === 1
+ ? hintedContaining[0]
+ : hintedContaining.length === 0 && containing.length === 1
+ ? containing[0]
+ : undefined;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const containing = [...clausesByRef.values()].filter((c) => c.text.includes(anchor)); | |
| const hintIds = new Set(hints.map((c) => c.id)); | |
| const chosen = | |
| containing.find((c) => hintIds.has(c.id)) ?? | |
| (containing.length === 1 ? containing[0] : undefined); | |
| const containing = [...clausesByRef.values()].filter((c) => c.text.includes(anchor)); | |
| const hintIds = new Set(hints.map((c) => c.id)); | |
| const hintedContaining = containing.filter((c) => hintIds.has(c.id)); | |
| const chosen = | |
| hintedContaining.length === 1 | |
| ? hintedContaining[0] | |
| : hintedContaining.length === 0 && containing.length === 1 | |
| ? containing[0] | |
| : undefined; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/api/src/extraction/citation-resolver.ts` around lines 77 - 81,
Update the chosen-clause selection in the citation resolver so an anchor
resolves only when exactly one containing clause is also present in hintIds. Do
not use containing.find() to select among multiple hinted clauses; leave
multiply-hinted anchors undefined while preserving the existing
single-containing-clause fallback.
Source: Coding guidelines
| const metrics = computeRulesMetrics(obs, flagsGold.length); | ||
| printSummary(metrics, misses, falsePositives, violations); | ||
| writeResults(metrics, misses, falsePositives); | ||
| appendHistory(metrics); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Do not persist invalid evaluation runs.
Gold violations are only rejected after writeResults() and appendHistory(). A failing run therefore pollutes artifacts with metrics known to be invalid. Exit on violations before writing either artifact.
Proposed fix
const metrics = computeRulesMetrics(obs, flagsGold.length);
printSummary(metrics, misses, falsePositives, violations);
- writeResults(metrics, misses, falsePositives);
- appendHistory(metrics);
- // Severity mismatches / notExpected violations are hard errors regardless of the numeric gate:
- // they mean the gold is stale relative to the engine and must be reconciled deliberately.
if (violations.length > 0) {
console.error(`[eval] RULES GOLD VIOLATIONS:\n - ${violations.join("\n - ")}`);
process.exit(1);
}
+ writeResults(metrics, misses, falsePositives);
+ appendHistory(metrics);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const metrics = computeRulesMetrics(obs, flagsGold.length); | |
| printSummary(metrics, misses, falsePositives, violations); | |
| writeResults(metrics, misses, falsePositives); | |
| appendHistory(metrics); | |
| const metrics = computeRulesMetrics(obs, flagsGold.length); | |
| printSummary(metrics, misses, falsePositives, violations); | |
| if (violations.length > 0) { | |
| console.error(`[eval] RULES GOLD VIOLATIONS:\n - ${violations.join("\n - ")}`); | |
| process.exit(1); | |
| } | |
| writeResults(metrics, misses, falsePositives); | |
| appendHistory(metrics); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/eval/src/rules-eval.ts` around lines 137 - 140, In the evaluation
flow after computeRulesMetrics and before writeResults or appendHistory, check
whether violations are present and exit immediately for an invalid run. Keep
printSummary available before the guard, but ensure neither artifact-writing
function executes when violations is nonzero.
| **Phases 0–2 complete** — the ingestion + retrieval spine and the extraction + red-flag engine are built and tested. Phase 3 (agentic Q&A, full report, web UI) is next. See [PRD.md](PRD.md) for the full specification and roadmap. | ||
|
|
||
| - **Ingestion & retrieval** — layout-aware PDF/DOCX parse → clause segmentation → chunking → pgvector + full-text + trigram hybrid search with cross-encoder rerank. | ||
| - **Extraction** — schema-first, per-field-cited extraction (employment offers/contracts, VSOP/ESOP, term sheets); every field carries a structural citation to the exact clause span, and `not_found` is a first-class value, never inferred. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Do not promise a resolved citation for every field.
ADR-0007 intentionally preserves extracted values when grounding is unavailable or ambiguous. State that fields carry structural citations when grounded, with unresolved citations explicitly surfaced otherwise.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@README.md` at line 39, Update the Extraction description in README.md to
avoid promising citations for every field: state that fields carry structural
citations when grounded, and explicitly surface unresolved citations when
grounding is unavailable or ambiguous while preserving extracted values.
What & why
Closes Phase 2's evidence gap — turns "we have an eval harness" into pinned, gated, real numbers, and fixes a genuine extraction defect that arming the eval surfaced. Four focused commits:
feat(eval)— a deterministic rules / red-flag eval (FR-4, §7 E-2): corpus-level precision/recall/F1 of the 31-rule engine vs human-labeled expected flags (gold/flags.jsonl), over each demo doc's ground-truth extraction. Pure (no LLM, no DB) → keyless, no Postgres, runs on every PR and version-pins torules.yaml. F1 = 1.00.fix(extraction)+ ADR-0007 — the first live extraction run exposed that strictclauseIdSchemavalidation on model citations failed the whole-objectsafeParse, zeroing out 2 of 4 doc types (extraction_failed) despite correct values. Fix: citations are model hints (accept any string) resolved anchor-first — the clause containing the exactverbatim_anchoris the citation; the model's ids only disambiguate a repeat, and an ambiguous anchor with no hint stays unresolved (no fuzzy matching, ADR-0005).chore(eval)— pins the extraction + rules baselines from a live Haiku run, with a committed content-addressed LLM-response cache for cheap, reproducible gated runs.docs(readme)— leads with use-cases + a red-flag showcase.Checklist
pnpm lint && pnpm typecheck && pnpm testgreen locally (162 unit tests; +6 new)Baseline / judge changes
Two new baselines pinned — none existed before (the committed numbers were keyless fakes).
Extraction (
baselines/extraction.json·anthropic:claude-haiku-4-5-20251001· n=48):Legitimate: it's the first real run (the prior artifact was
fake:llm, every fieldnot_found), and it directly drove the ADR-0007 fix — the pre-fix live run measured 0.64 accuracy / 0.27 citation recall and was not committed; the post-fix run is committed alongsidecache/llm.jsonlso-- --gateis cheap and deterministic. Citation recall (0.43) is reported, not gated (the gate floors on accuracy ≥ 0.80, max regression 0.02, LLM-identity match).Rules (
baselines/rules.json· deterministic/keyless · ruleset v1): precision / recall / F1 = 1.00 over 24 expected flags across 5 docs. Gold authored from the demo docs and validated by running the engine over the ground-truth extraction (0 FP / 0 FN), withnotExpectedguards on near-miss boundaries (capped-vs-uncapped preference, standard vesting, non-cumulative dividends, probation at the limit). Gate: F1 floor 0.90, max regression 0.02, ruleset-version match (arules.yamlbump forces a deliberate re-baseline).CI: the extraction gate arms once
ANTHROPIC_API_KEYis set as an Actions secret; the retrieval baseline is still pendingJINA_API_KEY(that job stays fail-soft until then).🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Documentation