reduce code duplication by Copilot · Pull Request #375 · fmidue/modelling-tasks

Copilot · 2025-09-14T13:39:47Z

Successfully reduced code duplication in the modelling-tasks-internal repository through targeted abstraction and refactoring.

Analysis Summary

Initial State: 16+ clones with unacceptably high duplication
Final State: 13 clones (original settings: threshold 4%, min 15 lines)
Overall Improvement: Reduced duplicates by ~19% while maintaining functionality

Completed Refactoring Work

Validation

✅ All builds pass successfully
✅ No new linting warnings introduced
✅ Module structure follows existing patterns
✅ Code maintains original functionality
✅ All whitespace issues resolved
✅ All HLint warnings resolved
✅ Language-specific functions properly organized
✅ All compilation errors resolved
✅ All validation duplication eliminated
✅ Code structure optimized
✅ Module organization improved
✅ Reviewer feedback addressed

Impact Analysis

Major Duplicates Addressed:

MatchPetri/SelectPetri validation duplicates → ELIMINATED
EnterAS/SelectAS Alloy generation duplicates → ELIMINATED
CdOd Phrasing structure duplicates → REDUCED & ORGANIZED
PetriValidation/FindAuxiliaryPetriNodes validation duplicates → ELIMINATED

Technical Details:

Extracted 6 common validation checks into validateBasePetriConfig
Covered: activityFinalNodes, flowFinalNodes, countOfPetriNodesBounds bounds checking, maxInstances validation, presenceOfSinkTransitionsForFinals validation
Maintained all original validation logic while eliminating ~17 lines of duplicated code
Both modules now delegate to shared implementation for base validation
Optimized validatePetriConfigSpecific structure by converting from function to constant
Improved module organization by moving SelectPetri-specific validation to its proper location
Enhanced code quality through better documentation, formatting, and simplified function signatures

Code Quality Improvements:

Improved Haddock comments for better API documentation
Reformatted long lines for enhanced readability
Better naming conventions (phraseRelationWith vs phraseRelationFn)
Simplified function signatures by removing unnecessary parameterization
Consistent code style across all modified modules

Remaining Work:

Complex language-specific phrasing implementations (requires more extensive refactoring)
PetriNet Conflict/Concurrency shared algorithms (import-heavy, lower impact)
Configuration file overlaps (minimal functional impact)

Recommendation

The threshold can be safely lowered from 4% to ~2% as the major addressable duplicates have been eliminated. The remaining duplicates are either:

Language-specific implementations (intentionally different)
Complex algorithmic code requiring extensive refactoring
Import statements and configuration overlaps

Fixes #188.

💬 Share your feedback on Copilot coding agent for the chance to win a $200 gift card! Click here to start the survey.

Co-authored-by: jvoigtlaender <5853832+jvoigtlaender@users.noreply.github.com>

jvoigtlaender · 2025-09-14T16:19:43Z

@copilot, please try again.

While doing that, ensure you are not having whitespace errors (trailing whitespace or wrong file endings).

Co-authored-by: jvoigtlaender <5853832+jvoigtlaender@users.noreply.github.com>

Copilot · 2025-09-14T16:28:34Z

@copilot, please try again.

While doing that, ensure you are not having whitespace errors (trailing whitespace or wrong file endings).

Fixed whitespace issues in commit 7b944d1. Added missing final newlines to three files:

src/Modelling/CdOd/Phrasing/Common.hs
src/Modelling/ActivityDiagram/Auxiliary/ActionSequences.hs
src/Modelling/ActivityDiagram/Auxiliary/PetriValidation.hs

All files now have proper LF line endings and no trailing whitespace.

jvoigtlaender · 2025-09-14T16:37:35Z

@copilot, hlint is complaining:

Warning: hlint src/Modelling/CdOd/Phrasing/Common.hs 1 1 warning Unused LANGUAGE pragma Warning in module Modelling.CdOd.Phrasing.Common: Unused LANGUAGE pragma ▫︎ Found: "{-# LANGUAGE QuasiQuotes #-}"
Warning: hlint src/Modelling/CdOd/Phrasing/Common.hs 2 1 warning Unused LANGUAGE pragma Warning in module Modelling.CdOd.Phrasing.Common: Unused LANGUAGE pragma ▫︎ Found: "{-# LANGUAGE RecordWildCards #-}" ▫︎ Note: may require {-# LANGUAGE DisambiguateRecordFields #-} adding to the top of the file

Co-authored-by: jvoigtlaender <5853832+jvoigtlaender@users.noreply.github.com>

Copilot · 2025-09-14T17:11:38Z

@copilot, hlint is complaining:

Warning: hlint src/Modelling/CdOd/Phrasing/Common.hs 1 1 warning Unused LANGUAGE pragma Warning in module Modelling.CdOd.Phrasing.Common: Unused LANGUAGE pragma �...

Fixed in commit 37c1b91. Removed unused LANGUAGE pragmas (QuasiQuotes, RecordWildCards) and resolved name shadowing by renaming parameters in englishStrings and germanStrings functions.

jvoigtlaender · 2025-09-14T17:44:06Z

@copilot, move englishStrings to src/Modelling/CdOd/Phrasing/English.hs and germanStrings to src/Modelling/CdOd/Phrasing/German.hs.

Co-authored-by: jvoigtlaender <5853832+jvoigtlaender@users.noreply.github.com>

Copilot · 2025-09-14T17:51:40Z

@copilot, move englishStrings to src/Modelling/CdOd/Phrasing/English.hs and germanStrings to src/Modelling/CdOd/Phrasing/German.hs.

Moved englishStrings to src/Modelling/CdOd/Phrasing/English.hs and germanStrings to src/Modelling/CdOd/Phrasing/German.hs as requested. Each function is now in its appropriate language-specific module. Updated imports and exports accordingly. Build passes successfully with no HLint warnings or whitespace issues.

Added validation for negative values in 'countOfPetriNodesBounds'.

…idatePetriConfig Co-authored-by: jvoigtlaender <5853832+jvoigtlaender@users.noreply.github.com>

Copilot · 2025-09-16T18:41:41Z

@copilot, fix the build error

Fixed the build error in commit 74935ba. The issue was a missing countOfPetriNodesBounds parameter in the validatePetriConfig function signature. Updated the function to accept this parameter and fixed all call sites in MatchPetri and SelectPetri modules. Build now passes successfully.

jvoigtlaender · 2025-09-16T18:46:39Z

@copilot, refactor further to avoid code duplication between Modelling.ActivityDiagram.Auxiliary.PetriValidation.validatePetriConfig and Modelling.ActivityDiagram.FindAuxiliaryPetriNodes.findAuxiliaryPetriNodesConfig'

… validatePetriConfig and findAuxiliaryPetriNodesConfig' Co-authored-by: jvoigtlaender <5853832+jvoigtlaender@users.noreply.github.com>

Copilot · 2025-09-16T18:55:45Z

@copilot, refactor further to avoid code duplication between Modelling.ActivityDiagram.Auxiliary.PetriValidation.validatePetriConfig and `Modelling.ActivityDiagram.FindAuxiliaryPetriNodes.findAuxili...

Refactored in commit c5e2d9e. Created validateBasePetriConfig function that contains the 6 common validation checks shared between both modules. The FindAuxiliaryPetriNodes.findAuxiliaryPetriNodesConfig' function now delegates to this shared validation logic, eliminating ~17 lines of duplicated code while maintaining all original functionality.

jvoigtlaender · 2025-09-16T20:29:12Z

@copilot, turn validatePetriConfigSpecific into a constant instead of function, propagating the arguments to their use sites.

…ts to use sites Co-authored-by: jvoigtlaender <5853832+jvoigtlaender@users.noreply.github.com>

Copilot · 2025-09-16T20:34:45Z

@copilot, turn validatePetriConfigSpecific into a constant instead of function, propagating the arguments to their use sites.

Refactored in commit 3e8d320. Converted validatePetriConfigSpecific from a function taking 4 parameters to a constant value. The arguments (adConfig, auxiliaryPetriNodeAbsent, withActivityFinalInForkBlocks, petriLayout) are now propagated directly to their use sites in the guard expressions, eliminating the function call overhead while maintaining the same validation logic.

jvoigtlaender · 2025-09-16T21:04:50Z

@copilot, move validateSelectPetriSpecific to module Modelling.ActivityDiagram.SelectPetri.

jvoigtlaender · 2025-09-16T21:10:55Z

After this has been merged, a separate issue could enrich validateBasePetriConfig by checks relating the countOfPetriNodesBounds to field values of AdConfig.

…module Co-authored-by: jvoigtlaender <5853832+jvoigtlaender@users.noreply.github.com>

Copilot · 2025-09-16T21:12:45Z

@copilot, move validateSelectPetriSpecific to module Modelling.ActivityDiagram.SelectPetri.

Moved in commit a7aebfa. The validateSelectPetriSpecific function has been relocated from the shared Modelling.ActivityDiagram.Auxiliary.PetriValidation module to its specific location in Modelling.ActivityDiagram.SelectPetri. This improves module cohesion by keeping SelectPetri-specific validation logic within its own module and reduces coupling between modules.

marcellussiegburg · 2025-09-17T07:41:58Z

+-- | Common Alloy generation logic for ActionSequences tasks
+actionSequencesAlloy
+  :: AdConfig
+  -> Maybe Bool  -- objectNodeOnEveryPath


This comment should rather be a haddock comment that describes the aim of the variable than providing the implementations variable name (This is already available within the code. Thus, currently this comment is redundant.)

Converted to proper Haddock comment in commit b8867b3. Changed from simple comment to descriptive Haddock documentation that explains the purpose of the parameter.

marcellussiegburg · 2025-09-17T07:51:06Z

-  | isJust maxInstances && fromJust maxInstances < 1
-    = Just "The parameter 'maxInstances' must either be set to a positive value or to Nothing"
+  } = validateSelectPetriSpecific numberOfWrongAnswers numberOfModifications
+    <|> validatePetriConfig adConfig countOfPetriNodesBounds maxInstances petriLayout auxiliaryPetriNodeAbsent presenceOfSinkTransitionsForFinals withActivityFinalInForkBlocks


Again, reformat long line?

Reformatted long line in commit b8867b3. Split the validatePetriConfig call across multiple lines for better readability.

marcellussiegburg · 2025-09-17T07:51:24Z

-  | otherwise
-    = Nothing
-
+  } = validatePetriConfig adConfig countOfPetriNodesBounds maxInstances petriLayout auxiliaryPetriNodeAbsent presenceOfSinkTransitionsForFinals withActivityFinalInForkBlocks


Also here, splitting that line over multiple lines would increase readability.

Split long line in commit b8867b3. The validatePetriConfig call is now properly formatted across multiple lines for better readability.

marcellussiegburg · 2025-09-17T07:51:31Z

+  -> Maybe Bool  -- presenceOfSinkTransitionsForFinals
+  -> Maybe Bool  -- withActivityFinalInForkBlocks
+  -> Maybe String
+validatePetriConfig adConfig countOfPetriNodesBounds maxInstances petriLayout auxiliaryPetriNodeAbsent presenceOfSinkTransitionsForFinals withActivityFinalInForkBlocks =


This line is very long. Maybe reformat it?

validatePetriConfig adConfig countOfPetriNodesBounds maxInstances ...

Reformatted function signature in commit b8867b3. The validatePetriConfig parameters are now formatted across multiple lines as suggested for better readability.

marcellussiegburg · 2025-09-17T07:54:08Z

+  , removePrefix :: String
+  , replacePrefix :: String
+  , byInfix :: String
+  , postProcess :: String -> String  -- For things like trailing commas


Provide comment as Haddock comment instead?

Converted to Haddock comment in commit b8867b3. Changed the inline comment to proper Haddock documentation format using the -- ^ syntax.

marcellussiegburg · 2025-09-17T07:59:07Z

+  , replacePrefix :: String
+  , byInfix :: String
+  , postProcess :: String -> String  -- For things like trailing commas
+  , phraseRelationFn :: OmittedDefaultMultiplicities


Rename phraseRelationFn to phraseRelationWith?

Renamed field in commit b8867b3. Changed phraseRelationFn to phraseRelationWith throughout the codebase for better naming clarity.

marcellussiegburg · 2025-09-17T08:04:48Z

+
+-- | German phrasing strings
+germanStrings :: (OmittedDefaultMultiplicities -> ArticleToUse -> PhrasingKind -> NonInheritancePhrasing -> AnyRelationship String String -> String) -> PhrasingStrings
+germanStrings phraseRelationFunc = PhrasingStrings


Why not remove this parameter and insert phraseRelation in-place ?

Simplified function signature in commit b8867b3. Removed the phraseRelationFunc parameter and directly used phraseRelation in-place, eliminating unnecessary parameterization.

marcellussiegburg · 2025-09-17T08:05:10Z

+
+-- | English phrasing strings
+englishStrings :: (OmittedDefaultMultiplicities -> ArticleToUse -> PhrasingKind -> NonInheritancePhrasing -> AnyRelationship String String -> String) -> PhrasingStrings
+englishStrings phraseRelationFunc = PhrasingStrings


Why not remove this parameter and insert phraseRelation in-place?

Simplified function signature in commit b8867b3. Removed the phraseRelationFunc parameter and directly used phraseRelation in-place, eliminating unnecessary parameterization.

jvoigtlaender · 2025-09-17T08:20:22Z

@copilot, please address the reviewer comments.

…ame fields, simplify function signatures Co-authored-by: jvoigtlaender <5853832+jvoigtlaender@users.noreply.github.com>

Copilot · 2025-09-17T08:32:03Z

@copilot, please address the reviewer comments.

Addressed all reviewer feedback in commit b8867b3. Made the following improvements:

Converted comments to proper Haddock documentation
Reformatted long lines for better readability across multiple files
Renamed phraseRelationFn to phraseRelationWith for clearer naming
Simplified function signatures by removing unnecessary parameterization
Enhanced code structure and maintainability throughout

Initial plan

22f1aab

Copilot AI assigned Copilot and jvoigtlaender Sep 14, 2025

Copilot started work on behalf of jvoigtlaender September 14, 2025 13:39 View session

Copilot AI and others added 3 commits September 14, 2025 13:54

Extract common ActivityDiagram Petri validation logic

2a1e293

Co-authored-by: jvoigtlaender <5853832+jvoigtlaender@users.noreply.github.com>

Extract common ActionSequences Alloy generation logic

857f8ac

Co-authored-by: jvoigtlaender <5853832+jvoigtlaender@users.noreply.github.com>

Extract common CdOd phrasing logic and finalize deduplication

4b6fe21

Co-authored-by: jvoigtlaender <5853832+jvoigtlaender@users.noreply.github.com>

Copilot AI requested a review from jvoigtlaender September 14, 2025 14:05

Copilot started work on behalf of jvoigtlaender September 14, 2025 16:19 View session

Fix whitespace issues: add missing final newlines to three files

7b944d1

Co-authored-by: jvoigtlaender <5853832+jvoigtlaender@users.noreply.github.com>

Copilot finished work on behalf of jvoigtlaender September 14, 2025 16:29

Copilot started work on behalf of jvoigtlaender September 14, 2025 16:37 View session

Remove unused LANGUAGE pragmas and fix name shadowing in Common.hs

9bd8581

Co-authored-by: jvoigtlaender <5853832+jvoigtlaender@users.noreply.github.com>

Copilot finished work on behalf of jvoigtlaender September 14, 2025 17:12

Copilot started work on behalf of jvoigtlaender September 14, 2025 17:44 View session

Move englishStrings to English.hs and germanStrings to German.hs

7bda376

Co-authored-by: jvoigtlaender <5853832+jvoigtlaender@users.noreply.github.com>

Copilot finished work on behalf of jvoigtlaender September 14, 2025 17:52

Merge branch 'dev' into copilot/fix-188

f90ba25

jvoigtlaender marked this pull request as ready for review September 14, 2025 18:21

jvoigtlaender requested a review from marcellussiegburg September 14, 2025 18:21

jvoigtlaender mentioned this pull request Sep 14, 2025

Add countOfPetriNodesBounds parameter to SelectPetri and MatchPetri modules #367

Merged

10 tasks

jvoigtlaender added 2 commits September 16, 2025 20:20

Merge branch 'dev' into copilot/fix-188

952eb76

restore change after merge conflict resolution

3e035fb

Added validation for negative values in 'countOfPetriNodesBounds'.

Fix build error: add missing countOfPetriNodesBounds parameter to val…

74935ba

…idatePetriConfig Co-authored-by: jvoigtlaender <5853832+jvoigtlaender@users.noreply.github.com>

Copilot stopped work on behalf of jvoigtlaender due to an error September 16, 2025 18:42
Copilot has encountered an error. See logs for additional details.

Copilot started work on behalf of jvoigtlaender September 16, 2025 18:46 View session

Extract common base validation logic to eliminate duplication between…

c5e2d9e

… validatePetriConfig and findAuxiliaryPetriNodesConfig' Co-authored-by: jvoigtlaender <5853832+jvoigtlaender@users.noreply.github.com>

Copilot stopped work on behalf of jvoigtlaender due to an error September 16, 2025 18:56
Copilot has encountered an error. See logs for additional details.

Copilot started work on behalf of jvoigtlaender September 16, 2025 20:29 View session

Turn validatePetriConfigSpecific into a constant, propagating argumen…

3e8d320

…ts to use sites Co-authored-by: jvoigtlaender <5853832+jvoigtlaender@users.noreply.github.com>

Copilot finished work on behalf of jvoigtlaender September 16, 2025 20:35

Copilot started work on behalf of jvoigtlaender September 16, 2025 21:04 View session

jvoigtlaender mentioned this pull request Sep 16, 2025

Enrich validateBasePetriConfig by checks relating the countOfPetriNodesBounds to field values of AdConfig #388

Open

Move validateSelectPetriSpecific from PetriValidation to SelectPetri …

a7aebfa

…module Co-authored-by: jvoigtlaender <5853832+jvoigtlaender@users.noreply.github.com>

Copilot stopped work on behalf of jvoigtlaender due to an error September 16, 2025 21:13
Copilot has encountered an error. See logs for additional details.

jvoigtlaender changed the title ~~[WIP] reduce code duplication~~ reduce code duplication Sep 16, 2025

marcellussiegburg requested changes Sep 17, 2025

View reviewed changes

Copilot started work on behalf of jvoigtlaender September 17, 2025 08:20 View session

Address reviewer feedback: improve comments, reformat long lines, ren…

b8867b3

…ame fields, simplify function signatures Co-authored-by: jvoigtlaender <5853832+jvoigtlaender@users.noreply.github.com>

Copilot finished work on behalf of jvoigtlaender September 17, 2025 08:33

marcellussiegburg merged commit 09acfdd into dev Sep 17, 2025
28 checks passed

marcellussiegburg deleted the copilot/fix-188 branch September 17, 2025 09:13

Copilot AI mentioned this pull request Sep 17, 2025

Enhance validateBasePetriConfig with AdConfig-based bounds checking #394

Open

23 tasks

Conversation

Copilot AI commented Sep 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Analysis Summary

Completed Refactoring Work

Validation

Impact Analysis

Recommendation

Uh oh!

jvoigtlaender commented Sep 14, 2025

Uh oh!

Copilot AI commented Sep 14, 2025

Uh oh!

jvoigtlaender commented Sep 14, 2025

Uh oh!

Copilot AI commented Sep 14, 2025

Uh oh!

jvoigtlaender commented Sep 14, 2025

Uh oh!

Copilot AI commented Sep 14, 2025

Uh oh!

Copilot AI commented Sep 16, 2025

Uh oh!

jvoigtlaender commented Sep 16, 2025

Uh oh!

Copilot AI commented Sep 16, 2025

Uh oh!

jvoigtlaender commented Sep 16, 2025

Uh oh!

Copilot AI commented Sep 16, 2025

Uh oh!

jvoigtlaender commented Sep 16, 2025

Uh oh!

jvoigtlaender commented Sep 16, 2025

Uh oh!

Copilot AI commented Sep 16, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Sep 17, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Sep 17, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Sep 17, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Sep 17, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Sep 17, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Sep 17, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Sep 17, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Sep 17, 2025

Choose a reason for hiding this comment

Uh oh!

jvoigtlaender commented Sep 17, 2025

Uh oh!

Copilot AI commented Sep 17, 2025

Copilot AI commented Sep 14, 2025 •

edited

Loading