fix(config): dual-emit for heartbeat null-mismatch and equal-values#25731
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 635442ee0e
ℹ️ 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".
| ctx.addIssue({ | ||
| code: z.ZodIssueCode.custom, | ||
| path: ["activeHoursEnd"], | ||
| message, |
There was a problem hiding this comment.
Preserve non-null hour when fixing null mismatch
Adding an issue on activeHoursEnd here causes validateWithSchema to delete both keys for any mixed-null input, so configs like { activeHoursStart: null, activeHoursEnd: 20 } are rewritten to defaults (8, 22) instead of retaining the valid explicit 20. This is a behavioral regression from the previous repair strategy and can silently widen heartbeat windows for users who only made one side null.
Useful? React with 👍 / 👎.
| ctx.addIssue({ | ||
| code: z.ZodIssueCode.custom, | ||
| path: ["activeHoursEnd"], | ||
| message: | ||
| "heartbeat.activeHoursStart and heartbeat.activeHoursEnd must not be equal (would create an empty window)", | ||
| message, |
There was a problem hiding this comment.
Keep one explicit hour when resolving equal-hour config
Emitting an additional issue on activeHoursStart makes cleanup drop both sides for every equal-hours input, so { activeHoursStart: 5, activeHoursEnd: 5 } now falls back to (8, 22) instead of preserving one explicit value (previously repaired to 5/22). The cascade you’re fixing only requires dual-emit for the default-collision case (22/22), so this broad change introduces avoidable data loss for other equal-hour typos.
Useful? React with 👍 / 👎.
|
Addressed in #26266 — heartbeat superRefine now emits only on the null side (for null-mismatch) and only on |
Addresses Codex P1 + Devin 🚩 feedback on #25685.
Converts both heartbeat superRefine issues (null-mismatch and equal-values) to dual-emit, mirroring the filing schema fix. Single-emit cascades to a full-config defaults reset when the retry hits a new mismatch, wiping unrelated keys like
maxTokens:{ start: null, end: 8 }→ strip start → default 8 → equal check fires → full-defaults fallback.{ start: 22, end: 22 }→ strip end → default 22 → equal check fires again → full-defaults fallback.Dual-emit strips both sides in one pass so both defaults restore cleanly.
Adds test coverage for the three cascade scenarios (
{ start: null, end: 8 },{ start: 22, end: null },{ start: 22, end: 22 }) confirmingmaxTokenssurvives revalidation.