ronantakizawa/codereview-bench huggingface dataset:
Repository: novuhq/novu
File: apps/api/src/app/widgets/dtos/feeds-response.dto.ts (TypeScript)

Code before:
  cta: MessageCTA;

  @ApiProperty({
    description: 'Current status of the notification.',
    enum: ['sent', 'error', 'warning'],
    example: 'sent',
    type: String,
  })
  status: 'sent' | 'error' | 'warning';

  @ApiProperty({
    description: 'The payload that was used to send the notification trigger.',
    type: 'object',
    additionalProperties: true,
    required: false,
    example: { key: 'value' },
  })
  payload?: Record<string, unknown>;

  @ApiPropertyOptional({
    description: 'The data sent with the notification.',
    type: 'object',
    nullable: true,
    example: { key: 'value' },
  })
  data?: Record<string, unknown> | null;

  @ApiProperty({
    description: 'Provider-specific overrides used when triggering the notification.',
    type: 'object',
    additionalProperties: true,
    required: false,
    example: { overrideKey: 'overrideValue' },
  })
  overrides?: Record<string, unknown>;

  @ApiPropertyOptional({
    description: 'Tags associated with the workflow that triggered the notification.',
    type: [String],
    nullable: true,
    example: ['tag1', 'tag2'],
  })
  tags?: string[] | null;
}

export class FeedResponseDto {
  @ApiPropertyOptional({
    description: 'Total number of notifications available.',
    example: 5,
    type: Number,
  })

Review comment (suggestion):
```suggestion
  @ApiPropertyOptional({
    description: 'The data sent with the notification.',
    type: 'object',
    nullable: true,
    example: { key: 'value' },
    additionalProperties: true
  })
```

this is needed for proper SDK type generation

Code after:
  cta: MessageCTA;

  @ApiProperty({
    description: 'Current status of the notification.',
    enum: ['sent', 'error', 'warning'],
    example: 'sent',
    type: String,
  })
  status: 'sent' | 'error' | 'warning';

  @ApiProperty({
    description: 'The payload that was used to send the notification trigger.',
    type: 'object',
    additionalProperties: true,
    required: false,
    example: { key: 'value' },
  })
  payload?: Record<string, unknown>;

  @ApiPropertyOptional({
    description: 'The data sent with the notification.',
    type: 'object',
    nullable: true,
    example: { key: 'value' },
    additionalProperties: true,
  })
  data?: Record<string, unknown> | null;

  @ApiProperty({
    description: 'Provider-specific overrides used when triggering the notification.',
    type: 'object',
    additionalProperties: true,
    required: false,
    example: { overrideKey: 'overrideValue' },
  })
  overrides?: Record<string, unknown>;

  @ApiPropertyOptional({
    description: 'Tags associated with the workflow that triggered the notification.',
    type: [String],
    nullable: true,
    example: ['tag1', 'tag2'],
  })
  tags?: string[] | null;
}

export class FeedResponseDto {
  @ApiPropertyOptional({
    description: 'Total number of notifications available.',
    example: 5,
    type: Number,
Repository: novuhq/novu
File: apps/dashboard/src/components/workflow-editor/steps/chat/chat-editor.tsx (TypeScript)

Code before:
import { FeatureFlagsKeysEnum, type UiSchema } from '@novu/shared';

import { getComponentByType } from '@/components/workflow-editor/steps/component-utils';
import { TabsSection } from '@/components/workflow-editor/steps/tabs-section';
import { useFeatureFlag } from '../../../../hooks/use-feature-flag';
import { cn } from '../../../../utils/ui';

type ChatEditorProps = { uiSchema: UiSchema };

export const ChatEditor = (props: ChatEditorProps) => {
  const isV2TemplateEditorEnabled = useFeatureFlag(FeatureFlagsKeysEnum.IS_V2_TEMPLATE_EDITOR_ENABLED);
  const { uiSchema } = props;
  const { body } = uiSchema?.properties ?? {};

  return (
    <div className="flex h-full flex-col">
      <TabsSection className={cn(isV2TemplateEditorEnabled ? 'p-0 pb-3' : '')}>
        {!isV2TemplateEditorEnabled && (
          <div className="flex items-center gap-2.5 text-sm font-medium">Chat template editor</div>
        )}
        <div
          className={cn(
            'rounded-12 flex flex-col gap-2 border border-neutral-100 p-2',
            isV2TemplateEditorEnabled ? 'bg-bg-weak' : ''
          )}
        >
          {getComponentByType({ component: body.component })}
        </div>
      </TabsSection>
    </div>
  );
};


Review comment (suggestion):
every editor is now deciding what to render in the prod env; for example in the email editor we render: subject, editor type, layout components in the disabled state;

Code after:
import { EnvironmentTypeEnum, FeatureFlagsKeysEnum, type UiSchema } from '@novu/shared';

import { getComponentByType } from '@/components/workflow-editor/steps/component-utils';
import { TabsSection } from '@/components/workflow-editor/steps/tabs-section';
import { useFeatureFlag } from '../../../../hooks/use-feature-flag';
import { cn } from '../../../../utils/ui';
import { useEnvironment } from '@/context/environment/hooks';
import { StepEditorUnavailable } from '../step-editor-unavailable';

type ChatEditorProps = { uiSchema: UiSchema };

export const ChatEditor = (props: ChatEditorProps) => {
  const { currentEnvironment } = useEnvironment();
  const isV2TemplateEditorEnabled = useFeatureFlag(FeatureFlagsKeysEnum.IS_V2_TEMPLATE_EDITOR_ENABLED);
  const { uiSchema } = props;
  const { body } = uiSchema?.properties ?? {};

  if (currentEnvironment?.type !== EnvironmentTypeEnum.DEV) {
    return <StepEditorUnavailable />;
  }

  return (
    <div className="flex h-full flex-col">
      <TabsSection className={cn(isV2TemplateEditorEnabled ? 'p-0 pb-3' : '')}>
        {!isV2TemplateEditorEnabled && (
          <div className="flex items-center gap-2.5 text-sm font-medium">Chat template editor</div>
        )}
        <div
          className={cn(
            'rounded-12 flex flex-col gap-2 border border-neutral-100 p-2',
            isV2TemplateEditorEnabled ? 'bg-bg-weak' : ''
          )}
        >
          {getComponentByType({ component: body.component })}
        </div>
      </TabsSection>
    </div>
  );
};
Repository: novuhq/novu
File: apps/dashboard/src/components/welcome/ai-prompts/framework-prompts/nextjs-prompt.ts (TypeScript)

Code before:
import { PromptConfig, replaceConfigVariables } from './types';

const KITCHEN_SINK_INBOX_SNIPPET = `'use client';
import { Inbox } from '@novu/nextjs';

export default function NotificationInbox({ subscriberId }: { subscriberId: string }) {
  // Ensure the environment variable is available
  const applicationIdentifier = process.env.NEXT_PUBLIC_NOVU_APPLICATION_IDENTIFIER;

  return (
    <Inbox
      // Required core configuration
      applicationIdentifier={applicationIdentifier}
      subscriberId={subscriberId}

      // Backend configuration (for EU region use https://eu.api.novu.co and https://eu.ws.novu.co)
      backendUrl=process.env.NOVU_BACKEND_URL
      socketUrl=process.env.NOVU_SOCKET_URL

      // Appearance configuration
      appearance={{
        // Base theme configuration
        baseTheme: 'dark', // Or undefined for light theme

        // Variables for global styling
        variables: {
          colorPrimary: '',
          colorPrimaryForeground: '',
          colorSecondary: '',
          colorSecondaryForeground: '',
          colorCounter: '',
          colorCounterForeground: '',
          colorBackground: '',
          colorRing: '',
          colorForeground: '',
          colorNeutral: '',
          colorShadow: '',

          // Typography and Layout
          fontSize: '',
        },
        elements: {
          bellIcon: {

Review comment (suggestion):
@iampearceman why not just inlining those? Without the use of env variables? 


```suggestion
      backendUrl="https://eu.api.novu.co"
```

Code after:
import { PromptConfig, replaceConfigVariables } from './types';

const KITCHEN_SINK_INBOX_SNIPPET = `'use client';
import { Inbox } from '@novu/nextjs';

export default function NotificationInbox({ subscriberId }: { subscriberId: string }) {
  // Ensure the environment variable is available
  const applicationIdentifier = process.env.NEXT_PUBLIC_NOVU_APPLICATION_IDENTIFIER;

  return (
    <Inbox
      // Required core configuration
      applicationIdentifier={applicationIdentifier}
      subscriberId={subscriberId}

      // Backend configuration (for EU region use https://eu.api.novu.co and wss://eu.ws.novu.co)
      backendUrl="https://eu.api.novu.co"
      socketUrl="wss://eu.ws.novu.co"

      // Appearance configuration
      appearance={{
        // Base theme configuration
        baseTheme: 'dark', // Or undefined for light theme

        // Variables for global styling
        variables: {
          colorPrimary: '',
          colorPrimaryForeground: '',
          colorSecondary: '',
          colorSecondaryForeground: '',
          colorCounter: '',
          colorCounterForeground: '',
          colorBackground: '',
          colorRing: '',
          colorForeground: '',
          colorNeutral: '',
          colorShadow: '',

          // Typography and Layout
          fontSize: '',
        },
        elements: {
          bellIcon: {
Repository: novuhq/novu
File: apps/api/src/app/environments-v1/usecases/output-renderers/base-translation-renderer.usecase.ts (TypeScript)

Code before:
    environmentId,
    organizationId,
    workflowId,
    locale,
  }: {
    content: string | Record<string, unknown>;
    variables: FullPayloadForRender;
    environmentId: string;
    organizationId: string;
    workflowId?: string;
    locale?: string;
  }): Promise<string | Record<string, unknown>> {
    if (!workflowId) {
      this.logger.error('Workflow ID is required for translation module', {
        workflowId,
        organizationId,
        environmentId,
        locale,
      });

      throw new Error('Workflow ID is required for translation module');
    }

    try {
      const translate = this.getTranslationModule();

      const contentString = typeof content === 'string' ? content : JSON.stringify(content);
      const liquidEngine = createLiquidEngine();

      const translatedContent = await translate.execute({
        workflowIdOrInternalId: workflowId,
        organizationId,
        environmentId,
        userId: 'system',
        locale,
        content: contentString,
        payload: variables,
        liquidEngine,
      });

      return typeof content === 'string' ? translatedContent : JSON.parse(translatedContent);
    } catch (error) {
      this.logger.error('Translation processing failed', {
        error: error?.message || error,
        workflowId,
        organizationId,
        environmentId,
        locale,
        stack: error?.stack,
      });


Review comment (suggestion):
I think this `if` statement was added during layouts impl. - I would suggest verifying if it doesn't break anything in layouts.

Code after:
      workflowId,
      locale,
      organization,
    }) as Promise<string>;
  }

  private async executeTranslation({
    content,
    variables,
    environmentId,
    organizationId,
    workflowId,
    locale,
    dbWorkflow,
    organization,
  }: {
    content: string | Record<string, unknown>;
    variables: FullPayloadForRender;
    environmentId: string;
    organizationId: string;
    workflowId?: string;
    locale?: string;
    dbWorkflow?: NotificationTemplateEntity;
    organization?: OrganizationEntity;
  }): Promise<string | Record<string, unknown>> {
    if (!workflowId) {
      this.logger.error('Workflow ID is required for translation module', {
        workflowId,
        organizationId,
        environmentId,
        locale,
      });

      throw new Error('Workflow ID is required for translation module');
    }

    try {
      const translate = this.getTranslationModule();

      const contentString = typeof content === 'string' ? content : JSON.stringify(content);
      const liquidEngine = createLiquidEngine();

      const translatedContent = await translate.execute({
        workflowIdOrInternalId: workflowId,
        organizationId,
        environmentId,
        userId: 'system',
        locale,
        content: contentString,
        payload: variables,
        liquidEngine,
Repository: nsqio/nsq
File: nsqd/lookup.go (Go)

Code before:
package main

import (
	"../nsq"
	"bitly/notify"
	"log"
	"net"
	"time"
)

var notifyChannelChan = make(chan interface{})
var notifyTopicChan = make(chan interface{})
var syncTopicChan = make(chan *nsq.LookupPeer)
var lookupPeers = make([]*nsq.LookupPeer, 0)

func lookupRouter(lookupHosts []string, exitChan chan int, exitSyncChan chan int) {
	tcpAddr, _ := net.ResolveTCPAddr("tcp", *tcpAddress)

	for _, host := range lookupHosts {
		tcpAddr, err := net.ResolveTCPAddr("tcp", host)
		if err != nil {
			log.Fatal("LOOKUP: could not resolve TCP address for %s", host)
		}
		log.Printf("LOOKUP: adding peer %s", tcpAddr.String())
		lookupPeer := nsq.NewLookupPeer(tcpAddr, func(lp *nsq.LookupPeer) {
			go func() {
				syncTopicChan <- lp
			}()
		})
		lookupPeers = append(lookupPeers, lookupPeer)
	}

	if len(lookupPeers) > 0 {
		notify.Start("new_channel", notifyChannelChan)
		notify.Start("new_topic", notifyTopicChan)
	}

	// for announcements, lookupd determines the host automatically
	ticker := time.Tick(15 * time.Second)
	for {
		select {
		case <-ticker:
			// send a heartbeat and read a response (read detects closed conns)
			for _, lookupPeer := range lookupPeers {
				log.Printf("LOOKUP: [%s] sending heartbeat", lookupPeer)
				_, err := lookupPeer.Command(nsq.Ping())
				if err != nil {
					log.Printf("ERROR: [%s] ping failed - %s", lookupPeer, err.Error())

Review comment (suggestion):
i think this type of logging is extremely helpful, and isn't that noisy (its only during initialization)


Code after:
package main

import (
	"../nsq"
	"bitly/notify"
	"log"
	"net"
	"os"
	"time"
)

var notifyChannelChan = make(chan interface{})
var notifyTopicChan = make(chan interface{})
var syncTopicChan = make(chan *nsq.LookupPeer)
var lookupPeers = make([]*nsq.LookupPeer, 0)

func lookupRouter(lookupHosts []string, exitChan chan int, exitSyncChan chan int) {
	tcpAddr, _ := net.ResolveTCPAddr("tcp", *tcpAddress)
	port := tcpAddr.Port

	netIps := getNetworkIPs(tcpAddr)
	for _, ip := range netIps {
		log.Printf("LOOKUP: identified interface %s", ip)
	}

	for _, host := range lookupHosts {
		tcpAddr, err := net.ResolveTCPAddr("tcp", host)
		if err != nil {
			log.Fatal("LOOKUP: could not resolve TCP address for %s", host)
		}
		log.Printf("LOOKUP: adding peer %s", tcpAddr.String())
		lookupPeer := nsq.NewLookupPeer(tcpAddr, func(lp *nsq.LookupPeer) {
			go func() {
				syncTopicChan <- lp
			}()
		})
		lookupPeers = append(lookupPeers, lookupPeer)
	}

	if len(lookupPeers) > 0 {
		notify.Start("new_channel", notifyChannelChan)
		notify.Start("new_topic", notifyTopicChan)
	}

	// for announcements, lookupd determines the host automatically
	ticker := time.Tick(15 * time.Second)
	for {
		select {
Repository: nsqio/nsq
File: nsqd/protocol_v2.go (Go)

Code before:
		client.Lock()
		client.SetWriteDeadline(time.Now().Add(time.Second))
		_, err := nsq.SendResponse(client, data)
		client.Unlock()
		if err != nil {
			if netErr, ok := err.(net.Error); ok && netErr.Timeout() {
				attempts++
				if attempts == 3 {
					return errors.New("E_WRITE_TIMEOUT")
				}
				continue
			}
			return err
		}
		break
	}
	return nil
}

func (p *ProtocolV2) Exec(client *ClientV2, params []string) ([]byte, error) {
	switch params[0] {
	case "SUB":
		return p.SUB(client, params)
	case "RDY":
		return p.RDY(client, params)
	case "FIN":
		return p.FIN(client, params)
	case "REQ":
		return p.REQ(client, params)
	case "CLS":
		return p.CLS(client, params)
	case "NOP":
		return p.NOP(client, params)
	}
	return nil, nsq.ClientErrInvalid
}

func (p *ProtocolV2) sendHeartbeat(client *ClientV2) error {
	clientData, err := nsq.Frame(nsq.FrameTypeResponse, []byte("_heartbeat_"))
	if err != nil {
		return err
	}

	err = p.Write(client, clientData)
	if err != nil {
		return err
	}

	return nil
}


Review comment (performance):
does a different bytes.Equal and call to params[0] for each case statement improve performance? It feels like the added verbosity at each case statement should lead to more work to be done for each case and slower performance, but I don't really have a great justification for why I think that. Just my impression


Code after:
		return err
	}

	attempts := 0
	for {
		client.SetWriteDeadline(time.Now().Add(time.Second))
		_, err := nsq.SendResponse(client, client.frameBuf.Bytes())
		if err != nil {
			if netErr, ok := err.(net.Error); ok && netErr.Timeout() {
				attempts++
				if attempts == 3 {
					return errors.New("E_WRITE_TIMEOUT")
				}
				continue
			}
			return err
		}
		break
	}

	return nil
}

func (p *ProtocolV2) Exec(client *ClientV2, params [][]byte) ([]byte, error) {
	switch {
	case bytes.Equal(params[0], []byte("SUB")):
		return p.SUB(client, params)
	case bytes.Equal(params[0], []byte("RDY")):
		return p.RDY(client, params)
	case bytes.Equal(params[0], []byte("FIN")):
		return p.FIN(client, params)
	case bytes.Equal(params[0], []byte("REQ")):
		return p.REQ(client, params)
	case bytes.Equal(params[0], []byte("CLS")):
		return p.CLS(client, params)
	case bytes.Equal(params[0], []byte("NOP")):
		return p.NOP(client, params)
	}
	return nil, nsq.ClientErrInvalid
}

func (p *ProtocolV2) sendHeartbeat(client *ClientV2) error {
	err := p.Send(client, nsq.FrameTypeResponse, []byte("_heartbeat_"))
	if err != nil {
		return err
	}

	return nil
}

func (p *ProtocolV2) messagePump(client *ClientV2) {
Repository: nsqio/nsq
File: nsqd/protocol_v2.go (Go)

Code before:
		client.Lock()
		client.SetWriteDeadline(time.Now().Add(time.Second))
		_, err := nsq.SendResponse(client, data)
		client.Unlock()
		if err != nil {
			if netErr, ok := err.(net.Error); ok && netErr.Timeout() {
				attempts++
				if attempts == 3 {
					return errors.New("E_WRITE_TIMEOUT")
				}
				continue
			}
			return err
		}
		break
	}
	return nil
}

func (p *ProtocolV2) Exec(client *ClientV2, params []string) ([]byte, error) {
	switch params[0] {
	case "SUB":
		return p.SUB(client, params)
	case "RDY":
		return p.RDY(client, params)
	case "FIN":
		return p.FIN(client, params)
	case "REQ":
		return p.REQ(client, params)
	case "CLS":
		return p.CLS(client, params)
	case "NOP":
		return p.NOP(client, params)
	}
	return nil, nsq.ClientErrInvalid
}

func (p *ProtocolV2) sendHeartbeat(client *ClientV2) error {
	clientData, err := nsq.Frame(nsq.FrameTypeResponse, []byte("_heartbeat_"))
	if err != nil {
		return err
	}

	err = p.Write(client, clientData)
	if err != nil {
		return err
	}

	return nil
}


Review comment (performance):
you can't compare slices like you compare strings using "normal" comparators.  the only way to write this (with slices) is as is.

I would imagine that performance for this specific section is roughly the same considering how string comparison would work internally (some function is called that walks the string and compares bytes).

It's visibly different but not functionally or speed different.


Code after:
		return err
	}

	attempts := 0
	for {
		client.SetWriteDeadline(time.Now().Add(time.Second))
		_, err := nsq.SendResponse(client, client.frameBuf.Bytes())
		if err != nil {
			if netErr, ok := err.(net.Error); ok && netErr.Timeout() {
				attempts++
				if attempts == 3 {
					return errors.New("E_WRITE_TIMEOUT")
				}
				continue
			}
			return err
		}
		break
	}

	return nil
}

func (p *ProtocolV2) Exec(client *ClientV2, params [][]byte) ([]byte, error) {
	switch {
	case bytes.Equal(params[0], []byte("SUB")):
		return p.SUB(client, params)
	case bytes.Equal(params[0], []byte("RDY")):
		return p.RDY(client, params)
	case bytes.Equal(params[0], []byte("FIN")):
		return p.FIN(client, params)
	case bytes.Equal(params[0], []byte("REQ")):
		return p.REQ(client, params)
	case bytes.Equal(params[0], []byte("CLS")):
		return p.CLS(client, params)
	case bytes.Equal(params[0], []byte("NOP")):
		return p.NOP(client, params)
	}
	return nil, nsq.ClientErrInvalid
}

func (p *ProtocolV2) sendHeartbeat(client *ClientV2) error {
	err := p.Send(client, nsq.FrameTypeResponse, []byte("_heartbeat_"))
	if err != nil {
		return err
	}

	return nil
}

func (p *ProtocolV2) messagePump(client *ClientV2) {
Repository: nsqio/nsq
File: nsq/reader.go (Go)

Code before:
				select {
				case q.lookupdRecheckChan <- 1:
				default:
				}
			}
			return
		}

		if msg.Success {
			if q.VerboseLogging {
				log.Printf("[%s] finishing %s", c, msg.Id)
			}
			err := c.sendCommand(Finish(msg.Id))
			if err != nil {
				log.Printf("[%s] error finishing %s - %s", c, msg.Id, err.Error())
				c.stopFinishLoop()
				continue
			}
			atomic.AddUint64(&c.messagesFinished, 1)
			atomic.AddUint64(&q.MessagesFinished, 1)
		} else {
			if q.VerboseLogging {
				log.Printf("[%s] requeuing %s", c, msg.Id)
			}
			err := c.sendCommand(Requeue(msg.Id, msg.RequeueDelayMs))
			if err != nil {
				log.Printf("[%s] error requeueing %s - %s", c, msg.Id, err.Error())
				c.stopFinishLoop()
				continue
			}
			atomic.AddUint64(&c.messagesRequeued, 1)
			atomic.AddUint64(&q.MessagesRequeued, 1)
		}

		atomic.AddInt64(&q.messagesInFlight, -1)
		if atomic.AddInt64(&c.messagesInFlight, -1) == 0 &&
			(atomic.LoadInt32(&c.stopFlag) == 1 || atomic.LoadInt32(&q.stopFlag) == 1) {
			c.stopFinishLoop()
			continue
		}
	}
}

func (q *Reader) updateReady(c *nsqConn) {
	if atomic.LoadInt32(&c.stopFlag) != 0 {
		return
	}

	remain := atomic.LoadInt64(&c.rdyCount)
	s := q.ConnectionMaxInFlight()
	// refill when at 1, or at 25% whichever comes first

Review comment (suggestion):
got it... even easier might be to just `close(c.dying)` instead of writing to it, a pattern we've followed elsewhere to use a channel as an exit signal.


Code after:
					log.Printf("[%s] requeuing %s", c, msg.Id)
				}
				err := c.sendCommand(Requeue(msg.Id, msg.RequeueDelayMs))
				if err != nil {
					log.Printf("[%s] error requeueing %s - %s", c, msg.Id, err.Error())
					q.stopFinishLoop(c)
					continue
				}
				atomic.AddUint64(&c.messagesRequeued, 1)
				atomic.AddUint64(&q.MessagesRequeued, 1)
			}

			if atomic.LoadInt64(&c.messagesInFlight) == 0 &&
				(atomic.LoadInt32(&c.stopFlag) == 1 || atomic.LoadInt32(&q.stopFlag) == 1) {
				q.stopFinishLoop(c)
				continue
			}
		}
	}
}

func (q *Reader) stopFinishLoop(c *nsqConn) {
	c.stopper.Do(func() {
		log.Printf("[%s] beginning stopFinishLoop logic", c)
		// This doesn't block because dying has buffer of 1
		c.dying <- struct{}{}

		// Drain the finishedMessages channel
		go func() {
			<-c.drainReady
			for atomic.AddInt64(&c.messagesInFlight, -1) >= 0 {
				<-c.finishedMessages
			}
		}()
		c.Close()
		delete(q.nsqConnections, c.String())

		log.Printf("there are %d connections left alive", len(q.nsqConnections))

		if len(q.nsqConnections) == 0 && len(q.lookupdHTTPAddrs) == 0 {
			// no lookupd entry means no reconnection
			if atomic.LoadInt32(&q.stopFlag) == 1 {
				q.stopHandlers()
			}
			return
		}

		// ie: we were the last one, and stopping
		if len(q.nsqConnections) == 0 && atomic.LoadInt32(&q.stopFlag) == 1 {
			q.stopHandlers()
		}