Keyboard shortcuts

Press ← or → to navigate between chapters

Press S or / to search in the book

Press ? to show this help

Press Esc to hide this help

HH-Lion Pragmatic Audit — 2026-05-09

This audit reviews hh-lion for maintainability, reliability, security, and production readiness with one explicit constraint:

Prefer the simplest change that materially improves quality.

Do not add abstractions, tools, or architecture layers unless they clearly reduce risk or maintenance cost.

Executive Summary

hh-lion is not a toy project. It has a credible recommendation-service architecture, strong operational scaffolding, broad test coverage, and thoughtful fail-safe behavior in several critical paths.

The main risks are not “wrong framework choices”. They are:

  • oversized core modules
  • incomplete API contract fidelity
  • too much module-global shared state
  • engineering/security parity gaps
  • incomplete Kubernetes and CI/CD hardening
  • fallback-heavy behavior that can mask primary-path regressions

Overall assessment

  • Maintainability: Medium
  • Reliability: Medium to Good
  • Security: Medium
  • Production readiness: Partial

Plain-language verdict

The project is promising and already has several strong production-oriented pieces. But it still needs a focused cleanup pass before it should be considered a clean, dependable, low-drama production service.

The good news: the most valuable fixes are mostly simplifications, not big rewrites.

Review Plan Followed

  1. Inspect repo structure, stack, config, and operational docs
  2. Audit architecture and module boundaries
  3. Audit FastAPI usage, routing, OpenAPI contract quality, and validation
  4. Audit state management and lifecycle
  5. Audit the data layer: ClickHouse, Feast, Redis, Kafka, MLflow
  6. Audit retrieval/ranking/model-serving patterns
  7. Audit auth, security, privacy, and secret handling
  8. Audit error handling, observability, resilience, and background work
  9. Audit testing, performance, deployment, and production readiness
  10. Convert findings into a practical remediation plan

Concrete Verification Performed

  • uv run ruff check recsys tests scripts ✅
  • Targeted tests ✅
    • tests/unit/test_serving_config.py
      • tests/integration/test_api_lifespan.py
    • tests/contract/test_homepage_api.py
    • Result: 60 passed
  • OpenAPI probe against the application import ✅
    • /features/session/{anonymous_id} 200 schema = {}
    • /v2/homepage/refresh only declares 200 and 422
  • Existing coverage.xml reports line-rate 0.6344 (~63.44%)
  • Searched Kubernetes manifests for:
    • NetworkPolicy
    • runAsNonRoot
    • readOnlyRootFilesystem
    • automountServiceAccountToken
    • serviceAccountName
    • Result: none found in checked manifests

Findings by Area

1) Architecture and Module Boundaries

What was audited

  • Repository layout
  • Separation of serving/retrieval/ranking/features/ingestion/training
  • Core file sizes and concentration of responsibilities

Current implementation

High-level repo structure is good:

  • recsys/serving
  • recsys/features
  • recsys/retrieval
  • recsys/ranking
  • recsys/ingestion
  • recsys/models
  • recsys/training
  • recsys/contracts

But several key files are too large and responsibility-heavy:

  • recsys/serving/app.py — 1558 lines
  • recsys/features/feast.py — 1343 lines
  • recsys/features/clickhouse.py — 932 lines
  • recsys/ingestion/kafka.py — 890 lines
  • recsys/features/service.py — 652 lines

Alignment with docs and best practices

Partial.

The top-level package split aligns with common service architecture. The internal implementation does not consistently follow the small-module, router-first approach recommended by FastAPI and common Python service practices.

Issues / risks / smells

  • The directory structure suggests a cleaner architecture than the code actually has.
  • recsys/serving/app.py is still a god file.
  • Adapters and orchestration layers often mix:
    • domain logic
    • normalization
    • fallback policy
    • metrics/logging
    • dependency bootstrapping

Simple, practical fix:

  • Split recsys/serving/app.py into:
    • app creation / lifespan
    • homepage routes
    • refresh/cache helpers
    • internal/debug routes
  • Do not add a heavy clean-architecture framework.
  • Just reduce file size and make ownership obvious.

File-level targets:

  • recsys/serving/app.py
  • recsys/features/feast.py
  • recsys/features/clickhouse.py
  • recsys/ingestion/kafka.py

References

  • FastAPI: Bigger Applications / APIRouter
    • https://fastapi.tiangolo.com/tutorial/bigger-applications/

2) FastAPI Usage, Routing, and API Contract Quality

What was audited

  • Route layout
  • response models
  • OpenAPI quality
  • security documentation
  • API consistency

Current implementation

Positive signs:

  • versioned endpoints under /v2
  • routers exist for health, identity, and features
  • homepage request/response modeling is stronger than average
  • FastAPI lifespan is used correctly for startup/shutdown

But runtime behavior and OpenAPI do not fully match.

Alignment with docs and best practices

Mixed.

FastAPI is used competently, but the OpenAPI contract is incomplete for several real behaviors.

Issues / risks / smells

  1. Historical note: auth existed at runtime but not in OpenAPI at audit time

    • This gap was part of the earlier audit context and is no longer relevant after the later internal-only serving cleanup.

  2. Incomplete response modeling

    • Verified: /features/session/{anonymous_id} has an empty 200 response schema.
    • Verified: /v2/homepage/refresh documents richer behavior than its schema declares.

  3. Debug/internal route design is mixed into public surface

    • /v2/recommendations behaves more like an internal/debug endpoint.

Simple, practical fix:

  • Keep the current endpoints.
  • Add missing response_model / responses= declarations.
  • Model API-key auth using FastAPI security helpers.
  • Move internal/debug endpoints under a clearly-labeled internal router.

Avoid:

  • full API gateway rework
  • auth microservice split
  • separate OpenAPI generation pipeline

References

  • FastAPI: Additional Responses in OpenAPI
    • https://fastapi.tiangolo.com/advanced/additional-responses/
  • FastAPI: Security / First Steps
    • https://fastapi.tiangolo.com/tutorial/security/first-steps/

3) Request/Response Validation and Pydantic Discipline

What was audited

  • request/response models
  • strictness
  • field defaults
  • identifier validation consistency

Current implementation

Homepage models are relatively disciplined:

  • extra="forbid"
  • aliases
  • examples
  • NanoID validation

Other surfaces are looser:

  • some mutable literal defaults are used directly
  • some path/query params lack equivalent validation
  • strictness is inconsistent across modules

Alignment with docs and best practices

Partial.

The strongest models align well with Pydantic practice; the rest are uneven.

Issues / risks / smells

  • inconsistent boundary strictness
  • weaker validation in feature/session query surfaces than in homepage body models
  • mutable defaults in some models increase code smell and future bug risk

Simple, practical fix:

  • Standardize on:
    • Field(default_factory=list)
    • Field(default_factory=dict)
    • extra="forbid" for external request models
  • Reuse NanoID validation for all anonymous_id / session_id inputs, not just some of them

Avoid:

  • building a custom schema/validation framework
  • over-abstracting model definitions into meta-model generators

References

  • Pydantic models docs
    • https://docs.pydantic.dev/latest/concepts/models/

4) State Management and Application Lifecycle

What was audited

  • startup/shutdown behavior
  • service initialization
  • module globals
  • app-scoped dependency storage

Current implementation

Good:

  • lifespan is used for startup/shutdown
  • runtime assembly through RuntimeComponents
  • refresh cache has Redis + in-memory fallback
  • environment validation is relatively strong

Less good:

  • several services and runtime objects are stored as module-global state:
    • model_context
    • _identity_resolver
    • _feature_service
    • refresh cache globals

Alignment with docs and best practices

Only partial.

Lifespan usage is aligned with FastAPI guidance. Heavy reliance on globals is not ideal.

Issues / risks / smells

  • harder test isolation
  • harder dependency swapping
  • multi-app mounting becomes awkward
  • hidden coupling between modules
  • get_config.cache_clear() in request flow is a smell

Simple, practical fix:

  • Move runtime singletons to app.state
  • Expose them through dependency functions
  • Keep RuntimeComponents as the typed container

This is a good simplification because it removes hidden state without adding much abstraction.

References

  • FastAPI: Lifespan Events
    • https://fastapi.tiangolo.com/advanced/events/

5) Data Layer: ClickHouse, Feast, Redis, Kafka, MLflow

What was audited

  • primary/secondary feature paths
  • event ingestion
  • serving data dependencies
  • model registry and artifact loading

Current implementation

The technology choices are credible and appropriate for a recommendation service:

  • Feast
  • ClickHouse
  • Redis
  • Kafka
  • MLflow
  • Faiss
  • CatBoost

There is a clear attempt to support:

  • online serving
  • feature reuse
  • model registry promotion
  • graceful degradation

Alignment with docs and best practices

Conceptually good. Implementation is strong in places but overly complicated in others.

Issues / risks / smells

  1. Primary-vs-fallback logic is hard to reason about

    • Feast is primary, ClickHouse is fallback, plus cohort/global fallback logic.

  2. Adapters are too smart

    • retrieval, normalization, fallback, and observability are often bundled together.

  3. Fallback-heavy behavior can hide primary-path regressions

    • availability is protected, but correctness drift can remain hidden.

  4. Large wrappers increase maintenance cost

    • especially feast.py, clickhouse.py, and ingestion code.

Simple, practical fix:

  • Document a strict serving precedence order:
    1. primary online source
    2. warehouse fallback
    3. cohort fallback
    4. static/global fallback
  • Add metrics/alerts for how often each fallback tier is used.
  • Separate normalization helpers from data access helpers where practical.

Avoid:

  • replacing the stack
  • building a new internal feature platform abstraction
  • adding another orchestration layer above Feast/ClickHouse

References

  • Feast docs
    • https://docs.feast.dev/
  • MLflow model registry workflow
    • https://mlflow.org/docs/latest/ml/model-registry/workflow/
  • ClickHouse insert best practices
    • https://clickhouse.com/docs/best-practices/selecting-an-insert-strategy
  • Confluent Kafka Python client docs
    • https://docs.confluent.io/platform/current/clients/confluent-kafka-python/html/index.html
  • Confluent Schema Registry schema evolution docs
    • https://docs.confluent.io/platform/current/schema-registry/fundamentals/schema-evolution.html

6) Retrieval, Ranking, and Model Serving

What was audited

  • ANN retrieval
  • reranking
  • feature completeness checks
  • model artifact loading
  • latency-oriented serving decisions

Current implementation

This is one of the better parts of the codebase.

Strengths:

  • Faiss wrapper makes trusted vs validated search explicit
  • CatBoost reranker has fallback behavior and feature completeness tracking
  • model artifact loading validates metadata and id-mapping
  • personalization flow is latency-aware

Alignment with docs and best practices

Mostly aligned with current recsys serving practice.

Issues / risks / smells

  • orchestration complexity is fairly high
  • there are many conditional branches across member/guest/fallback paths
  • fast-path vs background refresh behavior may be hard to debug

Simple, practical fix:

  • Add one short architecture note describing serving decision flow
  • Add a few high-value snapshot tests for fallback ladders and strategy decisions
  • Track fallback rate in production and alert when it stays elevated

Avoid:

  • replacing Faiss/CatBoost just to simplify code appearance
  • introducing complex policy engines for recommendation branching

7) Authentication, Security, Privacy, and Secret Handling

What was audited

  • CORS
  • cookies
  • K8s runtime hardening
  • CI/CD security posture
  • engineering vs production differences

Current implementation

Positive signs:

  • HMAC ownership checks for refresh flow
  • security scans exist in CI
  • serving container runs as non-root user
  • production manifests use seccomp and dropped capabilities

Alignment with docs and best practices

Mixed.

Security intent is good. Hardening depth is incomplete.

Issues / risks / smells

  1. Auth is middleware-enforced but not contract-modeled

    • OpenAPI does not show security requirements.

  2. Engineering auth gap

    • current auth middleware only enforces in production.

  3. CORS default is too permissive

    • wildcard default is tolerated in runtime config.

  4. Kubernetes hardening is incomplete

    • no NetworkPolicy
    • no runAsNonRoot in checked manifests
    • no readOnlyRootFilesystem
    • no automountServiceAccountToken: false
    • no serviceAccountName

  5. CI/CD supply-chain hardening is incomplete

    • third-party actions pinned to tags, not SHAs
    • GHCR usage still involves PAT-based flows in deploy path
    • no SBOM/attestation/image scanning in main path

Simple, practical fix:

  • [historical] Add proper FastAPI APIKeyHeader security dependencies
  • Enforce auth in engineering if reachable outside trusted infra
  • Fail closed on wildcard CORS in engineering/production
  • Add the missing K8s basics:
    • runAsNonRoot
    • explicit UID/GID
    • readOnlyRootFilesystem
    • automountServiceAccountToken: false
    • NetworkPolicy
  • Pin GitHub Actions to SHAs

These are high-value changes and do not add conceptual complexity.

References

  • Kubernetes Secrets
    • https://kubernetes.io/docs/concepts/configuration/secret/
  • Kubernetes security context
    • https://kubernetes.io/docs/tasks/configure-pod-container/security-context/
  • Kubernetes NetworkPolicy
    • https://kubernetes.io/docs/concepts/services-networking/network-policies/
  • GitHub Actions secure use reference
    • https://docs.github.com/en/actions/reference/security/secure-use

8) Error Handling, Resilience, and Observability

What was audited

  • exception handling style
  • fallback behavior
  • readiness/liveness
  • metrics/APM wiring
  • background job health

Current implementation

This area is relatively strong.

Strengths:

  • request IDs
  • structured logging
  • rich readiness checks
  • Prometheus metrics are domain-aware
  • background personalization health exists
  • release-readiness scripts exist

Alignment with docs and best practices

Mostly aligned, but not fully consistent.

Issues / risks / smells

  1. Broad exception swallowing in key request paths

    • helps uptime
    • can hide root-cause failures

  2. No strong centralized exception-handler strategy

    • behavior varies by route/module

  3. Observability wording exceeds implementation

    • setup_observability() is effectively a placeholder

  4. Fallbacks may become the hidden steady-state

    • without stronger alerting, degraded mode may look “healthy enough”

Simple, practical fix:

  • Add centralized exception handlers for known categories
  • Separate expected degraded-mode metrics from unexpected internal error metrics
  • Add alerts for sustained fallback-only behavior
  • Either wire real tracing or simplify observability claims/comments

Avoid:

  • introducing heavyweight distributed tracing infrastructure unless it is clearly needed now

9) Testing and Quality Gates

What was audited

  • test breadth
  • CI quality gates
  • coverage handling
  • guardrail tests

Current implementation

This repo has strong breadth of tests:

  • 135 test files
  • unit, integration, contract, performance, workflow guardrails
  • CI includes Ruff, MyPy, pytest, Bandit, pip-audit, release-readiness

Alignment with docs and best practices

Good overall.

Issues / risks / smells

  1. Coverage is measured, not enforced

    • current coverage is ~63.44%
    • no fail_under

  2. A large test suite can still miss behavioral gaps in complex fallback ladders

  3. Workflow guardrail tests are valuable, but they also reflect system complexity

Simple, practical fix:

  • Add a modest coverage floor to CI
  • Add a small mandatory smoke suite:
    • app import/boot
    • /health/ready
    • auth requirement behavior
    • OpenAPI snapshot assertions for critical endpoints

Do not chase coverage for its own sake. Use the floor to prevent regressions.

10) Performance and Deployment

What was audited

  • sync/async boundaries
  • rollout config
  • image usage
  • engineering parity

Current implementation

Good:

  • latency-aware personalization flow
  • Faiss + CatBoost are appropriate choices
  • production K8s has probes, HPA, PDB, spread constraints, graceful termination
  • evaluate/promote/rollback workflows are unusually mature

Alignment with docs and best practices

Good in deployment mechanics, mixed in app concurrency behavior.

Issues / risks / smells

  1. Async endpoints call sync services directly

    • can block the event loop under load

  2. Mutable image references still exist in parts of the repo

    • some jobs/manifests still use latest

  3. Engineering is not production-like enough

    • weaker hardening
    • weaker auth behavior
    • weaker signal for pre-prod confidence

Simple, practical fix:

  • Convert sync-I/O routes to def or offload blocking calls explicitly
  • Prefer image digests for deployable workloads
  • Bring engineering closer to production for security/probe shape

References

  • Kubernetes images docs
    • https://kubernetes.io/docs/concepts/containers/images/
  • Starlette middleware docs
    • https://www.starlette.io/middleware/

Prioritized Remediation Plan

Priority 0 — do these before calling the service “cleanly production-ready”

  1. Make auth and OpenAPI match reality

    • add FastAPI security dependencies
    • define security schemes
    • declare actual response variants

  2. Close engineering/security gaps

    • require auth in engineering if externally reachable
    • reject wildcard CORS in engineering/production

  3. Add missing Kubernetes hardening basics

    • runAsNonRoot
    • explicit UID/GID
    • readOnlyRootFilesystem
    • automountServiceAccountToken: false
    • NetworkPolicy

  4. Tighten CI/CD supply-chain basics

    • pin third-party actions to SHAs
    • reduce PAT usage where possible
    • add artifact/image provenance checks when practical

Priority 1 — biggest maintainability wins with low conceptual cost

  1. Split recsys/serving/app.py
  2. Move module globals into app.state + dependencies
  3. Make response/request schema discipline consistent
  4. Add fallback-rate alerting for primary-path degradation

Priority 2 — useful next improvements

  1. Add a modest coverage floor
  2. Add OpenAPI snapshot checks for critical endpoints
  3. Document serving decision/fallback flow
  4. Reduce size/complexity of Feast/ClickHouse/Kafka wrappers incrementally

Recommended Implementation Style

To keep this from becoming overengineered, follow these rules during remediation:

Prefer

  • moving code into smaller files over adding new abstraction layers
  • app-scoped dependencies over custom dependency frameworks
  • explicit models and handlers over clever metaprogramming
  • a few strong smoke tests over large new test frameworks
  • K8s hardening primitives over new security platforms
  • clearer docs and alerts over more fallback branches

Avoid

  • full architecture rewrites
  • introducing CQRS/event-sourcing style patterns
  • adding a custom internal framework
  • replacing working infrastructure just to “modernize”
  • adding new services when a file split or config cleanup is enough

Concrete, Low-Drama First Pass

If the goal is maximum ROI with minimum complexity, start with exactly these steps:

  1. Split recsys/serving/app.py into route modules and keep behavior unchanged
  2. [historical] Add FastAPI APIKeyHeader dependencies and correct OpenAPI metadata
  3. Fix missing response models on the known weak endpoints
  4. Move global runtime objects to app.state
  5. Enforce auth + non-wildcard CORS in engineering/production
  6. Add the missing K8s hardening fields and a basic NetworkPolicy
  7. Add CI coverage threshold and OpenAPI smoke checks

That set alone would noticeably improve maintainability, correctness, and operational confidence without making the system fancier.

Final Conclusion

hh-lion already has a respectable foundation, especially in ops workflow design and recsys-specific runtime thinking.

The best next step is not a rewrite.

The best next step is a pragmatic cleanup pass:

  • make the API contract honest
  • reduce hidden global state
  • break up oversized files
  • harden engineering and Kubernetes basics
  • make fallback behavior visible instead of silently relied upon

That is the shortest path to a cleaner, safer, more maintainable service.