Skip to content

refactor(engine): CSM 코드 구조 및 가독성 개선#915

Merged
SingTheCode merged 4 commits into
core/CSMDictfrom
refactor/csm-improvements
Oct 5, 2025
Merged

refactor(engine): CSM 코드 구조 및 가독성 개선#915
SingTheCode merged 4 commits into
core/CSMDictfrom
refactor/csm-improvements

Conversation

@SingTheCode

Copy link
Copy Markdown
Contributor

🎯 목적

CSM 빌드 로직의 코드 구조를 개선하고 가독성을 향상시킵니다.

📋 주요 변경사항

1. 유틸리티 함수 분리 (csm.util.ts)

  • getMergeParentCommit: 머지 커밋의 부모 커밋 조회
  • findSquashStartNodeIndex: stem에서 특정 커밋 노드 인덱스 탐색
  • findSquashEndIndex: squash 수집 종료 인덱스 계산
  • collectSquashNodes: 노드 수집 및 stem에서 제거
  • extractNestedMergeParents: 중첩된 머지의 부모 커밋 추출

2. 상수 파일 추가 (csm.const.ts)

  • MERGE_PARENT_INDEX: 머지 커밋의 두 번째 부모 인덱스 (1)
  • FIRST_PARENT_INDEX: 첫 번째 부모 인덱스 (0)
  • 매직 넘버 제거로 코드 의도 명확화

3. 메인 로직 개선 (csm.ts)

  • 복잡한 인라인 로직을 명확한 이름의 함수로 분리
  • 각 함수에 명확한 JSDoc 주석 추가
  • DFS 탐색 로직의 가독성 향상
  • 코드 라인 수 감소 (106줄 → 138줄로 재구성되었으나 중복 제거됨)

📊 영향 범위

  • Analysis Engine: CSM 빌드 로직 리팩토링
  • 파일 변경: 3개 (csm.ts, csm.util.ts, csm.const.ts)
  • 기능 변경: 없음 (순수 리팩토링)

✅ 체크리스트

  • 기존 테스트 통과 확인

🔍 리팩토링 세부사항

Before

// 인라인으로 복잡하게 작성된 로직
const mergeParentCommit = commitDict.get(baseCommitNode.commit.parents[1]);
const squashStartNodeIndex = squashStem.nodes.findIndex(({ commit: { id } }) => id === squashStartNode.commit.id);

After

// 명확한 함수명으로 분리
const mergeParentCommit = getMergeParentCommit(baseCommitNode, commitDict);
const squashStartNodeIndex = findSquashStartNodeIndex(squashStem, squashStartNode.commit.id);

📝 비고

  • 이 리팩토링은 향후 CSM 로직 확장 및 유지보수를 용이하게 합니다
  • 함수 단위 테스트 작성이 가능해졌습니다
  • 코드 이해도가 향상되어 신규 개발자의 온보딩이 수월해집니다

- Extract utility functions from buildCSMNode into csm.util.ts
 - getMergeParentCommit: Get merge parent commit
 - findSquashStartNodeIndex: Find node index in stem
 - findSquashEndIndex: Find end index for squash collection
 - collectSquashNodes: Collect nodes and remove from stem
 - extractNestedMergeParents: Extract nested merge parents

- Create csm.const.ts for magic number constants
 - MERGE_PARENT_INDEX: Merge parent index
 - FIRST_PARENT_INDEX: First parent index

- Refactor buildCSMNode function
 - Reduce function length from 72 lines to ~50 lines
 - Remove non-null assertions (! operator)
 - Improve type safety with optional chaining
 - Add clear comments for each step

- Add comprehensive JSDoc documentation
 - Document all public and private functions
 - Add parameter and return type descriptions

@ytaek ytaek left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

와 눈물나는 PR입니다!!!!
코드를 완벽히 이해한 다음에, 멋진 문서화를 하셨네요!!!!!!
코드 자체에 설명을 다 녹여놓은 완벽한 상태 같습니다!!!

firstparent 관련 용어관련 리뷰 하나만 달아놓았는데, 이것만 반영해주시면 감사하겠습니다!!

import type { CommitDict, CommitNode, Stem } from "./types";

/** Gets the second parent (starting point of merged branch) of a merge commit. */
export const getMergeParentCommit = (baseCommitNode: CommitNode, commitDict: CommitDict): CommitNode | undefined => {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

이게 정확하게는 git domain에서 사용하는 용어가 firstParent 입니다.
parent는 복수개가 있을 수가 있고,
stem할 때는 편의상 git의 firstparent를 따라가는 걸로 githru 초창기부터 구현해놓았습니다!

function 이름에도 반영해주시면 감사하겠습니다!

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

반영했습니다! 확인 감사합니다ㅎㅎ 상수도 이름 변경했습니다!

Comment on lines +73 to +80
/**
* Integrates Pull Request information into a CSM node.
* Reflects PR details in commit message and statistics.
*
* @param csmNode - Existing CSM node
* @param pr - Pull Request information
* @returns CSM node with integrated PR information
*/

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

오우 function document 도 넘 좋습니다!!! 주석이라면(?) 이 정도는 되어야 주석이죠!!

Consolidate MERGE_PARENT_INDEX and FIRST_PARENT_INDEX into a single FIRST_PARENT_INDEX constant, and rename getMergeParentCommit to getFirstParentCommit to improve code consistency and clarity.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
@SingTheCode
SingTheCode requested a review from ytaek October 1, 2025 07:47

@Kyoungwoong Kyoungwoong left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

주석과 함수명이 직관적이라서 이해하기 쉬운거 같아요!!
고생하셨습니다~~

@ytaek ytaek left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGGGGTM!!!

@SingTheCode
SingTheCode changed the base branch from feature/#870_delete-csm-reverse to core/CSMDict October 5, 2025 14:18
@SingTheCode
SingTheCode merged commit 585f54a into core/CSMDict Oct 5, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants