[Suggestion] Automatically assign reviewers for manually graded assignments about quickfeed

[meling]

This is a good idea. Regarding the last point of not all "teachers" being reviewers is a crucial design decision that we should discuss. There are probably several design choices, but first, let's think through some usage scenarios:

• I imagine the lecturer/teacher wanting to outsource this reviewing task to the TAs.
• But, some teachers may also want to participate in reviewing, maybe with a lesser load.
• There could be cases where a TA initially signed up to review tasks but quit or got sick.
• I envision some courses that want to divide the reviewing workload differently than evenly among the reviewers.
• There could be cases where one reviewer only knows how to review lab1 and lab2, and other reviewers know the other labs.

These are the scenarios I could think of off the top of my head.

Here are some design ideas:

Design 1

• We could add a uint32 ReviewFraction field to the Enrollment struct that would be set ONLY if the UserStatus is TEACHER. The ReviewFraction for all reviewers should sum up to 100 (if working with percentages). If a teacher does not want to review anything, the enrollment's ReviewFraction would be set to 0.
• This design would only tackle the first usage scenarios.

Design 2

• To support a more advanced division of labor, we could add a map field to Enrollment that maps each assignment to a ReviewFraction:
  • map<uint64, uint32> ReviewFractions = X; // AssignmentID -> ReviewFraction
• This design is more flexible but will require more configuration.
• One idea is to build this map-based approach on the backend and then initialize it (fill the map) as if we only support the first design above. That way, configuring the system is easy but can scale to support more advanced configurations later.
• One way to configure this design is to use a JSON formatted file loaded from the tests repository. However, this is not easy since we would need to map from the name of an enrollment.
• Another approach is to add a front-end page to allocate fractions.

[JosteinLindhom]

I like the second design idea. I will come back with more thoughts soon.

Here are some thought on how to implement the automatic part:

Unless I'm thinking overly complicated, we will also need a mechanism to trigger reviewer assignment when the deadline expires. Would be cool to do so in a manner that could be used for other "services" as well, such as the StreamServices.

One way we could do this is to create a Service interface, e.g.

type Service[T any] interface {
    Start(fn func())
    Stop()
    Signal(signal T)
}

Where Start() would start some function in a goroutine. The goroutine could do any number of things, such as triggering an action if a deadline expires.

Stop() will cancel the goroutine running in the service

Signal(signal T) serves as an entrypoint to the goroutine. This could be used to signal to some service that circumstances have changed, e.g. a new assignment has been added.

We could have a BaseService that implements the interface. The struct can then be embedded in other structs.

type BaseService[T any] struct {
    done    chan struct{}
    channel T
}

func (b *BaseService[T]) Start(fn func()) {
    go func() {
	defer close(b.channel)
	fn()
    }
}

func (b *BaseService[T]) Stop() {
    s.done <- struct{}{}
}

func (b *BaseService[T]) Signal(signal T) {
    s.channel <- signal
}

func NewService[T any]() *Service[T] {
    return &Service[T]{
        done:    make(chan struct{}),
        channel: make(chan T),
    }
}

Small example of usage with BaseService embedded:

type CounterService struct {
    BaseService[int]
    count int
}

func (s *CounterService) run() {
    s.Start(func() {
	for val := range s.channel {
            s.count += val
	}
    })
}

func NewCounterService(init int) *CounterService {
    service := CounterService{
	BaseService: NewService[int](),
	count: init,
    }
    service.run()
    return &service
}

// Somewhere else
{
    ...
    qf.services.CounterService.Signal(5) // increments count by 5
    ...
}

Using this we could build services such as

type ReviewerService struct {
    BaseService[Assignment]
    db database.Database
    logger *zap.Logger
    // additional fields
    // ...
}

// Could possibly do something similar to this as well
type NotificationService struct {
    BaseService[Notification] // handle database updates, pass data to internalStream
    internalStream *StreamService[uint64, Notification] // send data to users
    db database.Database
}

type Services struct {
    ReviewerService Service[Assignment]
    NotificationService Service[Notification]
}

For a "ReviewerService" we would need to be able to

1. Detect new assignments
2. Detect if an assignment is modified, e.g. deadline is extended
3. Assign reviewers when a deadline expires

func NewReviewerService(db database.Database, logger *zap.Logger) *ReviewerService {
    s := &ReviewerService{
	BaseService: NewService[Assignment](),
	db: db,
	logger: logger,
    }
    s.run()
    return s
}

func (s *ReviewerService) run() {
    s.Start(func() {
	// fetch all assignment deadlines
	for _, deadline := range deadlines {
	    time.AfterFunc(time.Until(deadline)), func() {
	    // Assign reviewers
	    })
        }

        for {
	    select {
            case <-s.done:
                // cancel all timers
	        return
	    case assignment := <-s.channel:
	        if assignment.assignmentID < 1 {
	            continue
	        }
	        // cancel timer for assignment, if any exists
	        // lookup assignment and get deadline
	        // create new timer for deadline	
	    }
        }
    })
}

[meling]

Sorry I didn't look at this before. I think this design is interesting, especially from a modularity perspective. I suggest prototyping it with test cases in a separate package internal/review or something like that.

There are some things to think about though.

• How to change the "deadline" and reschedule the pending "reviewer assignment" job?
• Making sure that the "reviewer assignment" job isn't lost because of a reboot of the server.
• How to change the reviewer assignment after it has happened (correctly), because something happened in the real-world that requires that some assignments be reallocated to another reviewer.

These are things I could think of that would require some extra handling and thus should also be tested as part of the test cases.