feat(config): add YAML source location to prompt reference errors#9940
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #9940 +/- ##
==========================================
+ Coverage 79.42% 79.46% +0.03%
==========================================
Files 922 924 +2
Lines 74008 74130 +122
Branches 23821 23861 +40
==========================================
+ Hits 58784 58905 +121
- Misses 15224 15225 +1
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 68b257b2d0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0008b8c4f0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| testSuite.tests || [], | ||
| testSuite.prompts, | ||
| typeof testSuite.defaultTest === 'object' ? testSuite.defaultTest : undefined, | ||
| { promptReferenceSources }, |
There was a problem hiding this comment.
Avoid using config locations for CLI-supplied tests
When --tests/--vars overrides the config tests, parsedTests comes from the CLI path (config.tests is set from cmdObj.tests || cmdObj.vars), but the validator is still given only promptReferenceSources from the config files. If the ignored config file happens to contain the same missing prompt name, the new error prefix points at that unused config line instead of the test file actually being validated, which sends users to edit the wrong source.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 65db3da327
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const column = line.indexOf(ref); | ||
| return column === -1 ? undefined : `${source.path}:${lineNumber}:${column + 1}`; |
There was a problem hiding this comment.
Avoid substring matches for prompt locations
When the missing prompt name is a substring of an earlier prompt reference in the same prompts block, this raw indexOf returns the earlier line and item.location ??= keeps it. For example, if a test lists Basic Assistant before missing Basic, and only Basic Assistant exists, the error for Basic points at the valid Basic Assistant entry instead of the bad line; the lookup should match the YAML list value rather than any substring on the line.
Useful? React with 👍 / 👎.
…ator A missing prompt whose name is a substring of a valid prompt referenced earlier in the same prompts block (e.g. 'Ghost' vs '- GhostBuster') was located via a raw indexOf, so the error pointed at the valid prompt's line. Prefer a list item whose whole value equals the ref, falling back to a substring match only when there's no exact match (keeps flow-style 'prompts: [ref]' working).
|
Reviewed and QA'd this thoroughly — the location hints are a genuinely nice DX win, and I verified the hand-rolled locator holds up well on real configs (via actual
On the three Codex P2s: #1 (ref-in-vars stealing the location) is already handled by the I pushed a small fix ( Leaving the merge to you since I added to it — ready once CI is green. |
|
Thank you! |
Summary
Prompt reference validation errors now point at the offending line in the configuration file.
Before
After