Dev

[AB-Law]

Closes #100
Closes #101
Closes #99

Summary by CodeRabbit

Release Notes

• Bug Fixes

  • Improved scraper error handling to fail-fast on load failures, preventing silent execution issues.

• Refactor

  • Consolidated authentication token resolution pipeline across services for streamlined credential handling.
  • Enhanced request tracing and metadata propagation throughout the system for improved debugging and system observability.

• Tests

  • Added comprehensive unit tests for token resolution and digest processing workflows.

[coderabbitai]

📝 Walkthrough

Walkthrough

This PR consolidates duplicated token resolution logic across three HTTP function handlers (AuthFunctions, CollectionFunctions, StylistFunctions) into a shared ITokenResolver interface and implementation. It also introduces LLM client singleton caching in digest processing, enhances error visibility in scraper runner, and adds trace ID propagation to Langfuse callbacks for improved observability.

Changes

Cohort / File(s)	Summary
Token Resolution Abstraction PluckIt.Functions/Auth/TokenResolver.cs, PluckIt.Functions/Program.cs	New ITokenResolver interface and TokenResolver sealed class centralize bearer token extraction, Google validation, session fallback, and local dev user resolution with secure logging. Registered as DI singleton.
Auth Functions Refactoring PluckIt.Functions/Functions/AuthFunctions.cs, PluckIt.Functions/Functions/CollectionFunctions.cs, PluckIt.Functions/Functions/StylistFunctions.cs	Replaced inline token validation and per-handler authentication logic with injected ITokenResolver dependency, removing duplicate validation and session resolution code. Simplified constructor signatures.
C# Unit Tests PluckIt.Tests/Unit/Auth/TokenResolverTests.cs, PluckIt.Tests/Unit/Functions/CollectionFunctionsTests.cs, PluckIt.Tests/Unit/Functions/StylistFunctionsTests.cs	Added comprehensive tests for TokenResolver covering token validation, session fallback, and local dev user scenarios. Updated existing function tests to use simplified TestTokenResolver mock.
Digest Agent Singleton & Tracing PluckIt.Processor/agents/digest_agent.py	Introduced per-process LLM singleton (_get_digest_llm) to reuse client across batch digest runs. Added parameterized _build_digest_llm with Langfuse callback support. Extended run_digest_for_user and _generate_suggestions with optional trace_id for trace propagation.
Function App Metadata & Trace Propagation PluckIt.Processor/function_app.py, PluckIt.Processor/agents/scraper_runner.py	Enhanced Langfuse callback construction to accept and propagate extra metadata (component, trace_label). Updated run_digest_now to extract X-Trace-Id header. Changed scraper runner error handling from silent return to exception re-raise for operational visibility.
Python Unit Tests PluckIt.Processor/tests/unit/test_digest_agent.py, PluckIt.Processor/tests/unit/test_routes.py, PluckIt.Processor/tests/unit/test_scraper_runner.py	Added tests for LLM singleton behavior, trace/metadata propagation through digest pipeline, and scraper runner failure modes. Verified X-Trace-Id header forwarding to digest and metadata inference paths.

Sequence Diagram(s)

sequenceDiagram
    participant Client
    participant HttpFunction as HTTP Function<br/>(AuthFunctions/CollectionFunctions<br/>/StylistFunctions)
    participant TokenResolver
    participant GoogleValidator as Google<br/>Validator
    participant SessionStore as RefreshSession<br/>Store
    participant Business as Business<br/>Logic

    Client->>HttpFunction: POST with Authorization header
    HttpFunction->>TokenResolver: ResolveUserIdAsync(request)
    
    alt Bearer Token Present
        TokenResolver->>TokenResolver: TryGetBearerToken()
        TokenResolver->>GoogleValidator: validateToken(token)
        alt Validation Success
            GoogleValidator-->>TokenResolver: userId
            TokenResolver-->>HttpFunction: userId
        else Validation Fails
            TokenResolver->>SessionStore: ResolveUserIdFromSessionToken()
            alt Session Resolves
                SessionStore-->>TokenResolver: userId
                TokenResolver-->>HttpFunction: userId
            else Session Fails
                TokenResolver-->>HttpFunction: null
            end
        end
    else No Bearer Token
        TokenResolver-->>HttpFunction: localDevUserId
    end
    
    HttpFunction->>Business: Process with resolved userId
    Business-->>Client: Response

Loading

sequenceDiagram
    participant Trigger as Timer Trigger
    participant FunctionApp as Function App<br/>(function_app.py)
    participant DigestAgent as Digest Agent
    participant DigestLLM as Digest LLM<br/>(Singleton)
    participant Langfuse as Langfuse<br/>Callbacks
    participant AzureOpenAI as Azure<br/>OpenAI

    Trigger->>FunctionApp: run_digest_now(trace_id)
    FunctionApp->>FunctionApp: Extract X-Trace-Id from headers
    FunctionApp->>DigestAgent: run_digest_for_user_with_status(user_id, trace_id)
    
    DigestAgent->>Langfuse: _build_digest_langfuse_callbacks(trace_id, user_id)
    Langfuse-->>DigestAgent: callbacks
    
    DigestAgent->>DigestAgent: _infer_climate_zone(callbacks, trace_id, user_id)
    DigestAgent->>DigestLLM: _get_digest_llm() [cached]
    DigestLLM->>AzureOpenAI: invoke with callbacks
    AzureOpenAI-->>DigestLLM: result
    DigestLLM-->>DigestAgent: climate_result
    
    DigestAgent->>DigestAgent: _generate_suggestions(callbacks, trace_id, user_id)
    DigestAgent->>DigestLLM: _get_digest_llm() [reused]
    DigestLLM->>AzureOpenAI: invoke with callbacks & metadata
    AzureOpenAI-->>DigestLLM: suggestions
    DigestLLM-->>DigestAgent: suggestions
    
    DigestAgent->>Langfuse: _flush_digest_langfuse_callbacks(callbacks)
    Langfuse-->>DigestAgent: flushed
    
    DigestAgent-->>FunctionApp: digest_result
    FunctionApp-->>Trigger: success

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

• feat: enhance user authentication and session management #47: Directly related as both PRs modify the same authentication codepaths (CollectionFunctions, StylistFunctions, and AuthFunctions) and integrate RefreshSessionStore-backed session fallback resolution patterns that TokenResolver now centralizes.
• feat: implement refresh session functionality with token management #45: Directly related because TokenResolver wires an optional RefreshSessionStore dependency to resolve session user IDs when token validation fails, operating on the same session/auth infrastructure.

Poem

🐰 A Rabbit's Refactoring Rhyme

Once tokens sprawled in triplicate mess,
Each handler a copy—a duplicative stress!
Now TokenResolver reigns, singular and bright,
While digest LLMs cache for the night.
🥕 Traces now flow from tail to toe—
One source of truth: observe and grow! ✨

🚥 Pre-merge checks | ✅ 2 | ❌ 3

❌ Failed checks (3 warnings)

Check name	Status	Explanation	Resolution
Title check	⚠️ Warning	The title 'Dev' is generic and vague, providing no meaningful information about the primary change in this substantial refactoring pull request.	Revise the title to clearly summarize the main objective, such as 'Consolidate token resolution and optimize digest LLM initialization' or 'Refactor auth token validation and fix scraper error handling'.
Description check	⚠️ Warning	The PR description is incomplete, listing only linked issue numbers without addressing required template sections (Summary, Changes, Testing, Risks/Checklist).	Complete the description by filling in Summary (what/why), Changes (bullet list), Testing (which tests ran), and Risks/Notes sections per the repository template.
Docstring Coverage	⚠️ Warning	Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	All three linked issues (#99, #100, #101) have corresponding implementations: TokenResolver consolidates auth logic [#100], scraper_runner re-raises exceptions [#101], and digest_agent implements LLM singleton caching [#99].
Out of Scope Changes check	✅ Passed	All changes directly support the three linked issues: token resolver refactoring, scraper error handling, digest LLM caching, and related test updates are all in scope.

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

✨ Finishing Touches

• 📝 Generate docstrings (stacked PR)
• 📝 Generate docstrings (commit on current branch)

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch dev

📝 Coding Plan

• Generate coding plan for human review comments

---

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

[coderabbitai]

🧹 Nitpick comments (1)

PluckIt.Tests/Unit/Functions/CollectionFunctionsTests.cs (1)

229-237: Consider centralizing TestTokenResolver in shared test helpers.

This test double is now duplicated across function test classes; moving it into a shared helper would reduce drift and maintenance overhead.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@PluckIt.Tests/Unit/Functions/CollectionFunctionsTests.cs` around lines 229 -
237, Centralize the duplicated test double by extracting the sealed
TestTokenResolver(string? userId) implementation (which implements
ITokenResolver and its ResolveUserIdAsync(HttpRequestData, CancellationToken)
method) into a shared test helper class in the test project and update all
function test classes to reference this single helper instead of their local
copies; remove the duplicated inner classes from tests and ensure the shared
TestTokenResolver is public/internal as needed so existing tests compile and
continue returning Task.FromResult<string?>(userId).

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@PluckIt.Tests/Unit/Functions/CollectionFunctionsTests.cs`:
- Around line 229-237: Centralize the duplicated test double by extracting the
sealed TestTokenResolver(string? userId) implementation (which implements
ITokenResolver and its ResolveUserIdAsync(HttpRequestData, CancellationToken)
method) into a shared test helper class in the test project and update all
function test classes to reference this single helper instead of their local
copies; remove the duplicated inner classes from tests and ensure the shared
TestTokenResolver is public/internal as needed so existing tests compile and
continue returning Task.FromResult<string?>(userId).

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 669f017a-b5c1-43db-a355-4aef80667b43

📥 Commits

Reviewing files that changed from the base of the PR and between 6f16e95 and 31fa30e.

📒 Files selected for processing (14)

• PluckIt.Functions/Auth/TokenResolver.cs
• PluckIt.Functions/Functions/AuthFunctions.cs
• PluckIt.Functions/Functions/CollectionFunctions.cs
• PluckIt.Functions/Functions/StylistFunctions.cs
• PluckIt.Functions/Program.cs
• PluckIt.Processor/agents/digest_agent.py
• PluckIt.Processor/agents/scraper_runner.py
• PluckIt.Processor/function_app.py
• PluckIt.Processor/tests/unit/test_digest_agent.py
• PluckIt.Processor/tests/unit/test_routes.py
• PluckIt.Processor/tests/unit/test_scraper_runner.py
• PluckIt.Tests/Unit/Auth/TokenResolverTests.cs
• PluckIt.Tests/Unit/Functions/CollectionFunctionsTests.cs
• PluckIt.Tests/Unit/Functions/StylistFunctionsTests.cs