Skip to content

Commit 94d98cf

Browse files
Merge pull request #11 from PredictabilityAtScale:codex/review-prompt-templates-for-validation-issues
Improve prompt validation with new static warnings for inputs, cache, provider, and tools
2 parents f2df40e + 5f41130 commit 94d98cf

3 files changed

Lines changed: 215 additions & 3 deletions

File tree

docs/validation.md

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,14 @@ const result = await kit.validatePrompt('support/reply');
3636
| `POK012` | Warning | Variable declared in `context.inputs` but never used |
3737
| `POK013` | Error | Invalid context regex pattern (`allow_regex` or `deny_regex`), including prompt id, variable name, field name, and raw configured value |
3838
| `POK014` | Warning | `trim` configured without `max_size` (trim-to-budget skipped) |
39+
| `POK040` | Warning | Risky context input appears unbounded (`max_size` missing) |
40+
| `POK041` | Warning | Context input has no hardening validators (`allow/deny regex`, `non_empty`, `reject_secrets`) |
41+
| `POK042` | Warning | Provider has no provider-specific cache config |
42+
| `POK043` | Warning | `cache.gemini.cached_content` and `cache.google.cached_content` conflict |
43+
| `POK044` | Warning | Provider configured without an explicit `model` |
44+
| `POK045` | Warning | Environment/tier cache override may be missing while base cache is defined |
45+
| `POK046` | Warning | Template uses variables but `context.inputs` is not declared |
46+
| `POK047` | Warning | Inline tool definition missing `description` or `input_schema` |
3947
| `POK033` | Runtime error | `non_empty` validation failed |
4048
| `POK034` | Runtime error | `reject_secrets` validation matched |
4149
| `POK020` | Error | Include resolution failed (missing file) |
@@ -49,7 +57,7 @@ Unknown front matter keys are checked against known keys using Levenshtein dista
4957
⚠ POK010: Unknown front matter field: "tempreature" (Did you mean "temperature"?)
5058
```
5159

52-
Known front matter keys: `id`, `schema_version`, `description`, `provider`, `model`, `fallback_models`, `reasoning`, `sampling`, `response`, `tools`, `mcp`, `context`, `includes`, `environments`, `tiers`, `metadata`.
60+
Known front matter keys: `id`, `schema_version`, `description`, `provider`, `model`, `fallback_models`, `reasoning`, `sampling`, `response`, `tools`, `mcp`, `context`, `includes`, `environments`, `tiers`, `metadata`, `cache`, `provider_options`.
5361

5462
## Variable validation
5563

@@ -114,6 +122,9 @@ context:
114122
- `reject_secrets` rejects common secret-like strings with `POK034`, unless `return_message` is configured.
115123
- During static validation and compilation, malformed `allow_regex` or `deny_regex` patterns are reported as `POK013`.
116124
- During static validation, `trim` without `max_size` returns a `POK014` warning.
125+
- During static validation, risky unbounded inputs and missing hardening are flagged as `POK040` and `POK041`.
126+
- During static validation, provider/cache hygiene checks can emit `POK042`–`POK045`.
127+
- During static validation, inline tool quality checks can emit `POK047`.
117128

118129
Regex compilation errors include the prompt id, variable name, field name, and raw configured value to make bad prompt definitions easy to locate.
119130

src/validation/validate.ts

Lines changed: 144 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@ import { extractVariables } from '../renderer/index.js';
44
import { resolveIncludes } from '../composition/index.js';
55
import {
66
compileContextRegex,
7-
formatInvalidContextRegexMessage,
87
getContextInputs,
98
getContextInputNames,
109
} from '../context.js';
@@ -26,9 +25,21 @@ export interface PromptValidationResult {
2625
const KNOWN_FRONT_MATTER_KEYS = new Set([
2726
'id', 'schema_version', 'description', 'provider', 'model', 'fallback_models',
2827
'reasoning', 'sampling', 'response', 'tools', 'mcp', 'context', 'includes',
29-
'environments', 'tiers', 'metadata', 'cache',
28+
'environments', 'tiers', 'metadata', 'cache', 'provider_options',
3029
]);
3130

31+
const RISKY_UNBOUNDED_INPUT_NAMES = [
32+
'message',
33+
'prompt',
34+
'history',
35+
'transcript',
36+
'document',
37+
'content',
38+
'input',
39+
'body',
40+
'context',
41+
];
42+
3243
/**
3344
* Validate a parsed prompt asset, returning all errors and warnings.
3445
*/
@@ -121,8 +132,42 @@ export function validateAsset(
121132
}
122133
}
123134

135+
if (usedVars.size > 0 && (!asset.context?.inputs || asset.context.inputs.length === 0)) {
136+
warnings.push({
137+
code: 'POK046',
138+
message: `Template uses ${usedVars.size === 1 ? 'a variable' : 'variables'} but context.inputs is not declared.`,
139+
filePath,
140+
suggestion: 'Declare context.inputs to enable input policy validation.',
141+
});
142+
}
143+
124144
// Context regex definitions compile successfully
125145
for (const input of getContextInputs(asset)) {
146+
const lowerName = input.name.toLowerCase();
147+
148+
if (input.max_size === undefined && RISKY_UNBOUNDED_INPUT_NAMES.some((needle) => lowerName.includes(needle))) {
149+
warnings.push({
150+
code: 'POK040',
151+
message: `Context input "${input.name}" has no max_size and appears unbounded.`,
152+
filePath,
153+
suggestion: 'Add max_size to constrain prompt payload growth.',
154+
});
155+
}
156+
157+
if (
158+
input.allow_regex === undefined
159+
&& input.deny_regex === undefined
160+
&& input.non_empty === undefined
161+
&& input.reject_secrets === undefined
162+
) {
163+
warnings.push({
164+
code: 'POK041',
165+
message: `Context input "${input.name}" has no input hardening validators.`,
166+
filePath,
167+
suggestion: 'Consider non_empty/reject_secrets and allow/deny regex validators.',
168+
});
169+
}
170+
126171
if (input.trim !== undefined && input.trim !== false && input.max_size === undefined) {
127172
warnings.push({
128173
code: 'POK014',
@@ -157,6 +202,103 @@ export function validateAsset(
157202
}
158203
}
159204

205+
if (asset.provider) {
206+
let providerCache: unknown;
207+
let cacheSuggestionField: string | undefined;
208+
209+
switch (asset.provider) {
210+
case 'openai':
211+
providerCache = asset.cache?.openai;
212+
cacheSuggestionField = 'cache.openai';
213+
break;
214+
case 'anthropic':
215+
providerCache = asset.cache?.anthropic;
216+
cacheSuggestionField = 'cache.anthropic';
217+
break;
218+
case 'gemini':
219+
case 'google':
220+
providerCache = asset.cache?.gemini ?? asset.cache?.google;
221+
cacheSuggestionField = 'cache.gemini';
222+
break;
223+
default:
224+
break;
225+
}
226+
227+
if (cacheSuggestionField && providerCache === undefined) {
228+
warnings.push({
229+
code: 'POK042',
230+
message: `Provider "${asset.provider}" has no provider-specific cache settings.`,
231+
filePath,
232+
suggestion: `Consider configuring ${cacheSuggestionField} for better cache-hit behavior.`,
233+
});
234+
}
235+
236+
if (!asset.model) {
237+
warnings.push({
238+
code: 'POK044',
239+
message: `Provider "${asset.provider}" is configured without a model.`,
240+
filePath,
241+
suggestion: 'Set model in prompt or defaults to avoid adapter-time errors.',
242+
});
243+
}
244+
}
245+
246+
if (
247+
asset.cache?.gemini?.cached_content
248+
&& asset.cache.google?.cached_content
249+
&& asset.cache.gemini.cached_content !== asset.cache.google.cached_content
250+
) {
251+
warnings.push({
252+
code: 'POK043',
253+
message: 'cache.gemini.cached_content and cache.google.cached_content are both set to different values.',
254+
filePath,
255+
suggestion: 'Use one canonical value; Gemini prefers cache.gemini.cached_content.',
256+
});
257+
}
258+
259+
for (const [envName, overrides] of Object.entries(asset.environments ?? {})) {
260+
if (asset.cache && !overrides.cache) {
261+
warnings.push({
262+
code: 'POK045',
263+
message: `Environment "${envName}" does not override cache while prompt-level cache is defined.`,
264+
filePath,
265+
suggestion: 'Confirm cache strategy is intentionally shared across environments.',
266+
});
267+
}
268+
}
269+
270+
for (const [tierName, overrides] of Object.entries(asset.tiers ?? {})) {
271+
if (asset.cache && !overrides.cache) {
272+
warnings.push({
273+
code: 'POK045',
274+
message: `Tier "${tierName}" does not override cache while prompt-level cache is defined.`,
275+
filePath,
276+
suggestion: 'Confirm cache strategy is intentionally shared across tiers.',
277+
});
278+
}
279+
}
280+
281+
for (const tool of asset.tools ?? []) {
282+
if (typeof tool !== 'string') {
283+
if (!tool.description) {
284+
warnings.push({
285+
code: 'POK047',
286+
message: `Inline tool "${tool.name}" is missing a description.`,
287+
filePath,
288+
suggestion: 'Add description to improve model tool-selection quality.',
289+
});
290+
}
291+
if (!tool.input_schema) {
292+
warnings.push({
293+
code: 'POK047',
294+
message: `Inline tool "${tool.name}" is missing input_schema.`,
295+
filePath,
296+
suggestion: 'Add input_schema so tool inputs are strongly typed.',
297+
});
298+
}
299+
}
300+
}
301+
160302
return {
161303
valid: errors.length === 0,
162304
errors,

tests/validation.test.ts

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,9 @@ describe('validateAsset', () => {
6666
sections: { prompt_template: '{{ pull_request }}' },
6767
});
6868
const warning = result.warnings.find((w) => w.code === 'POK011');
69+
const policyWarning = result.warnings.find((w) => w.code === 'POK046');
6970
expect(warning).toBeDefined();
71+
expect(policyWarning).toBeDefined();
7072
expect(warning?.message).toContain('pull_request');
7173
});
7274

@@ -175,6 +177,63 @@ describe('validateAsset', () => {
175177
expect(result.warnings.some((warning) => warning.code === 'POK014')).toBe(true);
176178
});
177179

180+
it('warns on risky unbounded context inputs and missing hardening', () => {
181+
const result = validateAsset({
182+
id: 'test',
183+
schema_version: 1,
184+
context: {
185+
inputs: [{ name: 'user_message' }],
186+
},
187+
sections: { prompt_template: '{{ user_message }}' },
188+
});
189+
190+
expect(result.valid).toBe(true);
191+
expect(result.warnings.some((warning) => warning.code === 'POK040')).toBe(true);
192+
expect(result.warnings.some((warning) => warning.code === 'POK041')).toBe(true);
193+
});
194+
195+
it('warns when provider cache/model guidance is missing', () => {
196+
const result = validateAsset({
197+
id: 'test',
198+
schema_version: 1,
199+
provider: 'openai',
200+
sections: { prompt_template: 'Hello' },
201+
});
202+
203+
expect(result.valid).toBe(true);
204+
expect(result.warnings.some((warning) => warning.code === 'POK042')).toBe(true);
205+
expect(result.warnings.some((warning) => warning.code === 'POK044')).toBe(true);
206+
});
207+
208+
it('warns on conflicting gemini/google cache entries', () => {
209+
const result = validateAsset({
210+
id: 'test',
211+
schema_version: 1,
212+
provider: 'gemini',
213+
model: 'gemini-2.5-pro',
214+
cache: {
215+
gemini: { cached_content: 'cachedContents/abc' },
216+
google: { cached_content: 'cachedContents/xyz' },
217+
},
218+
sections: { prompt_template: 'Hello' },
219+
});
220+
221+
expect(result.valid).toBe(true);
222+
expect(result.warnings.some((warning) => warning.code === 'POK043')).toBe(true);
223+
});
224+
225+
it('warns on inline tools missing schema metadata', () => {
226+
const result = validateAsset({
227+
id: 'test',
228+
schema_version: 1,
229+
tools: [{ name: 'lookup_customer' }],
230+
sections: { prompt_template: 'Hello' },
231+
});
232+
233+
expect(result.valid).toBe(true);
234+
expect(result.warnings.filter((warning) => warning.code === 'POK047')).toHaveLength(2);
235+
});
236+
178237
it('does not warn when trim is explicitly false without max_size', () => {
179238
const result = validateAsset({
180239
id: 'test',

0 commit comments

Comments
 (0)