Skip to content
This repository was archived by the owner on Mar 2, 2026. It is now read-only.

Ref/use new match syntax log#19

Open
AliiiBenn wants to merge 7 commits intomainfrom
ref/use-new-match-syntax-log
Open

Ref/use new match syntax log#19
AliiiBenn wants to merge 7 commits intomainfrom
ref/use-new-match-syntax-log

Conversation

@AliiiBenn
Copy link
Member

@AliiiBenn AliiiBenn commented Oct 5, 2025

Summary by CodeRabbit

  • New Features
    • More consistent success/error behavior across log endpoints for a smoother API experience.
  • Bug Fixes
    • Invalid or inactive API keys now correctly return 403 responses.
    • Input sanitization added for API keys to prevent malformed requests.
  • Refactor
    • Streamlined error handling in log endpoints to improve reliability and maintainability.
  • Tests
    • Added comprehensive tests for result handling to ensure predictable responses and robust error propagation.

AliiiBenn and others added 7 commits October 5, 2025 11:37
Add a standalone match function that provides functional pattern matching
for Result objects. This allows developers to handle both success and error
cases in a clean, declarative way.

The function follows the exact specification from issue #15:
- Function signature: def match(result: Result[T, E], *, on_success: Callable[[T], R], on_error: Callable[[E], R]) -> R:
- Uses keyword-only arguments with * syntax
- Calls appropriate callback based on result.is_ok() and result.is_err()
- Includes proper type hints following existing code style
- Works with both Ok and Err instances

Closes #15

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
Add comprehensive tests for the new match function covering:
- Basic success and error cases with numeric values
- Type-specific handling with custom functions
- Functions that return different types
- Edge cases with None values
- Exception handling in callbacks
- Usage examples from issue #15

All tests pass and verify the implementation meets the
acceptance criteria specified in the issue.

Closes #15

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
Add match method signature to the Result Protocol interface to enable
instance-based pattern matching on Result objects. This matches the
acceptance criteria for issue #16.

The signature follows the specification:
def match(self, on_success: Callable[[T], R], on_error: Callable[[E], R]) -> R:

This addition enables more functional programming patterns with Result objects
and allows for method chaining while maintaining compatibility with existing
code.

Closes #16

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
- Add match method implementation to Err class that calls on_error callback
- Add comprehensive test suite for instance method match() functionality
- Test includes basic usage, advanced patterns, type handling, and edge cases
- All tests pass covering success and error scenarios for method-based matching
- Maintains backward compatibility with existing standalone match function

Closes #16

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
Fix critical type safety issue where Err.unwrap() incorrectly returned
error value (type E) instead of declaring it would raise an exception.

Changes:
- Err.unwrap() now raises Exception("Tried to unwrap an Err") instead of returning self.error
- This fixes the type contract: method declares return type T but was returning E
- Behavior now matches typical Result/Maybe patterns where unwrap() on error raises
- Updated standlone match function to delegate to instance method instead of direct unwrap calls

This prevents potential runtime crashes and maintains type safety when
unwrap() is used with error results.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
Replace assert False with raise AssertionError() for better test reliability
in optimized Python environments (-O flag).

Changes:
- Replace assert False, "message" with raise AssertionError("message")
- Ensures test failures are properly caught in all Python environments
- Improves test consistency and reliability
- No functional changes to test logic

This addresses the reliability concern where assertions can be disabled
in optimized Python builds.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
- Replace repetitive is_err() and unwrap() pattern with declarative match() calls
- Add _raise_error helper function for exception handling in lambdas
- Update match() function signature to use named result parameter
- Convert all 9 route handlers in log router to use Result.match() directly
- Preserve identical functionality while improving code readability
- Remove unnecessary import of external match function

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
@coderabbitai
Copy link

coderabbitai bot commented Oct 5, 2025

Walkthrough

Introduces Result.match pattern in utils, updates log routes to use match-based control flow with a helper to re-raise errors, tightens API key dependency with sanitization and active checks, and adds tests covering match behavior and edge cases.

Changes

Cohort / File(s) Summary
Router refactor to match-based flow
src/routers/log.py
Added _raise_error(error) helper. Refactored route handlers to use Result.match(on_success, on_error) instead of manual checks. Enhanced get_api_key_dep with input sanitization and active-status validation; returns 403 for invalid/inactive keys. Minor formatting adjustments.
Result.match feature
src/utils/result.py
Added generic type R. Extended Result protocol with match(self, on_success, on_error) -> R. Implemented match in Ok and Err. Added module-level match(result=..., on_success=..., on_error=...). Updated Err.unwrap to raise the contained exception.
Tests for match API
tests/test_match.py
New tests for module-level match and instance Result.match, including success/error paths, type variations, None handling, exception propagation, and return-shape consistency.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor Client
  participant Router as Log Router
  participant Service as Domain Service
  participant Result as Result[T,E]
  participant Callbacks as on_success / on_error

  Client->>Router: HTTP request (e.g., create/update/get log)
  Router->>Service: invoke operation
  Service-->>Router: Result (Ok/Err)
  Router->>Result: match(on_success, on_error)
  activate Result
  alt Ok
    Result->>Callbacks: on_success(value)
    Callbacks-->>Router: Response payload
    Router-->>Client: 200/201 response
  else Err
    Result->>Callbacks: on_error(error)
    Callbacks-->>Router: raise error via _raise_error
    Router-->>Client: Error response (propagated)
  end
  deactivate Result
Loading
sequenceDiagram
  autonumber
  actor Client
  participant Dep as get_api_key_dep
  participant Store as DB Session

  Client->>Dep: Authorization: ApiKey <token>
  Dep->>Dep: sanitize/validate input
  Dep->>Store: lookup API key
  alt valid and active
    Store-->>Dep: APIKey
    Dep-->>Client: proceed (dependency resolves)
  else invalid or inactive
    Dep-->>Client: 403 Forbidden
  end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related issues

Poem

A rabbit taps keys with a gentle thrum,
Pattern-matching ready—here it comes! 🐇
Ok hops right, Err bounds left,
Clean little paths where bugs once crept.
Keys checked, logs flow, neat and bright—
Another tidy warren in the night.

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 31.58% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title clearly indicates that the pull request refactors log-related functionality to adopt the new match syntax, which is the primary focus of the changeset.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch ref/use-new-match-syntax-log

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

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 3d1f8b1 and a354513.

⛔ Files ignored due to path filters (3)
  • src/utils/__pycache__/result.cpython-312.pyc is excluded by !**/*.pyc
  • tests/__pycache__/__init__.cpython-312.pyc is excluded by !**/*.pyc
  • tests/__pycache__/test_match.cpython-312-pytest-8.4.2.pyc is excluded by !**/*.pyc
📒 Files selected for processing (3)
  • src/routers/log.py (14 hunks)
  • src/utils/result.py (4 hunks)
  • tests/test_match.py (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
tests/test_match.py (1)
src/utils/result.py (3)
  • Ok (18-47)
  • Err (50-73)
  • Result (9-15)
src/utils/result.py (1)
src/utils/maybe.py (2)
  • map (20-26)
  • bind (28-34)
src/routers/log.py (4)
src/core/log.py (8)
  • get_logs_by_tag (242-269)
  • create_log (18-53)
  • get_logs_by_project (56-80)
  • get_log_by_id (83-109)
  • update_log (112-153)
  • delete_log (156-177)
  • get_logs_by_level (180-212)
  • get_logs_by_category (215-239)
src/models/base.py (1)
  • get_db (18-23)
src/models/api_key.py (1)
  • APIKey (7-20)
src/utils/result.py (4)
  • match (15-15)
  • match (43-44)
  • match (69-70)
  • match (76-88)
🪛 Ruff (0.13.3)
tests/test_match.py

1-1: Shebang is present but file is not executable

(EXE001)


19-19: Unused lambda argument: e

(ARG005)


30-30: Unused lambda argument: e

(ARG005)


62-62: Unused lambda argument: e

(ARG005)


71-71: Unused lambda argument: e

(ARG005)


83-83: Unused lambda argument: e

(ARG005)


91-91: Unused lambda argument: x

(ARG005)


105-105: Unused lambda argument: x

(ARG005)


106-106: Unused lambda argument: e

(ARG005)


108-108: Avoid specifying long messages outside the exception class

(TRY003)


117-117: Unused lambda argument: x

(ARG005)


118-118: Unused lambda argument: e

(ARG005)


120-120: Avoid specifying long messages outside the exception class

(TRY003)


132-132: Unused lambda argument: e

(ARG005)


140-140: Unused lambda argument: e

(ARG005)


204-204: Do not catch blind exception: Exception

(BLE001)


221-221: Unused lambda argument: e

(ARG005)


229-229: Unused lambda argument: e

(ARG005)


237-237: Unused lambda argument: x

(ARG005)

src/utils/result.py

43-43: Unused method argument: on_error

(ARG002)


61-61: Create your own exception

(TRY002)


61-61: Avoid specifying long messages outside the exception class

(TRY003)


63-63: Unused method argument: f

(ARG002)


66-66: Unused method argument: f

(ARG002)


69-69: Unused method argument: on_success

(ARG002)

src/routers/log.py

74-74: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable

(B008)

Comment on lines +76 to +88
def match(*, result: Result[T, E], on_success: Callable[[T], R], on_error: Callable[[E], R]) -> R:
"""
Functional pattern matching for Result objects.

Args:
result: The Result object to match against
on_success: Function to call for success values
on_error: Function to call for error values

Returns:
The result of calling either on_success or on_error
"""
return result.match(on_success, on_error)
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Fix match() signature to accept the positional result argument

The helper is declared with a leading *, so result becomes keyword-only. Every new caller (e.g., tests/test_match.py, Line 16) passes the Result as a positional argument, causing TypeError: match() takes 0 positional arguments but 1 was given before any logic runs. This breaks all uses of the helper. Allow result to be positional (or update all call sites), e.g.:

-def match(*, result: Result[T, E], on_success: Callable[[T], R], on_error: Callable[[E], R]) -> R:
+def match(result: Result[T, E], *, on_success: Callable[[T], R], on_error: Callable[[E], R]) -> R:
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
def match(*, result: Result[T, E], on_success: Callable[[T], R], on_error: Callable[[E], R]) -> R:
"""
Functional pattern matching for Result objects.
Args:
result: The Result object to match against
on_success: Function to call for success values
on_error: Function to call for error values
Returns:
The result of calling either on_success or on_error
"""
return result.match(on_success, on_error)
def match(result: Result[T, E], *, on_success: Callable[[T], R], on_error: Callable[[E], R]) -> R:
"""
Functional pattern matching for Result objects.
Args:
result: The Result object to match against
on_success: Function to call for success values
on_error: Function to call for error values
Returns:
The result of calling either on_success or on_error
"""
return result.match(on_success, on_error)
🤖 Prompt for AI Agents
In src/utils/result.py around lines 76 to 88, the match helper was defined with
a leading * which made result keyword-only, causing callers that pass the Result
positionally to raise a TypeError; change the signature so result is a
positional parameter and keep on_success/on_error as keyword-only (e.g., make
result the first parameter and use a single * before the keyword-only callbacks)
and then return result.match(on_success, on_error) as before.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant