ronantakizawa/codereview-bench huggingface dataset:
Repository: toeverything/AFFiNE
File: packages/backend/server/src/models/workspace.ts (TypeScript)

Code before:
   */
  @Transactional()
  async create(userId: string) {
    const workspace = await this.db.workspace.create({
      data: { public: false },
    });
    this.logger.log(`Workspace created with id ${workspace.id}`);
    await this.models.workspaceUser.setOwner(workspace.id, userId);
    return workspace;
  }

  /**
   * Update the workspace with the given data.
   */
  async update(workspaceId: string, data: UpdateWorkspaceInput) {
    const workspace = await this.db.workspace.update({
      where: {
        id: workspaceId,
      },
      data,
    });
    this.logger.debug(
      `Updated workspace ${workspaceId} with data ${JSON.stringify(data)}`
    );

    if (workspace.enableDocEmbedding) {
      // trigger workspace embedding
      this.event.emit('workspace.embedding', {
        workspaceId: workspace.id,
      });
    }
    return workspace;
  }

  async get(workspaceId: string) {
    return await this.db.workspace.findUnique({
      where: {
        id: workspaceId,
      },
    });
  }

  async findMany(ids: string[]) {
    return await this.db.workspace.findMany({
      where: {
        id: { in: ids },
      },
    });
  }

  async listAfterSid(sid: number, limit: number) {

Review comment (suggestion):
A `workspace.updated` event should be added, and then this event should be listened to for AI embedding logic to avoid the logic of the workspace model being aware of AI

Code after:
   * Create a new workspace for the user, default to private.
   */
  @Transactional()
  async create(userId: string) {
    const workspace = await this.db.workspace.create({
      data: { public: false },
    });
    this.logger.log(`Workspace created with id ${workspace.id}`);
    await this.models.workspaceUser.setOwner(workspace.id, userId);
    return workspace;
  }

  /**
   * Update the workspace with the given data.
   */
  async update(workspaceId: string, data: UpdateWorkspaceInput) {
    const workspace = await this.db.workspace.update({
      where: {
        id: workspaceId,
      },
      data,
    });
    this.logger.debug(
      `Updated workspace ${workspaceId} with data ${JSON.stringify(data)}`
    );

    this.event.emit('workspace.updated', workspace);

    return workspace;
  }

  async get(workspaceId: string) {
    return await this.db.workspace.findUnique({
      where: {
        id: workspaceId,
      },
    });
  }

  async findMany(ids: string[]) {
    return await this.db.workspace.findMany({
      where: {
        id: { in: ids },
      },
    });
  }

  async listAfterSid(sid: number, limit: number) {
    return await this.db.workspace.findMany({
      where: {
        sid: { gt: sid },
Repository: toeverything/AFFiNE
File: packages/backend/server/src/models/copilot-workspace.ts (TypeScript)

Code before:
        docId: true,
        createdAt: true,
      },
      orderBy: { createdAt: 'desc' },
      skip: options?.offset,
      take: options?.first,
    });
  }

  /**
   * find docs to embed, excluding ignored and already embedded docs
   * newer docs will be list first
   * @param workspaceId id of the workspace
   * @returns docIds
   */
  @Transactional()
  async findDocsToEmbed(workspaceId: string): Promise<string[]> {
    const docIds = await this.db.snapshot
      .findMany({
        where: { workspaceId },
        select: { id: true },
      })
      .then(r => r.map(doc => doc.id));

    const skipDocIds = await Promise.all([
      this.models.copilotWorkspace.listEmbeddedDocIds(workspaceId),
      this.models.copilotWorkspace.listIgnoredDocIds(workspaceId),
    ]).then(r => new Set(r.flat().map(r => r.docId)));

    return docIds.filter(id => !skipDocIds.has(id));
  }

  @Transactional()
  async updateIgnoredDocs(
    workspaceId: string,
    add: string[] = [],
    remove: string[] = []
  ) {
    const removed = new Set(remove);
    const ignored = await this.listIgnoredDocIds(workspaceId).then(
      r => new Set(r.map(r => r.docId).filter(id => !removed.has(id)))
    );
    const added = add.filter(id => !ignored.has(id));

    const { count: addedCount } =
      await this.db.aiWorkspaceIgnoredDocs.createMany({
        data: added.map(docId => ({
          workspaceId,
          docId,
        })),
      });

Review comment (suggestion):
try 'none' https://www.prisma.io/docs/orm/reference/prisma-client-reference#none

or at least `this.listEmbededDocIds`

Code after:
        where: {
          workspaceId,
          embedding: {
            is: null,
          },
        },
        select: { id: true },
      })
      .then(r => r.map(doc => doc.id));

    const skipDocIds = await this.listIgnoredDocIds(workspaceId).then(
      r => new Set(r.map(r => r.docId))
    );

    return docIds.filter(id => !skipDocIds.has(id));
  }

  @Transactional()
  async updateIgnoredDocs(
    workspaceId: string,
    add: string[] = [],
    remove: string[] = []
  ) {
    const removed = new Set(remove);
    const ignored = await this.listIgnoredDocIds(workspaceId).then(
      r => new Set(r.map(r => r.docId).filter(id => !removed.has(id)))
    );
    const added = add.filter(id => !ignored.has(id));

    const { count: addedCount } =
      await this.db.aiWorkspaceIgnoredDocs.createMany({
        data: added.map(docId => ({
          workspaceId,
          docId,
        })),
      });

    const { count: removedCount } =
      await this.db.aiWorkspaceIgnoredDocs.deleteMany({
        where: {
          workspaceId,
          docId: {
            in: Array.from(removed),
          },
        },
      });

    return addedCount + removedCount;
  }

  @Transactional()
Repository: toeverything/AFFiNE
File: blocksuite/affine/blocks/surface/src/renderer/dom-renderer.ts (TypeScript)

Code before:
  }

  refresh() {
    if (this._refreshRafId !== null) return;

    this._refreshRafId = requestConnectedFrame(() => {
      this._refreshRafId = null;
      this._render();
    }, this._container);
  }

  removeOverlay(overlay: Overlay) {
    if (!this._overlays.has(overlay)) {
      return;
    }

    this._overlays.delete(overlay);
    this.refresh();
  }

  /**
   * Mark a specific element as dirty for incremental updates
   * @param elementId - The ID of the element to mark as dirty
   * @param updateType - The type of update (optional, defaults to ELEMENT_UPDATED)
   */
  markElementDirty(
    elementId: string,
    updateType: UpdateType = UpdateType.ELEMENT_UPDATED
  ) {
    this._markElementDirty(elementId, updateType);
  }

  /**
   * Force a full re-render of all elements
   */
  forceFullRender() {
    this._updateState.viewportDirty = true;
    this.refresh();
  }

  private _markElementDirty(elementId: string, updateType: UpdateType) {
    this._updateState.dirtyElementIds.add(elementId);
    const currentUpdates =
      this._updateState.pendingUpdates.get(elementId) || [];
    if (!currentUpdates.includes(updateType)) {
      currentUpdates.push(updateType);
      this._updateState.pendingUpdates.set(elementId, currentUpdates);
    }
  }

  private _markViewportDirty() {

Review comment (suggestion):
it is very likely the public API will be called with a modified `this` closure. We have a bunch of issues on sentry caused by this reason. Make sure the `this` is pointed to the correct instance by changing it to an arrow method.

Code after:
  getPropertyValue = (property: string) => {
    return this.provider.getPropertyValue?.(property) ?? '';
  };

  refresh = () => {
    if (this._refreshRafId !== null) return;

    this._refreshRafId = requestConnectedFrame(() => {
      this._refreshRafId = null;
      this._render();
    }, this._container);
  };

  removeOverlay = (overlay: Overlay) => {
    if (!this._overlays.has(overlay)) {
      return;
    }

    this._overlays.delete(overlay);
    this.refresh();
  };

  /**
   * Mark a specific element as dirty for incremental updates
   * @param elementId - The ID of the element to mark as dirty
   * @param updateType - The type of update (optional, defaults to ELEMENT_UPDATED)
   */
  markElementDirty = (
    elementId: string,
    updateType: UpdateType = UpdateType.ELEMENT_UPDATED
  ) => {
    this._markElementDirty(elementId, updateType);
  };

  /**
   * Force a full re-render of all elements
   */
  forceFullRender = () => {
    this._updateState.viewportDirty = true;
    this.refresh();
  };

  private _markElementDirty(elementId: string, updateType: UpdateType) {
    this._updateState.dirtyElementIds.add(elementId);
    const currentUpdates =
      this._updateState.pendingUpdates.get(elementId) || [];
    if (!currentUpdates.includes(updateType)) {
      currentUpdates.push(updateType);
      this._updateState.pendingUpdates.set(elementId, currentUpdates);
    }
  }
Repository: toeverything/AFFiNE
File: packages/backend/server/src/plugins/copilot/context/service.ts (TypeScript)

Code before:
    topK: number = 5,
    signal?: AbortSignal,
    threshold: number = 0.5
  ) {
    if (!this.embeddingClient) return [];
    const embedding = await this.embeddingClient.getEmbedding(content, signal);
    if (!embedding) return [];

    const [fileChunks, workspaceChunks] = await Promise.all([
      this.models.copilotWorkspace.matchFileEmbedding(
        workspaceId,
        embedding,
        topK * 2,
        threshold
      ),
      this.models.copilotContext.matchWorkspaceEmbedding(
        embedding,
        workspaceId,
        topK * 2,
        threshold
      ),
    ]);

    if (!fileChunks.length && !workspaceChunks.length) return [];

    return this.embeddingClient.reRank(
      content,
      [...fileChunks, ...workspaceChunks],
      topK,
      signal
    );
  }

  @OnEvent('workspace.doc.embed.failed')
  async onDocEmbedFailed({
    contextId,
    docId,
  }: Events['workspace.doc.embed.failed']) {
    const context = await this.get(contextId);
    await context.saveDocRecord(docId, doc => ({
      ...(doc as ContextDoc),
      status: ContextEmbedStatus.failed,
    }));
  }

  @OnEvent('workspace.file.embed.finished')
  async onFileEmbedFinish({
    contextId,
    fileId,
    chunkSize,
  }: Events['workspace.file.embed.finished']) {

Review comment (suggestion):
Keep adding an `await` call instead of returning a promise directly

Code after:
  async matchWorkspaceAll(
    workspaceId: string,
    content: string,
    topK: number = 5,
    signal?: AbortSignal,
    threshold: number = 0.5
  ) {
    if (!this.embeddingClient) return [];
    const embedding = await this.embeddingClient.getEmbedding(content, signal);
    if (!embedding) return [];

    const [fileChunks, workspaceChunks] = await Promise.all([
      this.models.copilotWorkspace.matchFileEmbedding(
        workspaceId,
        embedding,
        topK * 2,
        threshold
      ),
      this.models.copilotContext.matchWorkspaceEmbedding(
        embedding,
        workspaceId,
        topK * 2,
        threshold
      ),
    ]);

    if (!fileChunks.length && !workspaceChunks.length) return [];

    return await this.embeddingClient.reRank(
      content,
      [...fileChunks, ...workspaceChunks],
      topK,
      signal
    );
  }

  @OnEvent('workspace.doc.embed.failed')
  async onDocEmbedFailed({
    contextId,
    docId,
  }: Events['workspace.doc.embed.failed']) {
    const context = await this.get(contextId);
    await context.saveDocRecord(docId, doc => ({
      ...(doc as ContextDoc),
      status: ContextEmbedStatus.failed,
    }));
  }

  @OnEvent('workspace.file.embed.finished')
  async onFileEmbedFinish({
    contextId,
Repository: toeverything/AFFiNE
File: packages/backend/server/src/core/doc-service/controller.ts (TypeScript)

Code before:
  async getDoc(
    @Param('workspaceId') workspaceId: string,
    @Param('docId') docId: string,
    @Res() res: Response
  ) {
    const doc = await this.docReader.getDoc(workspaceId, docId);
    if (!doc) {
      throw new NotFound('Doc not found');
    }
    this.logger.debug(
      `get doc ${docId} from workspace ${workspaceId}, size: ${doc.bin.length}`
    );
    res.setHeader('x-doc-timestamp', doc.timestamp.toString());
    if (doc.editor) {
      res.setHeader('x-doc-editor-id', doc.editor);
    }
    res.send(doc.bin);
  }

  @SkipThrottle()
  @Internal()
  @Get('/workspaces/:workspaceId/docs/:docId/markdown')
  async getDocMarkdown(
    @Param('workspaceId') workspaceId: string,
    @Param('docId') docId: string,
    @Query('aiEditable') aiEditable: boolean
  ) {
    const result = await this.docReader.getDocMarkdown(
      workspaceId,
      docId,
      aiEditable
    );
    if (!result) {
      throw new NotFound('Doc not found');
    }
    return result;
  }

  @SkipThrottle()
  @Internal()
  @Post('/workspaces/:workspaceId/docs/:docId/diff')
  async getDocDiff(
    @Param('workspaceId') workspaceId: string,
    @Param('docId') docId: string,
    @RawBody() stateVector: Buffer | undefined,
    @Res() res: Response
  ) {
    const diff = await this.docReader.getDocDiff(
      workspaceId,
      docId,
      stateVector

Review comment (suggestion):
aiEditable is string，should convert it to bool

```ts
aiEditable === 'true'
```

Code after:
  async getDoc(
    @Param('workspaceId') workspaceId: string,
    @Param('docId') docId: string,
    @Res() res: Response
  ) {
    const doc = await this.docReader.getDoc(workspaceId, docId);
    if (!doc) {
      throw new NotFound('Doc not found');
    }
    this.logger.debug(
      `get doc ${docId} from workspace ${workspaceId}, size: ${doc.bin.length}`
    );
    res.setHeader('x-doc-timestamp', doc.timestamp.toString());
    if (doc.editor) {
      res.setHeader('x-doc-editor-id', doc.editor);
    }
    res.send(doc.bin);
  }

  @SkipThrottle()
  @Internal()
  @Get('/workspaces/:workspaceId/docs/:docId/markdown')
  async getDocMarkdown(
    @Param('workspaceId') workspaceId: string,
    @Param('docId') docId: string,
    @Query('aiEditable') aiEditable?: string
  ) {
    const result = await this.docReader.getDocMarkdown(
      workspaceId,
      docId,
      aiEditable === 'true'
    );
    if (!result) {
      throw new NotFound('Doc not found');
    }
    return result;
  }

  @SkipThrottle()
  @Internal()
  @Post('/workspaces/:workspaceId/docs/:docId/diff')
  async getDocDiff(
    @Param('workspaceId') workspaceId: string,
    @Param('docId') docId: string,
    @RawBody() stateVector: Buffer | undefined,
    @Res() res: Response
  ) {
    const diff = await this.docReader.getDocDiff(
      workspaceId,
      docId,
      stateVector
Repository: toeverything/AFFiNE
File: packages/frontend/core/src/components/hooks/affine/use-selfhost-login-version-guard.tsx (TypeScript)

Code before:
          <br />
          <a
            style={{
              whiteSpace: 'break-spaces',
              wordBreak: 'break-all',
              fontSize: 12,
              lineHeight: '16px',
            }}
          >
            https://docs.affine.pro/self-host-affine/install/upgrade
          </a>
        </div>
      </div>
    ),
  },
];

/**
 * Return the error tip if the server version is not meet the requirement
 */
export const useSelfhostLoginVersionGuard = (server: Server) => {
  const serverVersion =
    useLiveData(server.config$.selector(c => c.version)) ?? '0.0.0';

  for (const rule of rules) {
    if (serverVersion.localeCompare(rule.min) < 0) {
      return rule.tip(serverVersion, rule.min);
    }
  }

  return null;
};


Review comment (suggestion):
this isn't right.

```
"0.12.22".localeCompare("0.100.0") = 1
```

Code after:
          <span style={{ fontWeight: 500 }}>Instructions:</span>
          <br />
          <a
            style={{
              whiteSpace: 'break-spaces',
              wordBreak: 'break-all',
              fontSize: 12,
              lineHeight: '16px',
            }}
          >
            https://docs.affine.pro/self-host-affine/install/upgrade
          </a>
        </div>
      </div>
    ),
  },
];

/**
 * Return the error tip if the server version is not meet the requirement
 */
export const useSelfhostLoginVersionGuard = (server: Server) => {
  const serverVersion =
    useLiveData(server.config$.selector(c => c.version)) ?? '0.0.0';

  for (const rule of rules) {
    if (semver.lt(serverVersion, rule.min)) {
      return rule.tip(serverVersion, rule.min);
    }
  }

  return null;
};
Repository: tokio-rs/axum
File: axum/src/middleware/map_request.rs (Rust)

Code before:
    sync::Arc,
    task::{Context, Poll},
};
use tower::{util::BoxCloneService, ServiceBuilder};
use tower_layer::Layer;
use tower_service::Service;

// TODO(david): naming
pub trait IntoResult<T> {
    fn into_result(self) -> Result<T, Response>;
}

impl<T, E> IntoResult<T> for Result<T, E>
where
    E: IntoResponse,
{
    fn into_result(self) -> Result<T, Response> {
        self.map_err(IntoResponse::into_response)
    }
}

impl<B> IntoResult<Request<B>> for Request<B> {
    fn into_result(self) -> Result<Request<B>, Response> {
        Ok(self)
    }
}

pub fn map_request<F, T>(f: F) -> MapRequestLayer<F, (), T> {
    map_request_with_state((), f)
}

pub fn map_request_with_state<F, S, T>(state: S, f: F) -> MapRequestLayer<F, S, T> {
    map_request_with_state_arc(Arc::new(state), f)
}

pub fn map_request_with_state_arc<F, S, T>(state: Arc<S>, f: F) -> MapRequestLayer<F, S, T> {
    MapRequestLayer {
        f,
        state,
        _extractor: PhantomData,
    }
}

pub struct MapRequestLayer<F, S, T> {
    f: F,
    state: Arc<S>,
    _extractor: PhantomData<fn() -> T>,
}

impl<F, S, T> Clone for MapRequestLayer<F, S, T>
where

Review comment (suggestion):
This allows users to short-circuit, without requiring `try_*` versions. Though it might be easier to understand if we have the `try_*` versions.

Code after:
};
use tower_layer::Layer;
use tower_service::Service;

/// Create a middleware from an async function that transforms a request.
///
/// This differs from [`tower::util::MapRequest`] in that it allows you to easily run axum-specific
/// extractors.
///
/// # Example
///
/// ```
/// use axum::{
///     Router,
///     routing::get,
///     middleware::map_request,
///     http::Request,
/// };
///
/// async fn set_header<B>(mut request: Request<B>) -> Request<B> {
///     request.headers_mut().insert("x-foo", "foo".parse().unwrap());
///     request
/// }
///
/// async fn handler<B>(request: Request<B>) {
///     // `request` will have an `x-foo` header
/// }
///
/// let app = Router::new()
///     .route("/", get(handler))
///     .layer(map_request(set_header));
/// # let _: Router = app;
/// ```
///
/// # Rejection the request
///
/// The function given to `map_request` is allowed to also return a `Result` which can be used to
/// reject the request and return a response immediately, without calling the remaining
/// middleware.
///
/// Specifically the valid return types are:
///
/// - `Request<B>`
/// - `Request<Request<B>, E> where E:  IntoResponse`
///
/// ```
/// use axum::{
///     Router,
///     http::{Request, StatusCode},
///     routing::get,
///     middleware::map_request,
Repository: tokio-rs/axum
File: axum/src/middleware/map_request.rs (Rust)

Code before:
use crate::response::{IntoResponse, Response};
use axum_core::extract::{FromRequest, FromRequestParts};
use futures_util::future::BoxFuture;
use http::Request;
use std::{
    any::type_name,
    convert::Infallible,
    fmt,
    future::Future,
    marker::PhantomData,
    pin::Pin,
    sync::Arc,
    task::{Context, Poll},
};
use tower_layer::Layer;
use tower_service::Service;

/// Create a middleware from an async function that transforms a request.
///
/// This differs from [`tower::util::MapRequest`] in that it allows you to easily run axum specific
/// extractors.
///
/// # Example
///
/// ```
/// use axum::{
///     Router,
///     routing::get,
///     middleware::map_request,
///     http::Request,
/// };
///
/// async fn set_header<B>(mut request: Request<B>) -> Request<B> {
///     request.headers_mut().insert("x-foo", "foo".parse().unwrap());
///     request
/// }
///
/// async fn handler<B>(request: Request<B>) {
///     // `request` will have an `x-foo` header
/// }
///
/// let app = Router::new()
///     .route("/", get(handler))
///     .layer(map_request(set_header));
/// # let _: Router = app;

Review comment (suggestion):
```suggestion
/// This differs from [`tower::util::MapRequest`] in that it allows you to easily run axum-specific
```

Code after:
use crate::response::{IntoResponse, Response};
use axum_core::extract::{FromRequest, FromRequestParts};
use futures_util::future::BoxFuture;
use http::Request;
use std::{
    any::type_name,
    convert::Infallible,
    fmt,
    future::Future,
    marker::PhantomData,
    pin::Pin,
    sync::Arc,
    task::{Context, Poll},
};
use tower_layer::Layer;
use tower_service::Service;

/// Create a middleware from an async function that transforms a request.
///
/// This differs from [`tower::util::MapRequest`] in that it allows you to easily run axum-specific
/// extractors.
///
/// # Example
///
/// ```
/// use axum::{
///     Router,
///     routing::get,
///     middleware::map_request,
///     http::Request,
/// };
///
/// async fn set_header<B>(mut request: Request<B>) -> Request<B> {
///     request.headers_mut().insert("x-foo", "foo".parse().unwrap());
///     request
/// }
///
/// async fn handler<B>(request: Request<B>) {
///     // `request` will have an `x-foo` header
/// }
///
/// let app = Router::new()
///     .route("/", get(handler))
///     .layer(map_request(set_header));
/// # let _: Router = app;