Skip to content
This repository was archived by the owner on Dec 15, 2025. It is now read-only.

Conversation

Copy link
Contributor

Copilot AI commented Dec 13, 2025

Removes hard dependency between ClusterNode and Search by introducing an EagerEngine interface, enabling alternative engine implementations.

Changes

  • Added EagerEngine interface defining the engine contract:

    • input(data: string): string | null - returns formatted data or null on rejection
    • output(callback: (result: string) => void): number - executes search round
    • meta(): {input: string[], output: string[]} - exposes I/O patterns
  • Renamed ClusterNodeEagerNode with constructor signature change:

    // Before
    constructor(addr: string, id: string = randomUUID(), limit_size: number = 1000, buffer_size: number = 10000)
    
    // After
    constructor(engine: EagerEngine, addr: string, id: string = randomUUID())
  • Updated Search to implement EagerEngine (methods already matched interface)

  • Replaced non-null assertions with null checks in all engine.input() call sites:

    // Before
    const formattedItem = this.engine.input(item)!;
    this.data.add(formattedItem);
    
    // After
    const formattedItem = this.engine.input(item);
    if (formattedItem !== null) {
        this.data.add(formattedItem);
    }
  • Updated instantiation sites to inject Search instance:

    const engine = new Search();
    const node = new EagerNode(engine, listenAddr);

Usage

Engines can now be swapped at runtime:

class CustomEngine implements EagerEngine {
    input(data: string): string | null { /* ... */ }
    output(callback: (result: string) => void): number { /* ... */ }
    meta(): {input: string[], output: string[]} { /* ... */ }
}

const node = new EagerNode(new CustomEngine(), "0.0.0.0:8080");
Original prompt

This section details on the original issue you should resolve

<issue_title>将 ClusterNode 和 Search 彻底解耦合</issue_title>
<issue_description>ClusterNode改名叫做 EagerNode, 他接收一个对象,实现了EagerEngine就行, 而Search实现了EagerEngine。

EagerEngine大概有这几个接口:

  • input(string): string | null # 接收一个数据, 返回的如果是string就是接受了这个数据,并返回了格式化后的数据,如果是null就是不接受。所以现在有 this.engine.input(item)! 的地方不能直接用感叹号, 要判断非空了
  • output((string) -> void): number # 进行一轮搜索,将结果调用callback
  • meta(): {input: string[], output: string[]} # 提供当前状态下的输入输出模式

然后 EagerNode 的构造函数大概是 constructor(engine: EagerEngine, addr: string, id: string = randomUUID())</issue_description>

Comments on the Issue (you are @copilot in this section)

  • Fixes USTC-KnowledgeComputingLab/ddss#17

💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

- Renamed ClusterNode to EagerNode
- Created EagerEngine interface with input, output, and meta methods
- Updated Search class to implement EagerEngine interface
- Changed EagerNode constructor to accept EagerEngine instance
- Removed direct coupling between EagerNode and Search
- Fixed all non-null assertion usages to properly handle null values

Co-authored-by: hzhangxyz <11623447+hzhangxyz@users.noreply.github.com>
Copilot AI changed the title [WIP] Refactor ClusterNode and Search for decoupling Decouple EagerNode and Search via EagerEngine interface Dec 13, 2025
Copilot AI requested a review from hzhangxyz December 13, 2025 08:31
@hzhangxyz hzhangxyz marked this pull request as ready for review December 13, 2025 08:54
Copilot AI review requested due to automatic review settings December 13, 2025 08:54
@hzhangxyz hzhangxyz merged commit e810e2c into main Dec 13, 2025
1 check passed
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR successfully decouples the ClusterNode (now EagerNode) from the Search class by introducing an EagerEngine interface, enabling alternative engine implementations through dependency injection.

  • Introduced EagerEngine interface defining the contract for engine implementations
  • Renamed ClusterNode to EagerNode with constructor accepting an EagerEngine instance
  • Replaced non-null assertions with proper null checks throughout the codebase

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

将 ClusterNode 和 Search 彻底解耦合

2 participants