Skip to content

[Fix] 로드맵 장소 중복 문제 해결#108

Merged
iamb0ttle merged 1 commit into
mainfrom
feat/roadmap-dedupe-place-results
May 1, 2026
Merged

[Fix] 로드맵 장소 중복 문제 해결#108
iamb0ttle merged 1 commit into
mainfrom
feat/roadmap-dedupe-place-results

Conversation

@iamb0ttle

@iamb0ttle iamb0ttle commented May 1, 2026

Copy link
Copy Markdown
Member

1. 개요

2. 작업 내용

  • finalize 단계에서 place_id 기준 전역 중복 제거를 추가했습니다.
  • 중복 후보밖에 남지 않은 경우에도 가능한 대체 후보를 우선 선택하도록 했습니다.
  • skeleton 생성 및 복구 프롬프트에 동일 place_id 중복 금지 지시를 추가했습니다.
  • 관련 단위 테스트를 추가해 날짜를 넘어가는 중복 재사용 케이스를 검증했습니다.

3. AI 사용 및 검증

  • AI가 생성한 코드 포함
  • 100% 수동 작성
  • 검증 방법: git diff --check
  • 테스트: 현재 셸 환경에 uv/pytest가 없어 자동 테스트는 실행하지 못했습니다.

4. 스크린샷

  • 해당 없음

5. 체크리스트

  • uv run pre-commit run --all-files를 실행했는가?
  • 관련 테스트를 실행했는가? (현재 환경에서는 실행 불가)
  • 불필요한 print나 디버그 코드를 제거했는가?

Summary by CodeRabbit

릴리스 노트

  • 버그 수정

    • 루트 계획 시 여러 날짜와 슬롯에서 동일한 장소가 중복으로 추천되는 문제를 개선했습니다. 시스템이 더 다양한 장소를 선택하도록 로직을 강화했으며, 필요시에만 대체 장소를 사용하게 됩니다.
  • 테스트

    • 중복 제거 기능의 안정성을 검증하는 새로운 테스트가 추가되었습니다.

@iamb0ttle iamb0ttle requested a review from zweadfx as a code owner May 1, 2026 09:48
@coderabbitai

coderabbitai Bot commented May 1, 2026

Copy link
Copy Markdown

개요

이 변경사항은 Google Places API 후보를 사용할 때 선택된 장소에 대한 중복 제거 로직을 구현합니다. place_id를 기반으로 생성된 키를 사용하여 슬롯 간 중복을 방지하고, 프롬프트에 제약 조건을 추가하며, 테스트를 확장합니다.

변경 사항

Cohort / File(s) 요약
최종화 로직 중복 제거
app/graph/roadmap/nodes/finalize.py
_prepare_final_context의 장소 선택 로직을 수정하여 place_id(또는 정규화된 이름 + 주소)를 기반으로 한 키를 사용해 선택된 장소를 중복 제거합니다. _select_place_in_region 함수를 수정하여 선택된 장소, 중복 제거 키, 대체 사용 여부 플래그를 반환하고, route_dedupe 작업 로그를 생성합니다.
스켈레톤 생성 프롬프트 업데이트
app/graph/roadmap/nodes/skeleton.py
초기 스켈레톤 생성 및 JSON 수리 프롬프트에 Google Place API 후보를 사용할 때 동일한 place_id를 다른 슬롯 및 날짜에 재사용하지 않도록 하는 제약 조건을 추가합니다.
교차 일자 중복 제거 테스트
tests/test_roadmap_generation_simplified_pipeline.py
_prepare_final_context의 교차 일자 중복 제거를 검증하는 새로운 테스트를 추가하고, 기존 세그먼트 프롬프트 테스트를 확장하여 시스템 및 사용자 메시지의 "동일 place_id" 제약 조건 참조를 검증합니다.

추정 코드 리뷰 노력

🎯 3 (Moderate) | ⏱️ ~20 분

관련 이슈

관련 PR

제안 레이블

bug

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 40.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title '[Fix] 로드맵 장소 중복 문제 해결' accurately summarizes the main objective of the pull request, which is to fix duplicate roadmap places issue.
Description check ✅ Passed 풀 리퀘스트 설명이 필요한 모든 섹션을 포함하고 있으며, 구체적인 작업 내용과 검증 방법이 명시되어 있습니다.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/roadmap-dedupe-place-results

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@iamb0ttle iamb0ttle changed the title fix: prevent duplicate roadmap places [Fix] 로드맵 장소 중복 문제 해결 May 1, 2026
@iamb0ttle iamb0ttle self-assigned this May 1, 2026

@coderabbitai coderabbitai Bot 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.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@app/graph/roadmap/nodes/finalize.py`:
- Around line 73-95: The loop currently forces truthiness of place_key which
skips candidates with place_key=None and can return an already-used key as
fallback; update the checks that currently read "if place_key and place_key not
in used_place_keys" (the early-return condition and the similar check before
assigning fallback_place) to instead test membership only (e.g., "if place_key
not in used_place_keys") so candidates with None keys are considered as
fallback, and change the final duplicate flag expression "bool(fallback_key and
fallback_key in used_place_keys)" to "bool(fallback_key in used_place_keys)" so
the returned boolean correctly reflects whether the fallback_key is already
used; locate these changes around uses of _place_dedupe_key, used_place_keys,
region_bbox.contains, and the fallback_place/fallback_key logic.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 3c556347-e729-413d-ae9d-139ca75b1a0e

📥 Commits

Reviewing files that changed from the base of the PR and between 4b1bbd5 and 66d591b.

📒 Files selected for processing (3)
  • app/graph/roadmap/nodes/finalize.py
  • app/graph/roadmap/nodes/skeleton.py
  • tests/test_roadmap_generation_simplified_pipeline.py

Comment thread app/graph/roadmap/nodes/finalize.py
@iamb0ttle iamb0ttle merged commit 7a453c0 into main May 1, 2026
3 checks passed
@iamb0ttle iamb0ttle deleted the feat/roadmap-dedupe-place-results branch May 1, 2026 09:55
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.

1 participant