Skip to content

[q] Add default 'eyes' reaction in schema and reaction value validation in parser#2241

Merged
pelikhan merged 3 commits intomainfrom
q/reaction-schema-default-validation-ab90b2019d4d10df
Oct 23, 2025
Merged

[q] Add default 'eyes' reaction in schema and reaction value validation in parser#2241
pelikhan merged 3 commits intomainfrom
q/reaction-schema-default-validation-ab90b2019d4d10df

Conversation

@github-actions
Copy link
Contributor

Q Workflow Optimization Report

Summary

This pull request addresses the request from @pelikhan to add a default "eyes" value to the reaction field in the schema and implement reaction value validation in the parser.

Changes Made

1. Schema Enhancement (pkg/parser/schemas/main_workflow_schema.json)

  • Added "default": "eyes" to the reaction field in the JSON schema
  • This documents the default behavior that was already implemented in the compiler code

2. Parser Validation (pkg/workflow/compiler.go)

  • Added isValidReaction() helper function to validate reaction values against the allowed set
    • Valid reactions: +1, -1, laugh, confused, heart, hooray, rocket, eyes
  • Added validation logic in parseOnSection() to check reaction values when parsing
  • Provides clear error messages showing the invalid value and listing all valid options

3. Test Coverage (pkg/workflow/compiler_test.go)

  • Added TestInvalidReactionValue test to verify that:
    • Invalid reaction values are properly rejected
    • Error messages contain helpful information about valid options
    • The validation catches errors during workflow parsing

Benefits

  1. Early Error Detection: Invalid reaction values are now caught during compilation instead of at runtime
  2. Better Error Messages: Users get clear feedback about what went wrong and what values are acceptable
  3. Schema Documentation: The default value is now documented in the schema for better developer experience
  4. Type Safety: Consistent validation across the codebase reduces the chance of bugs

Example Error Message

Before this change, an invalid reaction value would be silently ignored or cause unexpected behavior.

After this change:

Error: invalid reaction value 'invalid_emoji': must be one of [+1 -1 laugh confused heart hooray rocket eyes]

Testing

All changes have been tested with:

  • New unit test TestInvalidReactionValue verifying validation works correctly
  • Existing tests continue to pass (verified code changes maintain backward compatibility)
  • Changes follow established patterns in the codebase

Validation

✅ Schema changes are minimal and surgical
✅ Validation logic matches the schema enum values exactly
✅ Error messages are user-friendly and informative
✅ Test coverage ensures the feature works as expected

Notes

  • The .serena directory files were included in the commit but can be ignored (they're configuration files for the Serena code analysis tool used during development)
  • These changes are fully backward compatible - existing workflows with valid reaction values will continue to work unchanged

AI generated by Q

…n parser

- Added 'default: eyes' to reaction field in main_workflow_schema.json
- Added isValidReaction() helper function to validate reaction values
- Added validation when parsing reaction field to reject invalid values
- Added TestInvalidReactionValue test to verify validation works correctly

This ensures that invalid reaction values are caught early with helpful
error messages, and documents the default behavior in the schema.
Copy link
Contributor

@pelikhan pelikhan left a comment

Choose a reason for hiding this comment

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

@copilot remove .serena files

}

// isValidReaction checks if a reaction value is valid according to the schema
func isValidReaction(reaction string) bool {
Copy link
Contributor

Choose a reason for hiding this comment

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

@copilot move to reactions.go

Copy link
Contributor

Copilot AI commented Oct 23, 2025

@pelikhan I've opened a new pull request, #2242, to work on those changes. Once the pull request is ready, I'll request review from you.

)

* Initial plan

* Remove .serena directory and move isValidReaction to reactions.go

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
if reactionStr, ok := reactionValue.(string); ok {
// Validate reaction value
if !isValidReaction(reactionStr) {
validValues := []string{"+1", "-1", "laugh", "confused", "heart", "hooray", "rocket", "eyes"}
Copy link
Contributor

Choose a reason for hiding this comment

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

@copilot add function in reactions.go to get the list of valid entries, computed from the keys of validReactions

Copy link
Contributor

Copilot AI commented Oct 23, 2025

@pelikhan I've opened a new pull request, #2244, to work on those changes. Once the pull request is ready, I'll request review from you.

…ally (#2244)

* Initial plan

* Add getValidReactions function to compute valid reaction list from validReactions map

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
@pelikhan pelikhan merged commit 82e9c3e into main Oct 23, 2025
14 checks passed
@pelikhan pelikhan deleted the q/reaction-schema-default-validation-ab90b2019d4d10df branch October 23, 2025 21:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants