feat: 무인자 비교 본인 고정+랜덤 선정 + /경험치 대상 지정 (ADR-0008 개정) - #58
Conversation
- cap_by_level → select_with_self: 실행 본인 무조건 포함 + 나머지 random.sample 비복원 - 본인 풀 부재 시 조용히 랜덤 10명 + 사유별 안내(미등록/키 미등록/챌 캐릭 없음) - 무인자 푸터 상시화: N≤10 전원(N명) / N>10 본인 포함 랜덤 10명 문구 분기 - /아이템 본인 맨 앞 정렬(_self_first), 나머지 랜덤/입력 순서 유지 - /아이템·/유니온·/스타포스·/잠재 description 카피 통일
- 지정 시 해당 유저 대표 캐릭터만 순위판+그래프(build_specified_payload) - 미등록·데이터 없는 지정 유저 제외 + 'N명은 미등록/데이터 없음' 안내, 전원 불가 시 안내만 - 무인자·매일 10시 발송·DM 구독 payload 불변(requested_users=None 기본값으로 무영향) - description 카피: 대상 지정 시 최대 5명만 비교 명시
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThis PR replaces the deterministic level-top-K target selection for unspecified comparisons with a self-included random selection helper ( ChangesRandom fanout and targeted leaderboard
Estimated code review effort: 3 (Moderate) | ~30 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant Handler
participant select_with_self
participant fanout_note
User->>Handler: invoke comparison command (no members)
Handler->>select_with_self: pass targets, self_id, cap
select_with_self-->>Handler: selected targets, total, self_included
Handler->>fanout_note: total, cap
fanout_note-->>Handler: footer text
Handler-->>User: embed with self_note/note footer
sequenceDiagram
participant User
participant handle_leaderboard
participant build_specified_payload
participant broadcast
User->>handle_leaderboard: /경험치 member1..member5
handle_leaderboard->>build_specified_payload: user_ids, realm
build_specified_payload->>broadcast: build_targets_payload(requested_users)
broadcast-->>build_specified_payload: payload or None
build_specified_payload-->>handle_leaderboard: payload
handle_leaderboard-->>User: comparison embed or _MSG_TARGETS_NONE
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
maple_mate/bot/comparison.py (1)
178-201: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueSelection logic matches the self-first + non-replacement random contract.
Verified against the test suite (
test_select_self_first_and_random_rest,test_select_self_absent_fills_full_cap,test_select_random_is_deterministic_with_seed): self is fixed at the front when present, remainder is drawn viarandom.samplewithout replacement, and behavior is deterministic when anrngis injected.One theoretical edge case worth a defensive thought: if
targetsever contained more than one entry with the sameself_id(duplicate rows for the same user),take = cap - len(self_targets)could underflow, andself_targets + chosencould exceedcap. This relies on the invariant that the resolved target pool has at most one row perdiscord_user_id, which appears to hold upstream — flagging only as a defensive note, not a live bug.💡 Optional defensive guard
self_targets = [t for t in targets if t.discord_user_id == self_id] others = [t for t in targets if t.discord_user_id != self_id] self_included = bool(self_targets) - take = cap - len(self_targets) + take = max(cap - len(self_targets), 0) chosen = picker.sample(others, take) - return self_targets + chosen, total, self_included + return (self_targets[:cap] if len(self_targets) > cap else self_targets) + chosen, total, self_included🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@maple_mate/bot/comparison.py` around lines 178 - 201, The selection contract is correct, but add a defensive guard in select_with_self for duplicate entries with the same discord_user_id. Ensure self_targets is deduplicated or capped so that take cannot become negative and the returned list never exceeds cap, even if targets contains multiple rows for self_id. Keep the existing self-first behavior and random.sample logic intact, and update the select_with_self path to handle this edge case safely.maple_mate/character/commands.py (1)
380-393: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicated note/self_note/footer wiring across 4 command handlers.
The
select_with_self→fanout_note→self_note→ footer-prepend loop pattern here is repeated almost verbatim inhandle_starforce(history/commands.py),handle_potential(potential_commands.py), andhandle_union(union/commands.py), differing only in thenoun/absent-message text. Consider extracting a shared helper incomparison.py, e.g.resolve_unspecified_selection(targets, self_id, *, absent_message, noun="등록자") -> (selected, note, self_note)and aprepend_notes(footer, *lines)helper, to avoid drift across the four call sites as this logic evolves.♻️ Illustrative refactor sketch
# maple_mate/bot/comparison.py def resolve_unspecified_selection(targets, self_id, *, absent_message, noun="등록자"): selected, total, self_included = select_with_self(targets, self_id) note = fanout_note(total, noun=noun) self_note = None if self_included else absent_message return selected, note, self_note def prepend_notes(footer, *lines): for line in lines: if line: footer = f"{line}\n{footer}" return footer- note: str | None = None - self_note: str | None = None - if not members: - targets, total, self_included = comparison.select_with_self( - targets, interaction.user.id - ) - note = comparison.fanout_note(total) - if not self_included: - self_note = ( - "본인은 챌린저스 캐릭터가 없어 포함되지 않았어요." - if realm is Realm.CHALLENGERS - else "본인은 미등록이라 포함되지 않았어요. `/캐릭터등록` 부터 해주세요!" - ) + note: str | None = None + self_note: str | None = None + if not members: + absent_message = ( + "본인은 챌린저스 캐릭터가 없어 포함되지 않았어요." + if realm is Realm.CHALLENGERS + else "본인은 미등록이라 포함되지 않았어요. `/캐릭터등록` 부터 해주세요!" + ) + targets, note, self_note = comparison.resolve_unspecified_selection( + targets, interaction.user.id, absent_message=absent_message + )Also applies to: 405-411
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@maple_mate/character/commands.py` around lines 380 - 393, The self-selection note/footer wiring in this handler is duplicated across the command handlers, so extract the repeated select_with_self/fanout_note/self_note flow into a shared helper in comparison.py and reuse it here. Update the handle flow to call a helper like resolve_unspecified_selection for targets/self_included handling, and centralize footer prepending so the noun and absent-message variations are passed as arguments instead of reimplemented in each command.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@maple_mate/bot/comparison.py`:
- Around line 178-201: The selection contract is correct, but add a defensive
guard in select_with_self for duplicate entries with the same discord_user_id.
Ensure self_targets is deduplicated or capped so that take cannot become
negative and the returned list never exceeds cap, even if targets contains
multiple rows for self_id. Keep the existing self-first behavior and
random.sample logic intact, and update the select_with_self path to handle this
edge case safely.
In `@maple_mate/character/commands.py`:
- Around line 380-393: The self-selection note/footer wiring in this handler is
duplicated across the command handlers, so extract the repeated
select_with_self/fanout_note/self_note flow into a shared helper in
comparison.py and reuse it here. Update the handle flow to call a helper like
resolve_unspecified_selection for targets/self_included handling, and centralize
footer prepending so the noun and absent-message variations are passed as
arguments instead of reimplemented in each command.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 91ce0c16-4a46-4f4b-9094-bce3b79e970e
📒 Files selected for processing (11)
docs/adr/0008-unspecified-comparison-fanout-cap.mddocs/random-fanout-work-order.mdmaple_mate/bot/comparison.pymaple_mate/character/commands.pymaple_mate/history/commands.pymaple_mate/history/potential_commands.pymaple_mate/leaderboard/broadcast.pymaple_mate/leaderboard/commands.pymaple_mate/union/commands.pytests/test_fanout_cap.pytests/test_leaderboard_commands.py
|
@coderabbitai 두 니트픽 검토 결과, 둘 다 의도적으로 미적용합니다. 1. 2. 4개 핸들러 배선 헬퍼 추출 — 미적용. 드리프트가 위험한 실제 로직(선정 규칙·푸터 문구)은 이미 |
|
Tip For best results, initiate chat on the files or code changes.
두 니트픽 모두 철회하겠습니다. (=^･ω･^=) ✏️ Learnings added
|
- 튜토리얼 10번 화면 제목·본문: 공평성 기조(본인 항상 포함 + 랜덤) + /경험치 Top 10 예외 명시 - TutorialDemos TargetDemo 라벨 동기 - commands.json: 아이템·유니온·스타포스·잠재 요약문 갱신(스타포스·잠재 '키 등록자' 유지), 경험치 tip에 대상 지정(최대 5명, 대표 캐릭터만) 추가 - 봇 PR #58(select_with_self·/경험치 대상 지정)과 짝, 드리프트 가드·빌드 그린
|
Tip For best results, initiate chat on the files or code changes.
(=^･ω･^=) ﾉ ✏️ Learnings added
|
1 participant
요약
그릴링 확정(2026-07-04, docs/random-fanout-work-order.md) 봇 PR1. 튜토리얼의 "대표 레벨 상위 최대 10명" 규칙이 상위권만 반복 노출되는 불공평을 해소한다.
1. 무인자 선정 규칙 개정 — ADR-0008 부분개정
cap_by_level(레벨 상위) →select_with_self(본인 무조건 포함 + 나머지 random.sample 비복원). 매 실행 독립, 테스트만 rng 주입.2. /경험치 — 랜덤 미적용, 대상 지정만 추가
requested_users=None기본값으로 payload 불변).3. 표시 정렬 — 명령별 핵심 지표
_self_first) + 나머지 랜덤/입력 순서.4. 무인자 푸터 상시화
5. 카피 통일
검증
uv run pytest -q— 824 passed, 1 deselected (베이스라인 815 + 순증 9)uv run ruff check ./ruff format --check .— clean후속
사이트 PR2(튜토리얼 10번 화면·TutorialDemos·commands.json)가 뒤따름 — Vercel 자동배포라 봇 배포와 순서 맞춰 머지.
Summary by CodeRabbit
New Features
/경험치.Bug Fixes
Documentation