fix(triage): degrade unusable supplied artifacts to null instead of failing - #25
Merged
Merged
Conversation
…ailing The triage decision schema rejected a supplied_spec/supplied_plan whose source did not carry its required field (a "body" source without content, or a "path" source without path), failing the entire triage run and forcing a manual shopfloor:failed:triage label removal to retry. Replace the two hard refinements with a transform that degrades an unusable artifact to null, so triage completes and the stage runs normally. The downstream apply step already no-ops on a missing path/content, so the seeding optimization is the only thing skipped. The refinements never constrained the model (they do not survive zodToJsonSchema); they only fired as a post-hoc parse failure. The transform is equally transparent to schema generation for both the Claude and Codex adapters. The prompt is tightened to state that the required field must be populated so the agent stops emitting the bad shape.
Contributor
There was a problem hiding this comment.
Shopfloor agent review: clean across 4/4 reviewers.
- PR #25 follows all documented conventions in CLAUDE.md (committed dist bundle, correct prompt/test file placement, no forbidden patterns or formatting-rule violations); no compliance issues found.
- The .refine→.transform swap correctly degrades unusable supplied artifacts to null; downstream apply already guards on path/content presence, the type export is preserved, and the new tests genuinely exercise the transform via the mock agent's schema.parse.
- The diff is a Zod validation refactor (refine→transform) for triage artifacts plus prompt/test updates; it introduces no exploitable security patterns.
- The triage decision refactor (.refine → .transform), prompt update, and regression tests are clean with no maintainability smells worth flagging.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
A real triage run failed hard on konfirmity/app#562:
The agent judged the issue body to be a spec (artifact-detection rule #3) and set
supplied_spec.source = "body"but did not extractcontent. The schema's hard.refinethen rejected the entire triage decision, escalating a recoverable gap into a fatal failure that requires manually removing theshopfloor:failed:triagelabel to retry.Two things make this a bug rather than correct strictness:
apply.tsonly seeds a spec PR whensource === "body" && content)..refinenever constrained the model — refinements do not survivezodToJsonSchema, so they only fired as a post-hoc.parse()throw.Fix
decision.ts— replace the two.refines with a.transformthat degrades an unusable artifact (bodywithoutcontent, orpathwithoutpath) tonull. Well-formed artifacts still seed PRs; malformed ones fall through to the normal stage flow. Verified that the transform is transparent tozodToJsonSchemafor both the Claude (default) and Codex (openAi) targets — the generated output contract is unchanged.prompt.system.md— state explicitly thatcontentis required forsource:"body"andpathforsource:"path", and that a missing required field is discarded, so the agent stops emitting the bad shape.test/stages/triage.test.ts— regression tests: body-without-content and path-without-path both degrade tonulland route throughneeds-specrather than throwing.dist/index.cjs— rebuilt (committed bundle).Design note
Chose silent degradation over backfilling
contentfrom the issue body. #562 has no explicit## Shopfloor Specmarker (judgment-based detection), so re-running the spec stage is the designed default for a large issue; backfilling the whole body would add magic with edge-case risk.Verification
pnpm typecheckcleanpnpm buildclean (bundle regenerated)pnpm test-> 286 passed (incl. 2 new degradation tests)biome checkclean on changed files