ronantakizawa/codereview-bench huggingface dataset:
Repository: h2oai/wave
File: ui/src/image_annotator_polygon.ts (TypeScript)

Code before:
import { F, S, U } from './core'
import { DrawnPoint, DrawnShape, ImageAnnotatorPoint, ImageAnnotatorRect } from "./image_annotator"
import { ARC_RADIUS } from "./image_annotator_rect"

export class PolygonAnnotator {
  private currPolygonPoints: ImageAnnotatorPoint[] = []
  private boundaryRect: ImageAnnotatorRect | null = null
  private draggedPoint: DrawnPoint | null = null
  private draggedShape: DrawnShape | null = null


  constructor(private canvas: HTMLCanvasElement, private ctx: CanvasRenderingContext2D | null) { }

  resetDragging() {
    // Update the boundaries of the polygon when point dragging ends.
    const draggedPolygon = this.draggedShape?.shape.polygon
    if (this.draggedShape && this.draggedPoint && draggedPolygon) {
      const { x1, x2, y1, y2 } = getPolygonBoundaries(draggedPolygon.vertices)
      this.draggedShape.boundaryRect!.x1 = x1
      this.draggedShape.boundaryRect!.x2 = x2
      this.draggedShape.boundaryRect!.y1 = y1
      this.draggedShape.boundaryRect!.y2 = y2
    }

    this.draggedPoint = null
    this.draggedShape = null
  }

  cancelAnnotating() {
    this.currPolygonPoints = []
    this.boundaryRect = null
  }

  addCurrPolygonPoint({ x, y }: { x: U, y: U }) {
    this.currPolygonPoints.push({ x, y })

Review comment (suggestion):
Can be updated in the constructor when `ctx` is set. That way arcRadius can be computed just once and thus be made a constant instead.

Code after:
import { F, S, U } from './core'
import { DrawnPoint, DrawnShape, ImageAnnotatorPoint, ImageAnnotatorRect } from "./image_annotator"
import { ARC_RADIUS_DEFAULT } from "./image_annotator_rect"

export class PolygonAnnotator {
  private currPolygonPoints: ImageAnnotatorPoint[] = []
  private boundaryRect: ImageAnnotatorRect | null = null
  private draggedPoint: DrawnPoint | null = null
  private draggedShape: DrawnShape | null = null
  private getArcRadius = () => this.ctx ? ARC_RADIUS_DEFAULT / this.ctx.getTransform().a : ARC_RADIUS_DEFAULT


  constructor(private canvas: HTMLCanvasElement, private ctx: CanvasRenderingContext2D | null) { }

  resetDragging() {
    // Update the boundaries of the polygon when point dragging ends.
    const draggedPolygon = this.draggedShape?.shape.polygon
    if (this.draggedShape && this.draggedPoint && draggedPolygon) {
      const { x1, x2, y1, y2 } = getPolygonBoundaries(draggedPolygon.vertices)
      this.draggedShape.boundaryRect!.x1 = x1
      this.draggedShape.boundaryRect!.x2 = x2
      this.draggedShape.boundaryRect!.y1 = y1
      this.draggedShape.boundaryRect!.y2 = y2
    }

    this.draggedPoint = null
    this.draggedShape = null
  }

  cancelAnnotating() {
    this.currPolygonPoints = []
    this.boundaryRect = null
  }

  addCurrPolygonPoint({ x, y }: { x: U, y: U }) {
Repository: hashicorp/nomad
File: command/job_start_test.go (Go)

Code before:
// Copyright (c) HashiCorp, Inc.
// SPDX-License-Identifier: BUSL-1.1

package command

import (
	"encoding/json"
	"github.com/hashicorp/nomad/ci"
	"github.com/hashicorp/nomad/command/agent"
	"github.com/hashicorp/nomad/helper/pointer"
	"github.com/hashicorp/nomad/helper/uuid"
	"github.com/hashicorp/nomad/nomad/mock"
	"github.com/hashicorp/nomad/nomad/structs"
	"github.com/mitchellh/cli"
	"github.com/posener/complete"
	"github.com/shoenig/test/must"
	"os"
	"path/filepath"
	"testing"
)

var _ cli.Command = (*JobStartCommand)(nil)

func TestJobStartCommand_Fails(t *testing.T) {
	ci.Parallel(t)

	srv, _, addr := testServer(t, true, func(c *agent.Config) {
		c.DevMode = true
	})
	defer srv.Shutdown()

	ui := cli.NewMockUi()
	cmd := &JobStartCommand{Meta: Meta{Ui: ui}}

	// Fails on misuse
	code := cmd.Run([]string{"-bad", "-flag"})
	must.One(t, code)

	out := ui.ErrorWriter.String()
	must.StrContains(t, out, "flag provided but not defined: -bad")

	ui.ErrorWriter.Reset()

	// Fails on nonexistent job ID
	code = cmd.Run([]string{"-address=" + addr, "non-existent"})
	must.One(t, code)

Review comment (refactor):
Typically you'll see 2 styles of import ordering and grouping: (a) is grouped by stdlib, then internal, then third-party. (b) is stdlib, then everything else. Either style will pass the `goimports` check but Nomad uses style (b).

```suggestion
	"encoding/json"
	"os"
	"path/filepath"
	"testing"

	"github.com/hashicorp/nomad/ci"
	"github.com/hashicorp/nomad/command/agent"
	"github.com/hashicorp/nomad/helper/pointer"
	"github.com/hashicorp/nomad/helper/uuid"
	"github.com/hashicorp/nomad/nomad/mock"
	"github.com/hashicorp/nomad/nomad/structs"
	"github.com/mitchellh/cli"
	"github.com/posener/complete"
	"github.com/shoenig/test/must"
```



Code after:
// Copyright (c) HashiCorp, Inc.
// SPDX-License-Identifier: BUSL-1.1

package command

import (
	"testing"

	"github.com/hashicorp/cli"
	"github.com/hashicorp/nomad/api"
	"github.com/hashicorp/nomad/ci"
	"github.com/hashicorp/nomad/command/agent"
	"github.com/hashicorp/nomad/helper/uuid"
	"github.com/hashicorp/nomad/nomad/mock"
	"github.com/hashicorp/nomad/nomad/structs"
	"github.com/posener/complete"
	"github.com/shoenig/test/must"
)

var _ cli.Command = (*JobStartCommand)(nil)

func TestStartCommand(t *testing.T) {
	ci.Parallel(t)

	srv, _, addr := testServer(t, true, func(c *agent.Config) {
		c.DevMode = true
	})
	defer srv.Shutdown()

	ui := cli.NewMockUi()
	cmd := &JobStartCommand{
		Meta: Meta{
			Ui:          ui,
			flagAddress: addr,
		},
	}

	t.Run("succeeds when starting a stopped job", func(t *testing.T) {
		job := testJob(uuid.Generate())

		client, err := cmd.Meta.Client()
		must.NoError(t, err)

		_, _, err = client.Jobs().Register(job, nil)
		must.NoError(t, err)
Repository: hashicorp/nomad
File: command/job_start.go (Go)

Code before:
// Copyright (c) HashiCorp, Inc.
// SPDX-License-Identifier: BUSL-1.1

package command

import (
	"fmt"
	"github.com/hashicorp/nomad/api"
	"github.com/hashicorp/nomad/api/contexts"
	"github.com/posener/complete"
	"os"
	"strings"
	"sync"
)

type JobStartCommand struct {
	Meta
}

func (c *JobStartCommand) Help() string {
	helpText := `
Usage: nomad job start [options] <job>
Alias: nomad start

  Start an existing stopped job. This command is used to start a previously stopped job's
  most recent running version up. Upon successful start, an interactive
  monitor session will start to display log lines as the job starts up its
  allocations based on its most recent running version. It is safe to exit the monitor
  early using ctrl+c.

  When ACLs are enabled, this command requires a token with the 'submit-job'
  and 'read-job' capabilities for the job's namespace. The 'list-jobs'
  capability is required to run the command with job prefixes instead of exact
  job IDs.


Review comment (suggestion):
```suggestion
	"fmt"
	"os"
	"strings"
	"sync"
	
	"github.com/hashicorp/nomad/api"
	"github.com/hashicorp/nomad/api/contexts"
	"github.com/posener/complete"
```

Code after:
// Copyright (c) HashiCorp, Inc.
// SPDX-License-Identifier: BUSL-1.1

package command

import (
	"fmt"
	"strings"
	"time"

	"github.com/hashicorp/nomad/api/contexts"
	"github.com/posener/complete"
)

type JobStartCommand struct {
	Meta
}

func (c *JobStartCommand) Help() string {
	helpText := `
Usage: nomad job start [options] <job>
Alias: nomad start

 Starts a stopped job. The job must be currently registered. Nomad will create a
 new version of the job based on its most recent version. Upon successful start,
 Nomad will enter an interactive monitor session. This is useful to watch
 Nomad's internals make scheduling decisions and place the submitted work onto
 nodes. The monitor will end once job placement is done. It is safe to exit the
 monitor early using ctrl+c.

 When ACLs are enabled, this command requires a token with the 'submit-job'
 capability for the job's namespace.

General Options:
Repository: hashicorp/nomad
File: command/job_start.go (Go)

Code before:
// SPDX-License-Identifier: BUSL-1.1

package command

import (
	"fmt"
	"github.com/hashicorp/nomad/api"
	"github.com/hashicorp/nomad/api/contexts"
	"github.com/posener/complete"
	"os"
	"strings"
	"sync"
)

type JobStartCommand struct {
	Meta
}

func (c *JobStartCommand) Help() string {
	helpText := `
Usage: nomad job start [options] <job>
Alias: nomad start

  Start an existing stopped job. This command is used to start a previously stopped job's
  most recent running version up. Upon successful start, an interactive
  monitor session will start to display log lines as the job starts up its
  allocations based on its most recent running version. It is safe to exit the monitor
  early using ctrl+c.

  When ACLs are enabled, this command requires a token with the 'submit-job'
  and 'read-job' capabilities for the job's namespace. The 'list-jobs'
  capability is required to run the command with job prefixes instead of exact
  job IDs.


General Options:

  ` + generalOptionsUsage(usageOptsDefault) + `

Start Options:

  -detach
    Return immediately instead of entering monitor mode. After the
    job start command is submitted, a new evaluation ID is printed to the
    screen, which can be used to examine the evaluation using the eval-status
    command.

  -consul-token
   The Consul token used to verify that the caller has access to the Service
   Identity policies associated in the targeted version of the job.


Review comment (suggestion):
```suggestion
  most recent running version. Upon successful start, an interactive
  monitor session will start to display log lines as the job starts its
```

Code after:
// SPDX-License-Identifier: BUSL-1.1

package command

import (
	"fmt"
	"strings"
	"time"

	"github.com/hashicorp/nomad/api/contexts"
	"github.com/posener/complete"
)

type JobStartCommand struct {
	Meta
}

func (c *JobStartCommand) Help() string {
	helpText := `
Usage: nomad job start [options] <job>
Alias: nomad start

 Starts a stopped job. The job must be currently registered. Nomad will create a
 new version of the job based on its most recent version. Upon successful start,
 Nomad will enter an interactive monitor session. This is useful to watch
 Nomad's internals make scheduling decisions and place the submitted work onto
 nodes. The monitor will end once job placement is done. It is safe to exit the
 monitor early using ctrl+c.

 When ACLs are enabled, this command requires a token with the 'submit-job'
 capability for the job's namespace.

General Options:

 ` + generalOptionsUsage(usageOptsDefault) + `

Start Options:

 -detach
   Return immediately instead of entering monitor mode. After the
   job start command is submitted, a new evaluation ID is printed to the
   screen, which can be used to examine the evaluation using the eval-status
   command.

 -verbose
   Display full information.
`
	return strings.TrimSpace(helpText)
}

func (c *JobStartCommand) Synopsis() string {
Repository: hashicorp/nomad
File: command/job_start.go (Go)

Code before:
	"sync"
)

type JobStartCommand struct {
	Meta
}

func (c *JobStartCommand) Help() string {
	helpText := `
Usage: nomad job start [options] <job>
Alias: nomad start

  Start an existing stopped job. This command is used to start a previously stopped job's
  most recent running version up. Upon successful start, an interactive
  monitor session will start to display log lines as the job starts up its
  allocations based on its most recent running version. It is safe to exit the monitor
  early using ctrl+c.

  When ACLs are enabled, this command requires a token with the 'submit-job'
  and 'read-job' capabilities for the job's namespace. The 'list-jobs'
  capability is required to run the command with job prefixes instead of exact
  job IDs.


General Options:

  ` + generalOptionsUsage(usageOptsDefault) + `

Start Options:

  -detach
    Return immediately instead of entering monitor mode. After the
    job start command is submitted, a new evaluation ID is printed to the
    screen, which can be used to examine the evaluation using the eval-status
    command.

  -consul-token
   The Consul token used to verify that the caller has access to the Service
   Identity policies associated in the targeted version of the job.

  -vault-token
   The Vault token used to verify that the caller has access to the Vault
   policies in the targeted version of the job.

  -verbose
    Display full information.
`
	return strings.TrimSpace(helpText)
}

func (c *JobStartCommand) Synopsis() string {

Review comment (suggestion):
Looks like we usually only have one blank line here:

```suggestion

```

Code after:
)

type JobStartCommand struct {
	Meta
}

func (c *JobStartCommand) Help() string {
	helpText := `
Usage: nomad job start [options] <job>
Alias: nomad start

 Starts a stopped job. The job must be currently registered. Nomad will create a
 new version of the job based on its most recent version. Upon successful start,
 Nomad will enter an interactive monitor session. This is useful to watch
 Nomad's internals make scheduling decisions and place the submitted work onto
 nodes. The monitor will end once job placement is done. It is safe to exit the
 monitor early using ctrl+c.

 When ACLs are enabled, this command requires a token with the 'submit-job'
 capability for the job's namespace.

General Options:

 ` + generalOptionsUsage(usageOptsDefault) + `

Start Options:

 -detach
   Return immediately instead of entering monitor mode. After the
   job start command is submitted, a new evaluation ID is printed to the
   screen, which can be used to examine the evaluation using the eval-status
   command.

 -verbose
   Display full information.
`
	return strings.TrimSpace(helpText)
}

func (c *JobStartCommand) Synopsis() string {
	return "Start a stopped job"
}

func (c *JobStartCommand) AutocompleteFlags() complete.Flags {
	return mergeAutocompleteFlags(c.Meta.AutocompleteFlags(FlagSetClient),
		complete.Flags{
			"-detach":  complete.PredictNothing,
			"-verbose": complete.PredictNothing,
		})
}
Repository: hashicorp/nomad
File: command/job_start.go (Go)

Code before:

	// Check that we got at least one job
	args = flags.Args()
	if len(args) < 1 {
		c.Ui.Error("This command takes at least one argument: <job>")
		c.Ui.Error(commandErrorText(c))
		return 1
	}

	var jobIDs []string
	for _, jobID := range flags.Args() {
		jobIDs = append(jobIDs, strings.TrimSpace(jobID))
	}

	// Get the HTTP client
	client, err := c.Meta.Client()
	if err != nil {
		c.Ui.Error(fmt.Sprintf("Error initializing client: %s", err))
		return 1
	}

	statusCh := make(chan int, len(jobIDs))
	var wg sync.WaitGroup

	for _, jobID := range jobIDs {
		jobID := jobID

		wg.Add(1)
		go func() {
			defer wg.Done()

			// Truncate the id unless full length is requested
			length := shortId
			if verbose {
				length = fullId
			}

			// Check if the job exists and has been stopped (status is dead)
			jobId, namespace, err := c.JobIDByPrefix(client, jobID, nil)
			if err != nil {
				c.Ui.Error(err.Error())
				statusCh <- 1
				return
			}
			job, err := c.JobByPrefix(client, jobId, nil)
			if err != nil {
				c.Ui.Error(err.Error())
				statusCh <- 1
				return
			}
			if *job.Status != "dead" {

Review comment (bug):
`jobID := range jobIDs` is the ideomatic way to write this, but in the goroutine we shadow this variable. That makes it easy to confuse whether we're dealing with the prefix we got from the user or the actual job ID we got from our `JobIDByPrefix` call. That results in a small bug later down.

Also, as of Go 1.22 we no longer need to rebind the loopvar, so we're free of having to remember! I always forget :blush:  so good that you caught it, but we can drop it now. See https://go.dev/wiki/LoopvarExperiment

```suggestion
	for _, jobIDPrefix := range jobIDs {
```

Code after:
		c.Ui.Error(fmt.Sprintf("Error initializing client: %s", err))
		return 1
	}

	// Truncate the id unless full length is requested
	length := shortId
	if verbose {
		length = fullId
	}

	job, err := c.JobByPrefix(client, jobIDPrefix, nil)
	if err != nil {
		c.Ui.Error(err.Error())
		return 1
	}

	var jobName string

	if job.Name != nil {
		jobName = *job.Name
	}

	if job.Stop == nil || !*job.Stop {
		c.Ui.Error(fmt.Sprintf("Job %q has not been stopped", jobName))
		return 1
	}

	// register the job in a not stopped state
	*job.Stop = false

	resp, _, err := client.Jobs().Register(job, nil)

	// Check if the job is periodic or is a parameterized job
	periodic := job.IsPeriodic()
	paramjob := job.IsParameterized()
	multiregion := job.IsMultiregion()
	if detach || periodic || paramjob || multiregion {
		c.Ui.Output("Job start successful")
		if periodic && !paramjob {
			loc, err := job.Periodic.GetLocation()
			if err == nil {
				now := time.Now().In(loc)
				next, err := job.Periodic.Next(now)
				if err != nil {
					c.Ui.Error(fmt.Sprintf("Error determining next launch time: %v", err))
				} else {
					c.Ui.Output(fmt.Sprintf("Approximate next launch time: %s (%s from now)",
						formatTime(next), formatTimeDifference(now, next, time.Second)))
				}
			}
		} else if !paramjob {
Repository: hashicorp/nomad
File: command/job_start.go (Go)

Code before:
		return 1
	}

	statusCh := make(chan int, len(jobIDs))
	var wg sync.WaitGroup

	for _, jobID := range jobIDs {
		jobID := jobID

		wg.Add(1)
		go func() {
			defer wg.Done()

			// Truncate the id unless full length is requested
			length := shortId
			if verbose {
				length = fullId
			}

			// Check if the job exists and has been stopped (status is dead)
			jobId, namespace, err := c.JobIDByPrefix(client, jobID, nil)
			if err != nil {
				c.Ui.Error(err.Error())
				statusCh <- 1
				return
			}
			job, err := c.JobByPrefix(client, jobId, nil)
			if err != nil {
				c.Ui.Error(err.Error())
				statusCh <- 1
				return
			}
			if *job.Status != "dead" {
				c.Ui.Info(fmt.Sprintf("Job  %v has not been stopped and has the following status: %v", *job.Name, *job.Status))
				statusCh <- 0
				return

			}

			// Get all versions associated to current job
			q := &api.QueryOptions{Namespace: namespace}

			versions, _, _, err := client.Jobs().Versions(jobID, true, q)
			if err != nil {
				c.Ui.Error(fmt.Sprintf("Error retrieving job versions: %s", err))
				statusCh <- 1
			}

			// Find the most recent running version for this job
			var chosenIndex uint64
			versionAvailable := false

Review comment (suggestion):
This is where we originally shadowed `jobID`, but as it turns out we're not _actually_ shadowing it because we changed the capitalization. This makes it really easy to miss that we're using the prefix when we make the `Jobs().Versions` call later.

But it turns out that we already need to call `JobByPrefix` below, which takes the prefix, so we can just delete this whole block.

```suggestion
```

Code after:
	if job.Name != nil {
		jobName = *job.Name
	}

	if job.Stop == nil || !*job.Stop {
		c.Ui.Error(fmt.Sprintf("Job %q has not been stopped", jobName))
		return 1
	}

	// register the job in a not stopped state
	*job.Stop = false

	resp, _, err := client.Jobs().Register(job, nil)

	// Check if the job is periodic or is a parameterized job
	periodic := job.IsPeriodic()
	paramjob := job.IsParameterized()
	multiregion := job.IsMultiregion()
	if detach || periodic || paramjob || multiregion {
		c.Ui.Output("Job start successful")
		if periodic && !paramjob {
			loc, err := job.Periodic.GetLocation()
			if err == nil {
				now := time.Now().In(loc)
				next, err := job.Periodic.Next(now)
				if err != nil {
					c.Ui.Error(fmt.Sprintf("Error determining next launch time: %v", err))
				} else {
					c.Ui.Output(fmt.Sprintf("Approximate next launch time: %s (%s from now)",
						formatTime(next), formatTimeDifference(now, next, time.Second)))
				}
			}
		} else if !paramjob {
			c.Ui.Output("Evaluation ID: " + resp.EvalID)
		}

		return 0
	}

	mon := newMonitor(c.Ui, client, length)
	return mon.monitor(resp.EvalID)
}
Repository: hashicorp/nomad
File: command/job_start.go (Go)

Code before:
	}

	statusCh := make(chan int, len(jobIDs))
	var wg sync.WaitGroup

	for _, jobID := range jobIDs {
		jobID := jobID

		wg.Add(1)
		go func() {
			defer wg.Done()

			// Truncate the id unless full length is requested
			length := shortId
			if verbose {
				length = fullId
			}

			// Check if the job exists and has been stopped (status is dead)
			jobId, namespace, err := c.JobIDByPrefix(client, jobID, nil)
			if err != nil {
				c.Ui.Error(err.Error())
				statusCh <- 1
				return
			}
			job, err := c.JobByPrefix(client, jobId, nil)
			if err != nil {
				c.Ui.Error(err.Error())
				statusCh <- 1
				return
			}
			if *job.Status != "dead" {
				c.Ui.Info(fmt.Sprintf("Job  %v has not been stopped and has the following status: %v", *job.Name, *job.Status))
				statusCh <- 0
				return

			}

			// Get all versions associated to current job
			q := &api.QueryOptions{Namespace: namespace}

			versions, _, _, err := client.Jobs().Versions(jobID, true, q)
			if err != nil {
				c.Ui.Error(fmt.Sprintf("Error retrieving job versions: %s", err))
				statusCh <- 1
			}

			// Find the most recent running version for this job
			var chosenIndex uint64
			versionAvailable := false
			for i := len(versions) - 1; i >= 0; i-- {

Review comment (suggestion):
```suggestion
			job, err := c.JobByPrefix(client, jobIDPrefix, nil)
```

Code after:
		jobName = *job.Name
	}

	if job.Stop == nil || !*job.Stop {
		c.Ui.Error(fmt.Sprintf("Job %q has not been stopped", jobName))
		return 1
	}

	// register the job in a not stopped state
	*job.Stop = false

	resp, _, err := client.Jobs().Register(job, nil)

	// Check if the job is periodic or is a parameterized job
	periodic := job.IsPeriodic()
	paramjob := job.IsParameterized()
	multiregion := job.IsMultiregion()
	if detach || periodic || paramjob || multiregion {
		c.Ui.Output("Job start successful")
		if periodic && !paramjob {
			loc, err := job.Periodic.GetLocation()
			if err == nil {
				now := time.Now().In(loc)
				next, err := job.Periodic.Next(now)
				if err != nil {
					c.Ui.Error(fmt.Sprintf("Error determining next launch time: %v", err))
				} else {
					c.Ui.Output(fmt.Sprintf("Approximate next launch time: %s (%s from now)",
						formatTime(next), formatTimeDifference(now, next, time.Second)))
				}
			}
		} else if !paramjob {
			c.Ui.Output("Evaluation ID: " + resp.EvalID)
		}

		return 0
	}

	mon := newMonitor(c.Ui, client, length)
	return mon.monitor(resp.EvalID)
}