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

Code before:
  /** The text displayed below the bar. */
  caption?: S
  /** The progress, between 0.0 and 1.0, or -1 (default) if indeterminate. */
  value?: F
  /** The width of the separator, e.g. '100px'. Defaults to '100%'. */
  width?: S
  /** True if the component should be visible. Defaults to True. */
  visible?: B
  /** An optional tooltip message displayed when a user clicks the help icon to the right of the component. */
  tooltip?: S
  /** An identifying name for this component. */
  name?: S
  /** The type of progress bar to be displayed */
  type?: S
}

export const
  XProgress = ({ model }: { model: Progress }) => {
    const
      {type, label, caption = 'Please wait...', value, name } = model
    return (
      <div data-test={name}>
        {type === 'spinner' ?
        <div>
        <Fluent.Spinner label={label} size={Fluent.SpinnerSize.medium} labelPosition='top'/>
        </div>
        :<Fluent.ProgressIndicator label={label} description={caption} percentComplete={value} />}
      </div>
    )
  }

Review comment (suggestion):
Is the wrapper `div` needed? Also, let's position the label to the right - `label` prop docstring will need update 

Code after:
  /** The text displayed above the bar or right to the spinner. */
  label: S
  /** The text displayed below the bar or spinner. */
  caption?: S
  /** The progress, between 0.0 and 1.0, or -1 (default) if indeterminate. */
  value?: F
  /** The width of the separator, e.g. '100px'. Defaults to '100%'. */
  width?: S
  /** True if the component should be visible. Defaults to True. */
  visible?: B
  /** An optional tooltip message displayed when a user clicks the help icon to the right of the component. */
  tooltip?: S
  /** An identifying name for this component. */
  name?: S
  /** The type of progress bar to be displayed. One of 'bar', 'spinner'. Defaults to 'bar'. */
  type?: 'bar' | 'spinner'
}

export const
  XProgress = ({ model }: { model: Progress }) => {
    const { type = 'bar', label, caption, value, name } = model
    return (
      <div data-test={name}>
        {type === 'spinner'
          ? <Fluent.Spinner
            styles={{ root: { justifyContent: 'auto' } }}
            label={caption || label}
            size={Fluent.SpinnerSize.medium}
            labelPosition={caption ? 'bottom' : 'right'}
          />
          : <Fluent.ProgressIndicator label={label} description={caption || 'Please wait...'} percentComplete={value} />}
      </div>
    )
  }
Repository: h2oai/wave
File: ui/src/progress.tsx (TypeScript)

Code before:
*/

const
  css = stylesheet({
    leftAlign: {
      position:'absolute',
      marginLeft:0
    },
  })

export interface Progress {
  /** The text displayed above the bar. */
  label: S
  /** The text displayed below the bar. */
  caption?: S
  /** The progress, between 0.0 and 1.0, or -1 (default) if indeterminate. */
  value?: F
  /** The width of the separator, e.g. '100px'. Defaults to '100%'. */
  width?: S
  /** True if the component should be visible. Defaults to True. */
  visible?: B
  /** An optional tooltip message displayed when a user clicks the help icon to the right of the component. */
  tooltip?: S
  /** An identifying name for this component. */
  name?: S
  /** The type of progress bar to be displayed */
  type?: 'bar'|'spinner'
}

export const
  XProgress = ({ model }: { model: Progress }) => {
    const
      {type, label, caption = 'Please wait...', value, name } = model
    return (
      <div data-test={name}>
        {type === 'spinner' ?
          <Fluent.Spinner className={css.leftAlign}  label={label} size={Fluent.SpinnerSize.medium} labelPosition='right'/>
        :<Fluent.ProgressIndicator label={label} description={caption} percentComplete={value} />}
      </div>
    )
  }

Review comment (suggestion):
```suggestion
  /** The type of progress bar to be displayed. One of 'bar', 'spinner'. */
```

Code after:

export interface Progress {
  /** The text displayed above the bar or right to the spinner. */
  label: S
  /** The text displayed below the bar or spinner. */
  caption?: S
  /** The progress, between 0.0 and 1.0, or -1 (default) if indeterminate. */
  value?: F
  /** The width of the separator, e.g. '100px'. Defaults to '100%'. */
  width?: S
  /** True if the component should be visible. Defaults to True. */
  visible?: B
  /** An optional tooltip message displayed when a user clicks the help icon to the right of the component. */
  tooltip?: S
  /** An identifying name for this component. */
  name?: S
  /** The type of progress bar to be displayed. One of 'bar', 'spinner'. Defaults to 'bar'. */
  type?: 'bar' | 'spinner'
}

export const
  XProgress = ({ model }: { model: Progress }) => {
    const { type = 'bar', label, caption, value, name } = model
    return (
      <div data-test={name}>
        {type === 'spinner'
          ? <Fluent.Spinner
            styles={{ root: { justifyContent: 'auto' } }}
            label={caption || label}
            size={Fluent.SpinnerSize.medium}
            labelPosition={caption ? 'bottom' : 'right'}
          />
          : <Fluent.ProgressIndicator label={label} description={caption || 'Please wait...'} percentComplete={value} />}
      </div>
    )
  }
Repository: h2oai/wave
File: ui/src/progress.tsx (TypeScript)

Code before:
    },
  })

export interface Progress {
  /** The text displayed above the bar. */
  label: S
  /** The text displayed below the bar. */
  caption?: S
  /** The progress, between 0.0 and 1.0, or -1 (default) if indeterminate. */
  value?: F
  /** The width of the separator, e.g. '100px'. Defaults to '100%'. */
  width?: S
  /** True if the component should be visible. Defaults to True. */
  visible?: B
  /** An optional tooltip message displayed when a user clicks the help icon to the right of the component. */
  tooltip?: S
  /** An identifying name for this component. */
  name?: S
  /** The type of progress bar to be displayed */
  type?: 'bar'|'spinner'
}

export const
  XProgress = ({ model }: { model: Progress }) => {
    const
      {type, label, caption = 'Please wait...', value, name } = model
    return (
      <div data-test={name}>
        {type === 'spinner' ?
          <Fluent.Spinner className={css.leftAlign}  label={label} size={Fluent.SpinnerSize.medium} labelPosition='right'/>
        :<Fluent.ProgressIndicator label={label} description={caption} percentComplete={value} />}
      </div>
    )
  }

Review comment (suggestion):
Let's add a default as well, just to be sure.

```suggestion
      {type = 'bar', label, caption = 'Please wait...', value, name } = model
```

Code after:
  value?: F
  /** The width of the separator, e.g. '100px'. Defaults to '100%'. */
  width?: S
  /** True if the component should be visible. Defaults to True. */
  visible?: B
  /** An optional tooltip message displayed when a user clicks the help icon to the right of the component. */
  tooltip?: S
  /** An identifying name for this component. */
  name?: S
  /** The type of progress bar to be displayed. One of 'bar', 'spinner'. Defaults to 'bar'. */
  type?: 'bar' | 'spinner'
}

export const
  XProgress = ({ model }: { model: Progress }) => {
    const { type = 'bar', label, caption, value, name } = model
    return (
      <div data-test={name}>
        {type === 'spinner'
          ? <Fluent.Spinner
            styles={{ root: { justifyContent: 'auto' } }}
            label={caption || label}
            size={Fluent.SpinnerSize.medium}
            labelPosition={caption ? 'bottom' : 'right'}
          />
          : <Fluent.ProgressIndicator label={label} description={caption || 'Please wait...'} percentComplete={value} />}
      </div>
    )
  }
Repository: h2oai/wave
File: py/examples/table_events_select.py (Python)

Code before:
# Table / Events / Select
# Register the `select` #event to emit Wave event on each #table row selection.
# #table #events #select
# ---
from h2o_wave import main, app, Q, ui

@app('/demo')
async def serve(q: Q):
    if q.events.table:
        if q.events.table.select:
            q.page['description'].content = f'{q.events.table.select}'
    else:
        q.page['table'] = ui.form_card(box='1 1 3 4', items=[
                ui.table(
                    name='table',
                    columns=[ui.table_column(name='text', label='Table select event')],
                    rows=[
                        ui.table_row(name='row1', cells=['Row 1']),
                        ui.table_row(name='row2', cells=['Row 2']),
                        ui.table_row(name='row3', cells=['Row 3'])
                    ],
                    multiple=True,
                    events=['select']
                )
            ])
        q.page['description'] = ui.markdown_card(box='4 1 3 4', title='Selected rows', content='No rows are selected yet.')
    await q.page.save()

Review comment (suggestion):
nit

```suggestion
    if q.events.table and q.events.table.select:
```

Code after:
# Table / Events / Select
# Register the `select` #event to emit Wave event on each #table row selection.
# #table #events #select
# ---
from h2o_wave import main, app, Q, ui

@app('/demo')
async def serve(q: Q):
    if q.events.table and q.events.table.select:
            q.page['description'].content = f'{q.events.table.select}'
    else:
        q.page['table'] = ui.form_card(box='1 1 3 4', items=[
                ui.table(
                    name='table',
                    columns=[ui.table_column(name='text', label='Table select event')],
                    rows=[
                        ui.table_row(name='row1', cells=['Row 1']),
                        ui.table_row(name='row2', cells=['Row 2']),
                        ui.table_row(name='row3', cells=['Row 3'])
                    ],
                    multiple=True,
                    events=['select']
                )
            ])
        q.page['description'] = ui.markdown_card(box='4 1 3 4', title='Selected rows', content='No rows are selected yet.')
    await q.page.save()
Repository: h2oai/wave
File: ui/src/form.tsx (TypeScript)

Code before:
  text_annotator?: TextAnnotator
  /** Image annotator. */
  image_annotator?: ImageAnnotator
  /** Facepile. */
  facepile?: Facepile
  /** Copyable text. */
  copyable_text?: CopyableText
  /** Menu. */
  menu?: Menu
  /** Tags. */
  tags?: Tags
  /** Time picker. */
  time_picker?: TimePicker
}

/** Create an inline (horizontal) list of components. */
interface Inline {
  /** The components laid out inline. */
  items: Component[]
  /** Specifies how to lay out the individual components. Defaults to 'start'. */
  justify?: 'start' | 'end' | 'center' | 'between' | 'around'
  /** Specifies how the individual components are aligned on the vertical axis. Defaults to 'center'. */
  align?: 'start' | 'end' | 'center' | 'baseline'
  /** Whether to display the components inset from the parent form, with a contrasting background. */
  inset?: B
  /** Custom height  in `px`, `vh` or `rem`. Use '1' to fill the height of the remaining space. E.g. '100vh', '300px' or '1'. */
  height?: S
}

/** Create a form. */
interface State {
  /** The components in this form. */
  items: Packed<Component[]>
  /** The title for this card. */
  title?: S
}

const
  defaults: Partial<State> = { items: [] },
  css = stylesheet({
    card: {
      display: 'flex',
      padding: 15,
    },
    vertical: {
      display: 'flex',
      flexDirection: 'column',
      flexGrow: 1,
      $nest: {
        '> [data-visible="true"] ~ div': {
          marginTop: 10

Review comment (suggestion):
```suggestion
  /** Height of the inline container. Accepts any valid CSS unit e.g. '100vh', '300px'. Use '1' to fill the remaining card space. */
```

Code after:
  text_annotator?: TextAnnotator
  /** Image annotator. */
  image_annotator?: ImageAnnotator
  /** Facepile. */
  facepile?: Facepile
  /** Copyable text. */
  copyable_text?: CopyableText
  /** Menu. */
  menu?: Menu
  /** Tags. */
  tags?: Tags
  /** Time picker. */
  time_picker?: TimePicker
}

/** Create an inline (horizontal) list of components. */
interface Inline {
  /** The components laid out inline. */
  items: Component[]
  /** Specifies how to lay out the individual components. Defaults to 'start'. */
  justify?: 'start' | 'end' | 'center' | 'between' | 'around'
  /** Specifies how the individual components are aligned on the vertical axis. Defaults to 'center'. */
  align?: 'start' | 'end' | 'center' | 'baseline'
  /** Whether to display the components inset from the parent form, with a contrasting background. */
  inset?: B
  /** Height of the inline container. Accepts any valid CSS unit e.g. '100vh', '300px'. Use '1' to fill the remaining card space. */
  height?: S
}

/** Create a form. */
interface State {
  /** The components in this form. */
  items: Packed<Component[]>
  /** The title for this card. */
  title?: S
}

const
  defaults: Partial<State> = { items: [] },
  css = stylesheet({
    card: {
      display: 'flex',
      padding: 15,
    },
    vertical: {
      display: 'flex',
      flexDirection: 'column',
      flexGrow: 1,
      $nest: {
        '> [data-visible="true"] ~ div': {
          marginTop: 10
Repository: h2oai/wave
File: card.go (Go)

Code before:
	// get cursor at key k
	get(k string) (Cur, bool)
	// overwrite all records
	put(v interface{})
	// set record at key k
	set(k string, v interface{})
	// dump contents
	dump() BufD
}

func loadBuf(ns *Namespace, b BufD) Buf {
	if b.C != nil {
		return loadCycBuf(ns, b.C)
	}
	if b.F != nil {
		return loadFixBuf(ns, b.F)
	}
	if b.M != nil {
		return loadMapBuf(ns, b.M)
	}
	return nil
}

// Card represents an item on a Page, and holds attributes and data for rendering views.
type Card struct {
	data             map[string]interface{}
	nameComponentMap map[string]interface{} // Cache for cards with items, secondary_items or buttons.
}

const dataPrefix = "~"

func loadCard(ns *Namespace, c CardD) *Card {
	card := &Card{make(map[string]interface{}), nil}
	ks := make([]string, 1) // to avoid allocation during card.set() below
	for k, v := range c.D {
		if len(k) > 0 && strings.HasPrefix(k, dataPrefix) {
			if f, ok := v.(float64); ok {
				i := int(f)
				if i >= 0 && i < len(c.B) {
					k, v = strings.TrimPrefix(k, dataPrefix), loadBuf(ns, c.B[i])
				}
			}
		}
		if k == "items" || k == "secondary_items" || k == "buttons" {
			if card.nameComponentMap == nil {
				card.nameComponentMap = make(map[string]interface{})
			}
			fillNameComponentMap(card.nameComponentMap, v)
		}
		ks[0] = k
		card.set(ks, v)

Review comment (suggestion):
Since the `go 1.18`, we suppose to use `any` instead of `interface{}` i.e.
```go
data map[string]any
```

But after a glance into `go.mod` `wave` is still using `1.15` so nvm :).

Code after:
	// get cursor at key k
	get(k string) (Cur, bool)
	// overwrite all records
	put(v any)
	// set record at key k
	set(k string, v any)
	// dump contents
	dump() BufD
}

func loadBuf(ns *Namespace, b BufD) Buf {
	if b.C != nil {
		return loadCycBuf(ns, b.C)
	}
	if b.F != nil {
		return loadFixBuf(ns, b.F)
	}
	if b.M != nil {
		return loadMapBuf(ns, b.M)
	}
	return nil
}

// Card represents an item on a Page, and holds attributes and data for rendering views.
type Card struct {
	data             map[string]any
	nameComponentMap map[string]any // Cache for cards with items, secondary_items or buttons.
}

const dataPrefix = "~"

func loadCard(ns *Namespace, c CardD) *Card {
	card := &Card{
		data:             make(map[string]any),
		nameComponentMap: nil,
	}
	ks := make([]string, 1) // to avoid allocation during card.set() below
	for k, v := range c.D {
		if len(k) > 0 && strings.HasPrefix(k, dataPrefix) {
			if f, ok := v.(float64); ok {
				i := int(f)
				if i >= 0 && i < len(c.B) {
					k, v = strings.TrimPrefix(k, dataPrefix), loadBuf(ns, c.B[i])
				}
			}
		}
		if k == "items" || k == "secondary_items" || k == "buttons" {
			if card.nameComponentMap == nil {
				card.nameComponentMap = make(map[string]any)
			}
			fillNameComponentMap(card.nameComponentMap, v)
Repository: h2oai/wave
File: card.go (Go)

Code before:
	}
	return CardD{data, bufs}
}

func deepClone(ix interface{}) interface{} {
	switch x := ix.(type) {
	case map[string]interface{}:
		m := make(map[string]interface{})
		for k, v := range x {
			m[k] = deepClone(v)
		}
		return m
	case []interface{}:
		s := make([]interface{}, len(x))
		for i, v := range x {
			s[i] = deepClone(v)
		}
		return s
	}
	return ix
}

func fillNameComponentMap(m map[string]interface{}, items interface{}) {
	for _, v := range items.([]interface{}) {
		// This map always has a single key - wrapper so this is O(1) not O(n).
		for _, v := range v.(map[string]interface{}) {
			component := v.(map[string]interface{})
			if name, ok := component["name"]; ok {
				m[name.(string)] = v
			}
			if items, ok := component["items"]; ok {
				fillNameComponentMap(m, items)
			}
			if secondaryItems, ok := component["secondary_items"]; ok {
				fillNameComponentMap(m, secondaryItems)
			}
			if buttons, ok := component["buttons"]; ok {
				fillNameComponentMap(m, buttons)
			}
		}
	}
}


Review comment (performance):
I'd fix the variable clashing here. I think it's simpler to reason about the code without clashing. You can do e.g.:

```go
for _, item := range items.([]interface{}) {
    // This map always has a single key - wrapper so this is O(1) not O(n).
    for _, v := range item.(map[string]interface{}) {
```

You can use `i` as well but it's more idiomatic to use `i` for index. Or you can rename `items interface{}` to `entries interface{}` and use `e` as a pointer.

Code after:
		} else {
			data[k] = deepClone(iv)
		}
	}
	return CardD{data, bufs}
}

func deepClone(ix any) any {
	switch x := ix.(type) {
	case map[string]any:
		m := make(map[string]any)
		for k, v := range x {
			m[k] = deepClone(v)
		}
		return m
	case []any:
		s := make([]any, len(x))
		for i, v := range x {
			s[i] = deepClone(v)
		}
		return s
	}
	return ix
}

func fillNameComponentMap(m map[string]any, wrappedItems any) {
	for _, wrappedItem := range wrappedItems.([]any) {
		// This map always has a single key - wrapper so this is O(1) not O(n).
		for _, item := range wrappedItem.(map[string]any) {
			component := item.(map[string]any)
			if name, ok := component["name"]; ok {
				if n, ok := name.(string); ok {
					m[n] = item
				}
			}
			if items, ok := component["items"]; ok {
				fillNameComponentMap(m, items)
			}
			if secondaryItems, ok := component["secondary_items"]; ok {
				fillNameComponentMap(m, secondaryItems)
			}
			if buttons, ok := component["buttons"]; ok {
				fillNameComponentMap(m, buttons)
			}
		}
	}
}
Repository: h2oai/wave
File: card.go (Go)

Code before:

func deepClone(ix interface{}) interface{} {
	switch x := ix.(type) {
	case map[string]interface{}:
		m := make(map[string]interface{})
		for k, v := range x {
			m[k] = deepClone(v)
		}
		return m
	case []interface{}:
		s := make([]interface{}, len(x))
		for i, v := range x {
			s[i] = deepClone(v)
		}
		return s
	}
	return ix
}

func fillNameComponentMap(m map[string]interface{}, items interface{}) {
	for _, v := range items.([]interface{}) {
		// This map always has a single key - wrapper so this is O(1) not O(n).
		for _, v := range v.(map[string]interface{}) {
			component := v.(map[string]interface{})
			if name, ok := component["name"]; ok {
				m[name.(string)] = v
			}
			if items, ok := component["items"]; ok {
				fillNameComponentMap(m, items)
			}
			if secondaryItems, ok := component["secondary_items"]; ok {
				fillNameComponentMap(m, secondaryItems)
			}
			if buttons, ok := component["buttons"]; ok {
				fillNameComponentMap(m, buttons)
			}
		}
	}
}


Review comment (suggestion):
To be safe here, I'd do a `type switch` or just check the correctness of type with `if`:
```go
if n, ok := name.(string); ok {
    m[n] = v
}
```

Code after:
	}
	return CardD{data, bufs}
}

func deepClone(ix any) any {
	switch x := ix.(type) {
	case map[string]any:
		m := make(map[string]any)
		for k, v := range x {
			m[k] = deepClone(v)
		}
		return m
	case []any:
		s := make([]any, len(x))
		for i, v := range x {
			s[i] = deepClone(v)
		}
		return s
	}
	return ix
}

func fillNameComponentMap(m map[string]any, wrappedItems any) {
	for _, wrappedItem := range wrappedItems.([]any) {
		// This map always has a single key - wrapper so this is O(1) not O(n).
		for _, item := range wrappedItem.(map[string]any) {
			component := item.(map[string]any)
			if name, ok := component["name"]; ok {
				if n, ok := name.(string); ok {
					m[n] = item
				}
			}
			if items, ok := component["items"]; ok {
				fillNameComponentMap(m, items)
			}
			if secondaryItems, ok := component["secondary_items"]; ok {
				fillNameComponentMap(m, secondaryItems)
			}
			if buttons, ok := component["buttons"]; ok {
				fillNameComponentMap(m, buttons)
			}
		}
	}
}
Repository: h2oai/wave
File: card.go (Go)

Code before:
			data[k] = deepClone(iv)
		}
	}
	return CardD{data, bufs}
}

func deepClone(ix interface{}) interface{} {
	switch x := ix.(type) {
	case map[string]interface{}:
		m := make(map[string]interface{})
		for k, v := range x {
			m[k] = deepClone(v)
		}
		return m
	case []interface{}:
		s := make([]interface{}, len(x))
		for i, v := range x {
			s[i] = deepClone(v)
		}
		return s
	}
	return ix
}

func fillNameComponentMap(m map[string]interface{}, items interface{}) {
	for _, v := range items.([]interface{}) {
		// This map always has a single key - wrapper so this is O(1) not O(n).
		for _, v := range v.(map[string]interface{}) {
			component := v.(map[string]interface{})
			if name, ok := component["name"]; ok {
				m[name.(string)] = v
			}
			if items, ok := component["items"]; ok {
				fillNameComponentMap(m, items)
			}
			if secondaryItems, ok := component["secondary_items"]; ok {
				fillNameComponentMap(m, secondaryItems)
			}
			if buttons, ok := component["buttons"]; ok {
				fillNameComponentMap(m, buttons)
			}
		}
	}
}


Review comment (suggestion):
Is it always expected to get an array? This will panic if not. It would be safer to expect `items []interface{}` but I assume the variable passed inside (`c.D`) is of type `interface{}`.

Code after:
			data[dataPrefix+k] = len(bufs)
			bufs = append(bufs, v.dump())
		} else {
			data[k] = deepClone(iv)
		}
	}
	return CardD{data, bufs}
}

func deepClone(ix any) any {
	switch x := ix.(type) {
	case map[string]any:
		m := make(map[string]any)
		for k, v := range x {
			m[k] = deepClone(v)
		}
		return m
	case []any:
		s := make([]any, len(x))
		for i, v := range x {
			s[i] = deepClone(v)
		}
		return s
	}
	return ix
}

func fillNameComponentMap(m map[string]any, wrappedItems any) {
	for _, wrappedItem := range wrappedItems.([]any) {
		// This map always has a single key - wrapper so this is O(1) not O(n).
		for _, item := range wrappedItem.(map[string]any) {
			component := item.(map[string]any)
			if name, ok := component["name"]; ok {
				if n, ok := name.(string); ok {
					m[n] = item
				}
			}
			if items, ok := component["items"]; ok {
				fillNameComponentMap(m, items)
			}
			if secondaryItems, ok := component["secondary_items"]; ok {
				fillNameComponentMap(m, secondaryItems)
			}
			if buttons, ok := component["buttons"]; ok {
				fillNameComponentMap(m, buttons)
			}
		}
	}
}