Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Oct 23, 2025

This PR addresses feedback from #2241 by refactoring the reaction validation logic to eliminate code duplication.

Changes

Previously, the valid reaction values were defined in two places:

  1. Inside the isValidReaction() function as a local map
  2. Hardcoded as a string slice in the error message in compiler.go

This PR refactors the code to:

  • Extract validReactions map to package level in reactions.go
  • Add getValidReactions() function that dynamically computes the list of valid reactions from the map keys
  • Update compiler.go to use getValidReactions() instead of the hardcoded list

Benefits

  • Single source of truth: Valid reactions are now defined in one place
  • Maintainability: Adding or removing reaction types only requires updating the map
  • Consistency: Error messages automatically reflect the current set of valid reactions

Example

// Before: validReactions was a local variable
func isValidReaction(reaction string) bool {
    validReactions := map[string]bool{...}
    return validReactions[reaction]
}

// After: validReactions is package-level, with a getter function
var validReactions = map[string]bool{...}

func getValidReactions() []string {
    reactions := make([]string, 0, len(validReactions))
    for reaction := range validReactions {
        reactions = append(reactions, reaction)
    }
    return reactions
}

Fixes feedback in #2241


✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

…lidReactions map

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copilot AI changed the title [WIP] Add function for valid reaction entries in schema Add getValidReactions function to compute valid reaction list dynamically Oct 23, 2025
Copilot AI requested a review from pelikhan October 23, 2025 20:56
@pelikhan pelikhan marked this pull request as ready for review October 23, 2025 21:00
@pelikhan pelikhan merged commit 9cceb0c into q/reaction-schema-default-validation-ab90b2019d4d10df Oct 23, 2025
@pelikhan pelikhan deleted the copilot/stack-pr-2241 branch October 23, 2025 21:00
pelikhan added a commit that referenced this pull request Oct 23, 2025
…on in parser (#2241)

* Add default 'eyes' reaction in schema and reaction value validation in 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.

* Remove .serena files and refactor isValidReaction to reactions.go (#2242)

* 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>

* Add getValidReactions function to compute valid reaction list dynamically (#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>

---------

Co-authored-by: Q <github-actions[bot]@users.noreply.github.com>
Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
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.

2 participants