Skip to content

Conversation

@RickyVishwakarma
Copy link

Problem:
Several users (including me) hit avoidable failures when running Ralph
due to small PRD mistakes (duplicate IDs, missing fields, invalid status).

Change:
This PR adds a lightweight PRD validation script that fails fast before
the agent loop starts, saving time and API usage. I also clarified the
PRD format in the README so expectations are explicit.

Why small + separate:
I intentionally kept this out of the main loop to avoid changing Ralph’s
runtime behavior and keep it optional.

@snarktank
Copy link
Owner

Thanks @RickyVishwakarma. You need to integrate it into the rest of the repo though. How does the agent know to run this check?

Copy link

@manishin2050 manishin2050 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice job

Have you tested..?

@RickyVishwakarma
Copy link
Author

Thanks @RickyVishwakarma. You need to integrate it into the rest of the repo though. How does the agent know to run this check?

That’s a fair point, thanks for calling it out.

I intentionally left it unhooked at first to avoid changing the agent’s execution flow without alignment. My assumption was that validation could either be a lightweight pre-flight step or remain manual depending on preference.

I’m happy to wire it in, but before doing that — do you prefer this to run automatically at the start of ralph.sh, or should it stay as an explicit/optional step?

@RickyVishwakarma
Copy link
Author

Nice job

Have you tested..?

Thanks!

Yes — I tested the script locally against prd.json.example and also tried a few failure cases (duplicate IDs and missing required fields) to confirm it fails fast with clear errors.

Happy to add or adjust tests if you’d like this covered differently.

@snarktank
Copy link
Owner

@RickyVishwakarma if you can please add a screencast of you running this and demonstrating it, then I'll have a look.

@RickyVishwakarma
Copy link
Author

@RickyVishwakarma if you can please add a screencast of you running this and demonstrating it, then I'll have a look.

yes I can provide you screencast here

10-33-05.mp4

@snarktank
Copy link
Owner

Screencast looks good, thanks!

Please wire it into ralph.sh to run automatically at the start. If validation fails, exit before the loop begins. That's the whole point of fail-fast.

Once that's in, I'll merge.

@RickyVishwakarma
Copy link
Author

Screencast looks good, thanks!

Please wire it into ralph.sh to run automatically at the start. If validation fails, exit before the loop begins. That's the whole point of fail-fast.

Once that's in, I'll merge.

Done — wired PRD validation into ralph.sh so it runs automatically at startup and exits before the loop on failure.

Thanks!

@tianyili-roller
Copy link

@RickyVishwakarma There's a potential issue.

The current implementation does not support boolean value, like passes

 VALUE=$(jq -r ".userStories[$i].$FIELD // empty" "$FILE")                                                        

The // empty operator in jq treats false as falsy, so:

  • passes: true → returns "true" → ✓ passes validation
  • passes: false → returns empty → ✗ incorrectly flagged as missing
  • field missing → returns empty → ✗ correctly flagged

Suggestion

use has() instead

 EXISTS=$(jq ".userStories[$i] | has(\"$FIELD\")" "$FILE")

@snarktank
Copy link
Owner

@RickyVishwakarma @tianyili-roller raised a valid point about the boolean handling bug. Can you fix that before I merge? Using has() as suggested should work.

@RickyVishwakarma
Copy link
Author

@RickyVishwakarma There's a potential issue.

The current implementation does not support boolean value, like passes

 VALUE=$(jq -r ".userStories[$i].$FIELD // empty" "$FILE")                                                        

The // empty operator in jq treats false as falsy, so:

  • passes: true → returns "true" → ✓ passes validation
  • passes: false → returns empty → ✗ incorrectly flagged as missing
  • field missing → returns empty → ✗ correctly flagged

Suggestion

use has() instead

 EXISTS=$(jq ".userStories[$i] | has(\"$FIELD\")" "$FILE")

Good catch — you’re right.

Using // empty conflates falsy values with missing fields. I’ve updated the validator to use has() so boolean fields like passes: false are handled correctly while still catching missing fields.

Thanks for pointing that out.

@RickyVishwakarma
Copy link
Author

@RickyVishwakarma There's a potential issue.

The current implementation does not support boolean value, like passes

 VALUE=$(jq -r ".userStories[$i].$FIELD // empty" "$FILE")                                                        

The // empty operator in jq treats false as falsy, so:

  • passes: true → returns "true" → ✓ passes validation
  • passes: false → returns empty → ✗ incorrectly flagged as missing
  • field missing → returns empty → ✗ correctly flagged

Suggestion

use has() instead

 EXISTS=$(jq ".userStories[$i] | has(\"$FIELD\")" "$FILE")

Good catch — you’re right.

Using // empty conflates falsy values with missing fields. I’ve updated the validator to use has() so boolean fields like passes: false are handled correctly while still catching missing fields.

Thanks for pointing that out.

I’ve updated the validator to use jq has() so existence checks don’t conflate falsy values with missing fields.

@RickyVishwakarma
Copy link
Author

@RickyVishwakarma @tianyili-roller raised a valid point about the boolean handling bug. Can you fix that before I merge? Using has() as suggested should work.

I’ve updated the validator to use jq has() for field existence checks so boolean or falsy values are no longer treated as missing. This correctly handles cases like passes: false while still catching genuinely missing fields.

The fix is pushed now. Thanks!

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.

4 participants