Skip to content

Conversation

@TaskerJang
Copy link

@TaskerJang TaskerJang commented May 22, 2025

Pull Request

πŸ’‘ PR μš”μ•½

Issue #1793μ—μ„œ μš”μ²­λœ clip_tokens ν•¨μˆ˜μ˜ λ‹¨μœ„ ν…ŒμŠ€νŠΈ μΆ”κ°€ 및 λ¬Έμ„œν™” κ°œμ„  μž‘μ—…μ„ μ™„λ£Œν–ˆμŠ΅λ‹ˆλ‹€. κΈ°μ‘΄ ν•¨μˆ˜μ˜ μ•ˆμ •μ„±μ„ ν•΄μΉ˜μ§€ μ•ŠμœΌλ©΄μ„œ ν…ŒμŠ€νŠΈ 컀버리지λ₯Ό λŒ€ν­ ν™•μž₯ν•˜κ³  개발자 κ²½ν—˜μ„ ν–₯μƒμ‹œμΌ°μŠ΅λ‹ˆλ‹€.

πŸ” μ£Όμš” 변경사항

  • tests/unittest/test_clip_tokens.py에 21개의 포괄적인 λ‹¨μœ„ ν…ŒμŠ€νŠΈ μΆ”κ°€
  • pr_agent/algo/utils.py의 clip_tokens ν•¨μˆ˜ docstring μ™„μ „ κ°œμ„ 
  • μ—£μ§€ μΌ€μ΄μŠ€, λ§€κ°œλ³€μˆ˜ μ‘°ν•©, μ˜ˆμ™Έ 처리 λ“± λͺ¨λ“  μ‹œλ‚˜λ¦¬μ˜€ ν…ŒμŠ€νŠΈ 컀버
  • 4개의 μ‹€μš©μ μΈ μ‚¬μš© μ˜ˆμ œμ™€ μƒμ„Έν•œ λ§€κ°œλ³€μˆ˜ μ„€λͺ… μΆ”κ°€
  • κΈ°μ‘΄ ν•¨μˆ˜ λ‘œμ§μ€ λ³€κ²½ν•˜μ§€ μ•Šμ•„ ν•˜μœ„ ν˜Έν™˜μ„± μ™„λ²½ μœ μ§€

πŸ”— μ—°κ΄€λœ 이슈

qodo-ai#1793

πŸ“Έ μŠ€ν¬λ¦°μƒ· (선택)

ν…ŒμŠ€νŠΈμ½”λ“œ

βœ… 체크리슀트

  • ν…ŒμŠ€νŠΈ μ½”λ“œλ₯Ό μž‘μ„±ν•˜μ˜€λ‚˜μš”? (21개 포괄적 ν…ŒμŠ€νŠΈ)
  • κ΄€λ ¨ λ¬Έμ„œλ₯Ό μ—…λ°μ΄νŠΈν•˜μ˜€λ‚˜μš”? (μ™„μ „ν•œ docstring)
  • Breaking Changeκ°€ μžˆλ‚˜μš”? (κΈ°μ‘΄ κΈ°λŠ₯ μ™„λ²½ ν˜Έν™˜)
  • μ½”λ“œ ν¬λ§·νŒ…κ³Ό 린트 검사λ₯Ό μ™„λ£Œν•˜μ˜€λ‚˜μš”?

πŸ™ 리뷰어 참고사항

  • κΈ°μ‘΄ ν•¨μˆ˜ λ‘œμ§μ€ μ „ν˜€ μˆ˜μ •ν•˜μ§€ μ•Šκ³  λ¬Έμ„œν™”λ§Œ κ°œμ„ ν•˜μ—¬ μ•ˆμ „μ„± 보μž₯
  • λͺ¨λ“  ν…ŒμŠ€νŠΈκ°€ ν†΅κ³Όν•˜μ—¬ κΈ°μ‘΄ κΈ°λŠ₯과의 ν˜Έν™˜μ„± 확인
  • Issue #1793의 μ„Έ κ°€μ§€ μ£Όμš” μš”κ΅¬μ‚¬ν•­ λͺ¨λ‘ μΆ©μ‘±

πŸ“‹ μΆ”κ°€ μ»¨ν…μŠ€νŠΈ (선택)

이번 μž‘μ—…μœΌλ‘œ clip_tokens ν•¨μˆ˜μ˜ 신뒰성이 크게 ν–₯μƒλ˜μ—ˆμŠ΅λ‹ˆλ‹€. 21개의 ν…ŒμŠ€νŠΈ μΌ€μ΄μŠ€λŠ” μœ λ‹ˆμ½”λ“œ 처리, κΈ΄ ν…μŠ€νŠΈ 처리, λ‹€μ–‘ν•œ λ§€κ°œλ³€μˆ˜ μ‘°ν•© 등을 λͺ¨λ‘ κ²€μ¦ν•˜λ©°, ν–₯ν›„ ν•¨μˆ˜ μˆ˜μ • μ‹œ νšŒκ·€ ν…ŒμŠ€νŠΈ 역할을 ν•  수 μžˆμŠ΅λ‹ˆλ‹€. λ˜ν•œ μ™„μ „νžˆ κ°œμ„ λœ docstring으둜 λ‹€λ₯Έ κ°œλ°œμžλ“€μ΄ ν•¨μˆ˜λ₯Ό μ˜¬λ°”λ₯΄κ²Œ μ‚¬μš©ν•  수 μžˆλ„λ‘ μΆ©λΆ„ν•œ κ°€μ΄λ“œλ₯Ό μ œκ³΅ν•©λ‹ˆλ‹€.

TaskerJang added 2 commits May 22, 2025 14:33
- 21개 ν…ŒμŠ€νŠΈ μΌ€μ΄μŠ€λ‘œ μ—£μ§€ μΌ€μ΄μŠ€ 및 λ§€κ°œλ³€μˆ˜ μ‘°ν•© 검증
- μœ λ‹ˆμ½”λ“œ, 특수문자, κΈ΄ ν…μŠ€νŠΈ 처리 ν…ŒμŠ€νŠΈ 포함
- μ˜ˆμ™Έ 상황 및 ν•˜μœ„ ν˜Έν™˜μ„± 확인
- λͺ¨λ“  λ§€κ°œλ³€μˆ˜μ— λŒ€ν•œ μ™„μ „ν•œ μ„€λͺ… μΆ”κ°€
- 4개의 μ‹€μš©μ μΈ μ‚¬μš© 예제 포함
- λ°˜ν™˜κ°’ 쑰건 및 λ™μž‘ λͺ…μ‹œ
- μ•ˆμ „ κ³„μˆ˜(0.9) 및 였λ₯˜ 처리 μ„€λͺ…
@KangmoonSeo
Copy link
Member

수고 λ§ŽμœΌμ…¨μŠ΅λ‹ˆλ‹€!! λ©”μΈν…Œμ΄λ„ˆκ°€ μ—„μ²­λ‚˜κ²Œ μ’‹μ•„ν•  것 κ°™μ•„μš”. γ…‹γ…‹γ…‹γ…‹γ…‹γ…Žγ…Ž
PR μ˜¬λ¦¬μ‹œκΈ° 전에 λ†“μΉ˜μ‹€ 수 μžˆλŠ” 뢀뢄을 짚자면..! ν•œκΈ€λ‘œ 된 컀밋 λ©”μ‹œμ§€λ₯Ό μ‹€ PRμ—μ„œλŠ” μ˜μ–΄λ‘œ μ˜¬λ €μ£Όμ‹œλ©΄ 졜고일 것 κ°™μ•„μš”. πŸ˜€


# Generated by CodiumAI

import pytest
Copy link
Member

Choose a reason for hiding this comment

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

Takser..God λ‹˜, μ½”λ“œ λ³΄λ©΄μ„œ 개인적으둜 κΆκΈˆν•œ 점이 μƒκ²Όμ–΄μš” :)
test_clip_tokens.py처럼 κΌΌκΌΌν•œ ν…ŒμŠ€νŠΈ μΌ€μ΄μŠ€λ“€μ„ 보면
κ΅¬μƒν•˜μ‹€ λ•Œ μ–΄λ–€ 것뢀터 μ‹œμž‘ν•΄μ„œ 살을 λΆ™μ—¬λ‚˜λŠ”μ§€ κ·Έ 과정이 κΆκΈˆν•©λ‹ˆλ‹€.

ν•¨μˆ˜ 핡심 κΈ°λŠ₯을 λ¨Όμ € 작고,
κ·Έλ‹€μŒμ— μž…λ ₯ κ°’ λ– μ˜¬κ³ ,
μ˜΅μ…˜λ³„ λ™μž‘μ΄λ‚˜ μ˜ˆμ™Έ 처리 순으둜 μ²΄κ³„μ μœΌλ‘œ λ§Œλ“€μ–΄κ°€μ‹œλŠ” 건지
μ•„λ‹ˆλ©΄ 개인이 주둜 μ‚¬μš©ν•˜λŠ” 방식이 μžˆλŠ”μ§€ λ“±λ“±...
λΉŒλ“œμ—… 과정에 λŒ€ν•œ μƒκ°μ˜ 흐름 νŒμ„ 쑰금만 κ³΅μœ ν•΄ μ£Όμ‹€ 수 μžˆμ„κΉŒμš”?
(μ•„λ¬΄λž˜λ„ μ œκ°€ μ •λŸ‰μ μΈ κ²½ν—˜μ΄ μ λ‹€λ³΄λ‹ˆ... μƒκ°μ˜ 흐름도가 κΆκΈˆν•©λ‹ˆλ‹€!)

항상 많이 λ°°μ›λ‹ˆλ‹€πŸ™πŸ”₯πŸ”₯πŸ”₯

Copy link
Author

@TaskerJang TaskerJang May 23, 2025

Choose a reason for hiding this comment

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

1. 일단 ν•¨μˆ˜ 보고 "λ­ν•˜λŠ” ν•¨μˆ˜μ§€?"

def clip_tokens(text: str, max_tokens: int, add_three_dots=True...)

토큰 자λ₯΄λŠ” ν•¨μˆ˜κ΅¬λ‚˜. 그럼 일단 기본적인 κ±°λΆ€ν„° ν…ŒμŠ€νŠΈν•΄λ³΄μž.

2. κΈ°μ‘΄ ν…ŒμŠ€νŠΈ 파일 λ΄€λ”λ‹ˆ 2κ°œλ°–μ— μ—†μŒ

def test_clip(self):
    text = "line1\nline2..."
    # μ΄κ²ƒλ§Œ μžˆλ„€?

이거 λ„ˆλ¬΄ λΆ€μ‘±ν•˜λ‹€ μ‹Άμ–΄μ„œ Issue λ‚΄μš© λ‹€μ‹œ λ΄€μ–΄μš”.

3. Issueμ—μ„œ μš”κ΅¬ν•œ 것듀 체크

  • "edge cases" β†’ μ•„ 빈 λ¬Έμžμ—΄, 음수 같은 κ±° ν…ŒμŠ€νŠΈν•΄μ•Όκ² λ‹€
  • "parameter combinations" β†’ add_three_dots, delete_last_line μ‘°ν•©λ“€
  • "error handling" β†’ μ˜ˆμ™Έ 상황듀

4. μ‹€μ œλ‘œλŠ” ν•˜λ‚˜μ”© λ§Œλ“€λ©΄μ„œ "μ•„ 이것도 ν…ŒμŠ€νŠΈν•΄μ•Όκ² λ„€" ν•˜λ©΄μ„œ μΆ”κ°€

def test_empty_input_text(self):  # 일단 빈 λ¬Έμžμ—΄
def test_negative_max_tokens(self):  # 음수 λ„£μœΌλ©΄ μ–΄λ–»κ²Œ 될까?

μ΄λ ‡κ²Œ ν•˜λ‚˜μ”© λ§Œλ“€λ‹€κ°€ "μ•„ μœ λ‹ˆμ½”λ“œλŠ”?" "0으둜 λ‚˜λˆ„κΈ°λŠ”?" 이런 μ‹μœΌλ‘œ 계속 λ– μ˜¬λΌμ„œ μΆ”κ°€ν•œ κ±°μ˜ˆμš”.

5. μ†”μ§νžˆ 21κ°œλ‚˜ 될 쀄 λͺ°λžμŒ γ…‹γ…‹

μ²˜μŒμ—” 10개 정도 μƒκ°ν–ˆλŠ”λ° ν•˜λ‹€λ³΄λ‹ˆ "이것도 ν…ŒμŠ€νŠΈν•΄μ•Όκ² λ„€" ν•˜λ©΄μ„œ 계속 λŠ˜μ–΄λ‚¬μ–΄μš”.

특히 mock ν…ŒμŠ€νŠΈ 뢀뢄은... 사싀 μ’€ ν—€λ§Έμ–΄μš”. TokenEncoder μ–΄λ–»κ²Œ mock ν•΄μ•Ό ν•˜λ‚˜ κ³ λ―Όν•˜λ©΄μ„œ μ—¬λŸ¬ 번 μ‹œλ„ν•΄λ΄€κ³ μš”.

μ§„μ§œ μ™„λ²½ν•˜κ²Œ κ³„νš μ„Έμš°κ³  ν•œ 게 μ•„λ‹ˆλΌ λ§Œλ“€λ©΄μ„œ "μ•„ 이것도, 저것도" ν•˜λ©΄μ„œ 덧뢙인 거라 πŸ˜…

Copy link
Member

Choose a reason for hiding this comment

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

κ³ μŠ€νŠΈλ°”λ‘‘μ™•μ²˜λŸΌ
1->2->3->4->5 μ΄λ ‡κ²Œ λΉ™μ˜ν•˜λ©΄μ„œ λ”°λΌκ°€λ‹ˆκΉŒ 이해가 κ°”μŠ΅λ‹ˆλ‹€..
Issueμ—μ„œ μš”κ΅¬ν•œ 것듀 체크 이 뢀뢄도 μ€‘μš”ν•˜κ³ 
κ²°κ΅­ ν•˜λ‚˜ ν•˜λ‚˜ λΆ™μ—¬λ‚˜κ°€λŠ”κ²Œ λ°©λ²•μ΄μ—ˆκ΅°μš”!
(킬링 ν¬μΈνŠΈγ…‹γ…‹) - > μ†”μ§νžˆ 21κ°œλ‚˜ 될 쀄 λͺ°λžμŒ

ν…μŠ€νŠΈ μ•ˆμž˜λ¦¬κ³  μ œλŒ€λ‘œ λ‚˜μ˜€λŠ”μ§€, ν…μŠ€νŠΈ 잘리고+ λΆ™λŠ”μ§€, 0으둜 λ‚˜λˆ μ§€λŠ” 였λ₯˜ ν…ŒμŠ€νŠΈ λ“±...
λ°”λ‘œ λ‚˜μ˜¨κ²Œ μ•„λ‹ˆλΌ ν•œλ•€ ν•œλ•€, μ—¬λŸ¬ 번 κ³ λ―Όμ΄λž‘ μ‹œλ„κ°€ λ“€μ–΄κ°€μ„œ λ‚˜μ˜¨κ±°κ΅°μš”
덕뢄에 많이 λ°°μ›Œκ°‘λ‹ˆλ‹€! ✍️

(ν™œλ™ 이후에도 λΈ”λ‘œκ·Έ κΈ€ λͺ°λž˜ ν™•μΈν•˜κ² μŠ΅λ‹ˆλ‹€..)

result = clip_tokens(text, max_tokens)
expected_results = 'line1\nline2\nline3\n\n...(truncated)'
assert result == expected_results
assert result == expected_results
Copy link

@Kkan9ma Kkan9ma May 25, 2025

Choose a reason for hiding this comment

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

μ†Œμ†Œν•˜μ§€λ§Œ EOF 도 챙겨주십사.. μ½”λ©˜νŠΈ λ‚¨κ²¨λ΄…λ‹ˆλ‹€ :)

HOPARKSUNG added a commit that referenced this pull request May 26, 2025
HOPARKSUNG pushed a commit that referenced this pull request May 26, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants