혼합 광고·채팅 흐름·Tuist 캐시 개선과 전체 모듈 문서화 - #44
Conversation
Rejected: Dashboard 인증 실패 시 캐시를 강제로 끄는 흐름 | 로컬 캐시 재사용을 방해한다.
탐색은 첫 Kakao 광고 이후 콘텐츠 3개마다 서버 광고를 순환 표시한다. 큐레이션과 마이페이지는 서버 광고와 Kakao를 교대하고 빈 응답에는 Kakao를 표시한다.
42개 모듈의 직접 구현·Interface 의존성을 SVG로 생성하고 README에서 펼쳐 볼 수 있게 한다.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
새 탐색 목록을 요청할 때 표시 위치별 노출 기록을 비우고 서버 광고 오류를 로그로 남긴다.
채팅 코디네이터를 앱 조립 계층으로 옮기고 투표 화면의 뒤로가기와 스크롤을 보완한다. 시작 노드가 null인 시나리오는 첫 노드로 매핑한다. 현재 Tuist 및 패키지 설정과 다이어그램 검증 자료를 함께 보존한다.
Roy-wonji
changed the title
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7dee2d1d30
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| _ = try await client.send( | ||
| AdsService.impressions(body: AdsImpressionsRequest(codes: codes)), | ||
| as: String.self | ||
| ) |
There was a problem hiding this comment.
When the impressions endpoint returns a normal no-content success or an envelope whose data is null, requesting String.self makes NetworkClient.unwrap throw dataMissing, because missing payloads are accepted only for PickeEmptyResponse. In Hifi this removes the deduplication marker and can resend an already accepted impression whenever visibility changes, inflating ad metrics; use the request's default PickeEmptyResponse response instead.
Useful? React with 👍 / 👎.
| public enum FeedAdUseCaseDependency: TestDependencyKey { | ||
| public static var testValue: FeedAdInterface { MockFeedAdClient() } | ||
| } |
There was a problem hiding this comment.
Replace this standalone FeedAdUseCaseDependency key with FeedAdUseCaseImpl conforming directly to DependencyKey, including explicit live, test, and preview values. The current parallel interface-level key preserves a duplicate repository/use-case registration topology rather than the repository-hidden, implementation-backed use-case dependency required for new domain IO.
AGENTS.md reference: AGENTS.md:L641-L643
Useful? React with 👍 / 👎.
| case let .adVisible(code, itemID): | ||
| guard state.ads.contains(where: { $0.code == code }), | ||
| state.reportedAdCodesByItemID[itemID, default: []].insert(code).inserted else { return .none } |
There was a problem hiding this comment.
When the user leaves Explore and later returns, AppMainTabCoordinator retains the existing Hifi route state, while .onAppear fetches the feed and ads again without clearing reportedAdCodesByItemID. If the same battle/ad pair is displayed after returning or navigating back, this insertion returns false and the new exposure is never reported; clear or scope the deduplication set to the current appearance/feed load.
Useful? React with 👍 / 👎.
Rejected: String 응답 강제 | 성공한 집계를 실패로 오인해 중복 전송할 수 있음
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| public let network: String | ||
| public let title: String | ||
| public let subtitle: String | ||
| public let imageURL: String |
There was a problem hiding this comment.
🟡 [P3] Minor
imageURL과 clickURL 속성은 String 대신 URL 타입으로 선언하는 것이 더 안전하고, 타입 안정성을 높이며, URL 객체에서 제공하는 유용한 기능을 활용할 수 있습니다.
| public let subtitle: String | ||
| public let imageURL: String | ||
| public let ctaText: String | ||
| public let clickURL: String |
There was a problem hiding this comment.
🟡 [P3] Minor
imageURL과 clickURL 속성은 String 대신 URL 타입으로 선언하는 것이 더 안전하고, 타입 안정성을 높이며, URL 객체에서 제공하는 유용한 기능을 활용할 수 있습니다.
| public static var testValue: FeedAdInterface { MockFeedAdClient() } | ||
| } | ||
|
|
||
| public enum FeedAdRepositoryDependency: TestDependencyKey { |
There was a problem hiding this comment.
🟠 [P2] Major
FeedAdRepositoryDependency는 AdDomain의 Interface 모듈이 아닌, Data 계층(AdData 또는 AdDataInterface)에 정의되어야 합니다. Domain 계층은 Repository의 구체적인 존재나 인터페이스에 직접 의존해서는 안 되며, 오직 UseCase 인터페이스에만 의존해야 합니다. 현재 FeedAdInterface가 UseCase와 Repository 인터페이스 역할을 동시에 수행하고 있어 모듈 아키텍처의 책임이 불분명합니다.
| product: .framework, | ||
| settings: .settings(), | ||
| dependencies: [ | ||
| .serviceAssembly, |
There was a problem hiding this comment.
🟠 [P2] Major
AdDomain이 .serviceAssembly에 의존하는 것은 모듈 계층 구조 위반입니다. Domain 계층은 Data 계층의 추상화된 인터페이스(DataInterface)에만 의존해야 하며, ServiceAssembly와 같이 Network 또는 APIEndpoint에 가까운 하위 계층에 직접 의존해서는 안 됩니다. 이는 Domain과 Data 간의 단방향 의존성 규칙을 위반합니다.
| public static var liveValue: FeedAdInterface { FeedAdUseCaseImpl() } | ||
| } | ||
|
|
||
| extension FeedAdRepositoryDependency: DependencyKey { |
There was a problem hiding this comment.
🟠 [P2] Major
FeedAdRepositoryDependency의 liveValue 구현은 AdDomain 모듈이 아닌 Data 계층(AdData 모듈)에 있어야 합니다. AdDomain은 UseCase 구현을 담당하며, Repository 구현에 대한 지식을 가져서는 안 됩니다. 이는 모듈 아키텍처의 책임 분리 원칙을 위반합니다.
|
|
||
| struct AdDomainTests { | ||
| @Test func 광고_DTO를_도메인으로_매핑한다() { | ||
| let dto = FeedAdDTO( |
There was a problem hiding this comment.
🟠 [P2] Major
FeedAdDTO에 대한 테스트는 Data 계층에 DTO가 위치해야 한다는 원칙에 따라 AdDataTests와 같은 Data 계층 테스트 모듈에서 수행되어야 합니다. 현재 AdDomainTests에서 FeedAdDTO를 직접 테스트하는 것은 DTO-to-Entity 매핑이 Data 계층에 머물러야 한다는 모듈 아키텍처 원칙을 위반합니다.
| )) | ||
| } | ||
|
|
||
| @Test func 광고_조회_요청을_매핑한다() throws { |
There was a problem hiding this comment.
🟠 [P2] Major
API 요청 매핑 테스트는 APIEndpoint 모듈의 테스트에서 수행되어야 합니다. Domain 계층 테스트에서 API 요청 세부 사항을 직접 확인하는 것은 Domain이 Network 계층의 구현 세부 사항에 의존하게 만드는 모듈 아키텍처 위반입니다.
| #expect(request.url?.query?.contains("size=20") == true) | ||
| } | ||
|
|
||
| @Test func 광고_노출은_codes_JSON을_POST한다() throws { |
There was a problem hiding this comment.
🟠 [P2] Major
API 요청 매핑 테스트는 APIEndpoint 모듈의 테스트에서 수행되어야 합니다. Domain 계층 테스트에서 API 요청 세부 사항을 직접 확인하는 것은 Domain이 Network 계층의 구현 세부 사항에 의존하게 만드는 모듈 아키텍처 위반입니다.
| .service(.analytics, .interface), | ||
| ], | ||
| hasTests: true, | ||
| hasTests: false, |
There was a problem hiding this comment.
🟠 [P2] Major
Feature/Ad 모듈은 복잡한 광고 노출 및 클릭 로직, 그리고 서버 광고와 카카오 광고를 번갈아 표시하는 기능을 포함하고 있습니다. 이러한 복잡성을 고려할 때, hasTests: false로 설정하여 테스트를 비활성화하는 것은 코드 변경에 대한 안정성을 보장하기 어렵습니다. Feature 모듈에 대한 테스트를 추가하여 기능의 정확성을 검증해야 합니다.
| .package(url: "https://github.com/Roy-wonji/TCAFlow.git", exact: "1.1.8"), | ||
| // SQLiteData 1.11.0의 section API가 요구하는 Select.From: Table 제약을 유지한다. | ||
| .package(url: "https://github.com/pointfreeco/swift-structured-queries", exact: "0.36.0"), | ||
| .package(url: "https://github.com/Roy-wonji/TCAFlow.git", branch: "main"), |
There was a problem hiding this comment.
🟠 [P2] Major
TCAFlow 패키지 의존성을 branch: "main"으로 지정하는 것은 빌드 안정성을 저해할 수 있습니다. main 브랜치는 예고 없이 변경될 수 있으며, 이는 예기치 않은 빌드 실패나 동작 변경으로 이어질 수 있습니다. 의존성 관리를 위해 특정 버전(exact) 또는 커밋 해시를 사용하는 것이 권장됩니다.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 072111ca02
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| func recordImpression(for ad: FeedAd) { | ||
| guard scenePhase == .active, !didRecordImpression else { return } | ||
| didRecordImpression = true |
There was a problem hiding this comment.
When the impression POST fails while the device is offline or the request otherwise never reaches the server, didRecordImpression remains true for the rest of this appearance, so subsequent visibility or foreground callbacks are discarded and the impression is permanently lost. Reset the flag in the failure path, matching HifiFeature's impressionResponse handling, so the Curation and Profile placements can retry when they become visible again.
Useful? React with 👍 / 👎.
1 participant
변경 이유
홈 외 광고 지면에서 카카오 광고와 서버 광고를 함께 제공하고, 실제 표시된 광고만 노출로 집계합니다.
검증
검증 범위
전체 앱 빌드는 로컬 Xcode의 clang 정보 수집 단계에서 진행되지 않아 확인하지 못했습니다. 실제 기기 광고 동작과 TCA 테스트 실행은 미검증입니다. 전체 도메인 독립 HTML은 일부 데스크톱 너비에서 가로 넘침이 남아 있으며, GitHub README는 별도 SVG를 표시합니다.