Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

fix(csv): reject __expected0 instead of silently dropping its config#9815

Merged
ianw-oai merged 1 commit into
promptfoo:mainpromptfoo/promptfoo:mainfrom
he-yufeng:fix/csv-expected-index-zerohe-yufeng/promptfoo:fix/csv-expected-index-zeroCopy head branch name to clipboard
Jun 21, 2026
Merged

fix(csv): reject __expected0 instead of silently dropping its config#9815
ianw-oai merged 1 commit into
promptfoo:mainpromptfoo/promptfoo:mainfrom
he-yufeng:fix/csv-expected-index-zerohe-yufeng/promptfoo:fix/csv-expected-index-zeroCopy head branch name to clipboard

Conversation

@he-yufeng

Copy link
Copy Markdown
Contributor

What

A __config:__expected0:threshold column is silently dropped instead of being rejected.

__expected<N> indices are 1-based (__expected1 → assertion 0). The parser runs Number.parseInt(n, 10) - 1, so __expected0 becomes -1. That isn't undefined, so it slips past the "must be a positive integer" guard, and the config is written to assertionConfigs[-1] — a property the apply loop (for i = 0..asserts.length) never reads. The threshold just vanishes with no error.

This is inconsistent with __expectedX (non-numeric), which already throws Invalid expected key ... Must be __expected or __expected<N> where N is a positive integer. This change makes __expected0 take that same path by only assigning targetIndex when the index is >= 1.

Why

The error message already promises that N must be a positive integer, but 0 was accepted and then silently no-op'd. A user who indexes from 0 loses their threshold config without any signal. Erroring matches the documented contract and the existing __expectedX behavior.

Testing

Added a test/csv.test.ts case asserting __config:__expected0:threshold throws the same "positive integer" error as __expectedX. It fails on main (expected [Function] to throw an error, since the config is silently dropped) and passes with this change. npx vitest run test/csv.test.ts is green; tsc --noEmit and prettier/biome are clean (the pre-existing complexity warning on testCaseFromCsvRow is unrelated).

@he-yufeng
he-yufeng requested review from a team and mldangelo-oai as code owners June 19, 2026 16:18
@codecov

codecov Bot commented Jun 19, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 79.26%. Comparing base (1be089f) to head (72fbee8).

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #9815   +/-   ##
=======================================
  Coverage   79.26%   79.26%           
=======================================
  Files         917      917           
  Lines       73607    73609    +2     
  Branches    23661    23662    +1     
=======================================
+ Hits        58342    58344    +2     
  Misses      15265    15265           
Flag Coverage Δ
backend 81.14% <100.00%> (+<0.01%) ⬆️
site 3.94% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@he-yufeng
he-yufeng force-pushed the fix/csv-expected-index-zero branch 2 times, most recently from 9ecc4f1 to a9d227f Compare June 20, 2026 13:06
@he-yufeng
he-yufeng force-pushed the fix/csv-expected-index-zero branch from a9d227f to 72fbee8 Compare June 21, 2026 17:36
@ianw-oai
ianw-oai merged commit 5097c23 into promptfoo:main Jun 21, 2026
33 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.

2 participants

Morty Proxy This is a proxified and sanitized view of the page, visit original site.