Skip to content

feat(types): add CrudFilter TypedDict definitions#44

Merged
ywkim merged 18 commits into
mainfrom
refactor/reduce-any-types
Jan 30, 2026
Merged

feat(types): add CrudFilter TypedDict definitions#44
ywkim merged 18 commits into
mainfrom
refactor/reduce-any-types

Conversation

@ywkim

@ywkim ywkim commented Jan 29, 2026

Copy link
Copy Markdown
Contributor

요약

  • CrudFilter TypedDict 추가 (Refine 호환)
  • protocols.pytypes.py 이름 변경 (Python 커뮤니티 관례)
  • _build_filter_conditions 반환 타입을 list[ColumnElement[bool]]로 명시

변경 사항

새 타입 정의 (types.py)

  • LogicalFilter: 단일 필드 필터 (field, operator, value)
  • ConditionalFilter: OR/AND 조합 필터
  • CrudFilter: LogicalFilter | ConditionalFilter
  • FilterOperator, ConditionalOperator: Literal 타입

타입 개선

  • filters: list[dict[str, Any]]list[CrudFilter]
  • _build_filter_conditions 반환: list[Any]list[ColumnElement[bool]]

Breaking Changes

  • from aioia_core.protocols import ...from aioia_core.types import ...
  • from aioia_core import ...는 영향 없음 (re-export 유지)

체크리스트

  • 테스트 통과
  • pyright 타입 체크 통과

🤖 Generated with Claude Code

ywkim and others added 8 commits January 29, 2026 18:43
Refine 호환 filter 타입 정의 추가:
- LogicalFilter: 단일 필드 필터 (field, operator, value)
- ConditionalFilter: OR/AND 조합 필터
- CrudFilter: LogicalFilter | ConditionalFilter

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
filters 파라미터 타입을 list[dict[str, Any]]에서 list[CrudFilter]로 변경

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- filters 파라미터: list[dict[str, Any]] → list[CrudFilter]
- _build_filter_conditions 반환: list[Any] → list[ColumnElement[bool]]

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- _decamelize_filter_fields: list[dict[str, Any]] → list[CrudFilter]
- DeleteResponse.data: dict[str, Any] → dict[str, bool]

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
dict[str, bool] 변경 시 실제 반환 구조 {id, type, metadata}와 불일치

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
pyright가 isinstance 체크만으로 타입을 추론함

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
아키텍처 원칙 (DIP) 적용:
- Protocol과 관련 타입이 같은 파일에 위치
- 올바른 의존성 방향: repositories.py → protocols.py

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Python 커뮤니티 관례 (FastAPI, Pydantic) 준수

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

@gemini-code-assist gemini-code-assist 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.

Code Review

This PR significantly enhances type safety by introducing TypedDict definitions for CrudFilter, renaming protocols.py to types.py, and explicitly typing the return of _build_filter_conditions. However, the implementation of dynamic filtering and sorting lacks proper validation of field names, which could lead to information exposure or Denial of Service. Additionally, the recursive processing of nested filters is susceptible to stack overflow attacks due to the absence of a recursion depth limit. To further improve type safety, consider clarifying the TypedDict definitions for LogicalFilter and ConditionalFilter.

Comment thread python/aioia_core/types.py
Comment thread python/aioia_core/repositories.py
Comment thread python/aioia_core/repositories.py
Comment thread python/aioia_core/fastapi/base_crud_router.py
ywkim and others added 4 commits January 29, 2026 23:00
Union TypedDict에서 한쪽에만 존재하는 키 접근 시 object로 추론되는 문제 해결.
isinstance(field, str) 체크로 런타임 검증과 타입 narrowing 동시 수행.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
spread 연산({**filter_item, ...})이 TypedDict 타입 정보를 잃는 문제 해결.
- processed_filters를 list[Any]로 처리
- 반환 시 cast(list[CrudFilter], ...)로 함수 계약 명시
- filter_item.get("field", "")로 Union TypedDict 키 접근

cast 필요 이유: TypedDict Union에서 spread 시 operator 타입이
FilterOperator | ConditionalOperator로 병합되어 정확한 멤버 결정 불가.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
dict 리터럴이 TypedDict로 자동 추론되지 않는 문제 해결.
filters 변수에 list[CrudFilter] 타입 주석 추가.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
total=False TypedDict에서 optional 키 접근 시 .get() 사용.
pyright의 reportTypedDictNotRequiredAccess 에러 해결.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

Copilot AI 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.

Pull request overview

이 PR은 CrudFilter TypedDict 정의를 추가하고 protocols.pytypes.py로 이름을 변경합니다. Refine 프레임워크와 호환되는 필터 구조를 정의하며, 타입 안정성을 개선하기 위해 제네릭 dict[str, Any] 대신 구조화된 타입을 도입합니다.

Changes:

  • LogicalFilterConditionalFilter TypedDict 정의 추가
  • protocols.pytypes.py 이름 변경 및 모든 import 경로 업데이트
  • _build_filter_conditions 반환 타입을 list[ColumnElement[bool]]로 명시

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
python/aioia_core/types.py 새로운 필터 타입 정의 추가 (LogicalFilter, ConditionalFilter, CrudFilter) 및 프로토콜 정의 포함
python/aioia_core/repositories.py 필터 파라미터 타입을 CrudFilter로 변경하고 반환 타입 명시 개선
python/aioia_core/fastapi/base_crud_router.py import 경로 업데이트 및 _decamelize_filter_fields에 타입 캐스팅 추가
python/tests/unit/test_base_repository.py CrudFilter 타입 사용 및 테스트 변수명 개선
python/aioia_core/testing/crud_fixtures.py import 경로 업데이트 및 필터 처리 로직 개선 (.get() 사용)
python/aioia_core/factories/base_repository_factory.py import 경로 업데이트
python/aioia_core/factories/base_manager_factory.py import 경로 업데이트
python/aioia_core/init.py 새로운 필터 타입 re-export 추가
Comments suppressed due to low confidence (2)

python/aioia_core/types.py:43

  • TypedDict의 total=False 설정으로 인해 모든 필드가 optional이 됩니다. LogicalFilter의 경우 field, operator, value 필드가 모두 필수이며, ConditionalFilter의 경우 operatorvalue 필드가 필수입니다.

total=False를 제거하고 선택적 필드만 NotRequired로 표시하는 것을 권장합니다. 또는 LogicalFilter의 경우 value 필드만 optional이고 (null/nnull 연산자는 value가 필요 없음), 나머지는 required로 정의해야 합니다.

예시:

class LogicalFilter(TypedDict):
    field: str
    operator: FilterOperator
    value: NotRequired[Any]

python/aioia_core/types.py:58

  • TypedDict의 total=False 설정으로 인해 모든 필드가 optional이 됩니다. ConditionalFilter의 경우 operatorvalue 필드가 모두 필수입니다.

total=False를 제거하여 모든 필드를 required로 만들어야 합니다.

예시:

class ConditionalFilter(TypedDict):
    operator: ConditionalOperator
    value: list[CrudFilter]

{
**filter_item,
"field": decamelize(str(filter_item["field"])),
"field": decamelize(str(filter_item.get("field", ""))),

Copilot AI Jan 29, 2026

Copy link

Choose a reason for hiding this comment

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

line 562에서 이미 "field" in filter_item을 확인했으므로, filter_item.get("field", "")는 항상 값을 반환합니다. str() 변환이 불필요합니다.

다음과 같이 단순화할 수 있습니다:

"field": decamelize(filter_item["field"])

또는 타입 안정성을 위해:

"field": decamelize(str(filter_item["field"]))
Suggested change
"field": decamelize(str(filter_item.get("field", ""))),
"field": decamelize(str(filter_item["field"])),

Copilot uses AI. Check for mistakes.

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.

mypy는 in 체크로 TypedDict narrowing을 지원하지 않아 filter_item["field"] 직접 접근 시 typeddict-item 에러 발생.

aioia_core/fastapi/base_crud_router.py:566: error: TypedDict "ConditionalFilter" has no key "field"  [typeddict-item]

.get() 유지 필요.

suppressed 제안(NotRequired 패턴)은 b88e19f에서 적용 완료.

total=False 대신 NotRequired 패턴 사용으로 필수/선택 키 명확화.
- LogicalFilter: field, operator 필수, value optional (null/nnull)
- ConditionalFilter: operator, value 모두 필수

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@ywkim

ywkim commented Jan 29, 2026

Copy link
Copy Markdown
Contributor Author

/gemini review

@gemini-code-assist gemini-code-assist 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.

Code Review

이 PR은 CrudFilter TypedDict 정의를 추가하고 protocols.pytypes.py로 변경하여 코드의 타입 안정성과 가독성을 크게 향상시켰습니다. 필터 구조가 명확해지고, 관련 코드에서 타입 힌트가 개선되어 유지보수성이 높아졌습니다. 특히 _build_filter_conditions 메서드의 반환 타입 명시는 매우 훌륭한 개선입니다. 전반적으로 잘 구현된 변경사항입니다.

{
**filter_item,
"field": decamelize(str(filter_item["field"])),
"field": decamelize(str(filter_item.get("field", ""))),

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

filter_itemLogicalFilter 타입인 경우, field는 필수 필드이므로 .get() 메서드 대신 filter_item["field"]와 같이 직접 접근하는 것이 TypedDict의 의도를 더 명확하게 반영합니다. elif "field" in filter_item: 조건문으로 이미 field 키의 존재 여부를 확인하고 있으므로 안전합니다. 또한 LogicalFilterfield는 이미 str 타입이므로 str() 캐스팅도 불필요합니다.

Suggested change
"field": decamelize(str(filter_item.get("field", ""))),
"field": decamelize(filter_item["field"])

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.

mypy는 in 체크로 TypedDict narrowing을 지원하지 않아 filter_item["field"] 직접 접근 시 에러 발생:

aioia_core/fastapi/base_crud_router.py:566: error: TypedDict "ConditionalFilter" has no key "field"  [typeddict-item]

pyright는 통과하지만 mypy 호환을 위해 .get() 유지 필요.

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.

TypeGuard 적용으로 .get() 제거 및 직접 접근 사용.

def is_logical_filter(f: CrudFilter) -> TypeGuard[LogicalFilter]:
    return "field" in f

5930606, fa36524 커밋 참조.

ywkim and others added 3 commits January 30, 2026 10:13
is_logical_filter, is_conditional_filter TypeGuard 함수 추가.
mypy가 in 체크로 TypedDict narrowing을 지원하지 않으므로
명시적 TypeGuard로 타입 좁히기 구현.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
is_logical_filter, is_conditional_filter TypeGuard 사용으로
명시적 타입 좁히기 적용. .get() 암시적 회피 제거.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
is_logical_filter, is_conditional_filter TypeGuard 사용으로
명시적 타입 좁히기 적용. isinstance 체크 및 .get() 암시적 회피 제거.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@ywkim

ywkim commented Jan 30, 2026

Copy link
Copy Markdown
Contributor Author

/gemini review

@gemini-code-assist gemini-code-assist 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.

Code Review

이 PR은 CrudFilter TypedDict 정의를 추가하고 protocols.pytypes.py로 이름을 변경하여 코드베이스 전반의 타입 안정성을 크게 향상시켰습니다. 새로운 타입을 도입하고 이를 코드 전체에 일관되게 적용하여 정적 분석을 용이하게 하고 유지보수성을 높인 점이 훌륭합니다. 한 가지 제안 사항으로, 테스트 목(mock) 객체가 프로토콜을 올바르게 준수하도록 수정하여 코드의 일관성과 신뢰성을 더욱 높일 수 있습니다.

Comment thread python/aioia_core/testing/crud_fixtures.py

Copilot AI 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.

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (2)

python/aioia_core/types.py:231

  • 타입 가드 함수 is_logical_filteris_conditional_filter__all__ 목록에 포함되지 않았습니다. 이 함수들은 __init__.py에서 re-export되고 있으므로, 일관성을 위해 types.py__all__에도 추가해야 합니다.
    python/aioia_core/types.py:82
  • 새로 추가된 타입 가드 함수 is_logical_filteris_conditional_filter에 대한 테스트 커버리지가 없습니다. 다른 함수들과 마찬가지로 이 함수들도 테스트해야 합니다. 예를 들어, 각 타입 가드가 올바른 필터 구조를 식별하고 잘못된 구조를 거부하는지 테스트해야 합니다.

ywkim and others added 2 commits January 30, 2026 10:48
is_logical_filter, is_conditional_filter를 __all__에 추가.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Remove __all__ from types.py as it duplicates exports already defined
in __init__.py. The module-level __all__ was unused (no wildcard imports
found in the project) and caused pylint R0801 duplicate-code warning.

Package-level API is controlled solely through __init__.py's __all__.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@ywkim ywkim merged commit 120ca04 into main Jan 30, 2026
2 checks passed
@ywkim ywkim deleted the refactor/reduce-any-types branch January 30, 2026 02:54
@github-actions

github-actions Bot commented Feb 4, 2026

Copy link
Copy Markdown

🎉 This PR is included in version 2.0.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants