complexity-assessment.md

complexity-assessment.md

Complexity Assessment & Refactoring Recommendations

Date: 2026-03-06

Methodology

Three tools were used to triangulate complexity from different angles:

• complexity crate — per-function cognitive complexity scores for every function in the codebase (based on Cognitive Complexity by G. Ann Campbell). Cognitive complexity measures how hard code is to understand, penalizing nested control flow more than flat branching.
• clippy cognitive_complexity lint — same metric but only flags functions above a threshold, confirming the top offenders.
• scc — file-level complexity (branch point counting), line counts, and COCOMO cost estimation for overall codebase assessment.

Overall Assessment

The codebase is well-structured for its size and does not need urgent refactoring.

Rationale:

• ~590 functions total. 185 have complexity > 0. Only 5 functions (0.8%) score above 25. Only 25 functions (4.2%) score above 10.
• 80 functions (43% of non-trivial functions) score exactly 1 — simple one-branch logic.
• 30 functions score 2, 13 score 3. The vast majority of the codebase is low-complexity.
• Complexity is concentrated and predictable — it clusters in a custom expression parser, a game state machine, and infrastructure loops (season processing, job queue, Redis reconnection). These are domains where complexity is inherent to the problem, not symptoms of poor design.
• File sizes are moderate: only 2 files exceed 1,000 lines, and both are tied at 1,468 lines. For a 16k-line codebase, this is healthy.
• COCOMO estimates ~10 months / ~4 people, consistent with a medium-sized backend service.

What "well-structured" means concretely: The complexity isn't scattered. You won't find a 40-complexity function buried in a utility module or a 300-line handler in an unrelated file. The hot spots are exactly where you'd expect them: the expression engine, the most complex game endpoint, and the season lifecycle. A developer unfamiliar with the codebase can identify the hard parts by looking at file names alone.

---

Complexity Distribution

Score range   Functions   % of total (185)   Character
─────────────────────────────────────────────────────────────
  1              80           43.2%           Trivial — single branch or guard
  2-5            64           34.6%           Simple — straightforward handlers
  6-10           22           11.9%           Moderate — multi-branch business logic
  11-25          14            7.6%           Notable — deserve attention when modifying
  26-50           3            1.6%           High — refactoring candidates
  51+             2            1.1%           Very high — should be split

The distribution is right-skewed (long tail toward high complexity with a heavy concentration at the low end), which is the healthy pattern. An unhealthy codebase would show a flatter distribution or a bump in the 15-30 range.

---

Detailed Function Analysis

Tier 1 — Refactoring recommended

These functions have complexity significantly above the rest of the codebase. They represent the highest ROI for refactoring because they are the hardest to reason about, the most likely to harbor subtle bugs, and the most expensive to modify safely.

---

1. Condition::from_str — cognitive complexity 86

File: src/notifications/types/condition/mod.rs:154 (190 lines)

What it does: Parses notification condition strings (e.g. "games_played > 5 && score >= 100") into a Vec<ConditionPart> — a tokenized representation used by the shunting-yard evaluator.

Why it's the most complex function: The entire lexer is a single while let loop over a Peekable<Chars> with a match on the current character. Each arm handles one token type:

• Single-char operators: +, /, %, (, )
• Two-char operators with look-ahead: ==, !=, >=, <=, &&, ||, **
• Context-sensitive operators: - is unary or binary depending on the preceding token
• Multi-char literals: numbers (with decimal points), quoted strings, identifiers
• Keyword resolution: true, false, null, function names

Each look-ahead for two-character tokens adds a nested if let Some(&next_char) = chars.peek() inside the match arm, and the unary minus detection requires inspecting the previous token. This creates deep nesting that the cognitive complexity metric penalizes heavily.

Why it matters: This is a parser — bugs here silently misinterpret notification conditions, potentially spamming or suppressing notifications for all users. The function is already 190 lines with no tests for edge cases like --x (double unary), 5.5.5 (malformed number), or empty input.

Refactoring approach: Split into two phases:

1. tokenize(s: &str) -> Result<Vec<Token>, Error> — Character-level lexing. Produces a flat stream of typed tokens. Each token type (operator, number, string, identifier) becomes a helper function: lex_number(&mut chars), lex_string(&mut chars), lex_identifier(&mut chars), lex_operator(&mut chars, &prev_token).

2. from_str calls tokenize() and maps Tokens to ConditionParts — keyword resolution (true/false/null/function names) happens here.

This doesn't change any logic. It mechanically separates "what characters make up this token" from "what does this token mean." The individual lex helpers are ~10-15 lines each and independently testable.

Effort: Low. No logic changes, just extraction. The function already has natural seam points between token types.

Risk of not refactoring: The function works today. But any future change to the expression language (new operators, escape sequences in strings, nested function calls) will make this function even harder to modify correctly. The 190-line single-function parser is the kind of code that accumulates band-aid fixes over time.

---

2. submit_leg — cognitive complexity 59

File: src/http/api/protected/async_pvp/mod.rs:232 (347 lines, 232-578)

What it does: Handles a player submitting answers for their turn in an async PvP game. This is the main game progression endpoint.

Why it's complex: After validation and score recording (~100 lines), the function branches into four distinct paths based on game state:

1. Partial answers remain (line 363) — Update state, keep waiting for more answers
2. All questions answered + game ends (line 385) — Determine winner, archive game to historic table, send game-ended notification
3. All questions answered + player picks next category (line 464) — Generate category choices, set awaiting-pick state
4. All questions answered + opponent's turn (line 483) — Collect question IDs from scores, set awaiting-report state for opponent, send turn notification

After the transaction commits (line 519), there's another branch for building the response: turn-switched path fetches both users' public data and sends a notification, non-switched path just fetches opponent data.

Each path involves 40-80 lines of DB operations, notification assembly, and error handling. The game-end path (path 2) alone is ~80 lines containing winner determination, DB deletion + historic insertion, user data lookup, notification context building, and notification dispatch.

Why it matters: This is the most exercised game endpoint. Every completed turn flows through it. The branching logic determines game state transitions — a bug here corrupts game state for both players. The function's length makes it hard to verify that all paths correctly update game.game_data.state, commit the transaction, and build the right response.

Refactoring approach: Extract each state transition path into a named function:

• conclude_game(&mut tx, &mut game, user_id, opponent_id, last_leg_score, &ctx) -> Result<(), StatusCode> — Lines 385-463. Winner determination, DB archival, notification.
• advance_to_category_pick(&mut tx, &mut game, user_id) -> Result<(), StatusCode> — Lines 464-482. Category generation, state update.
• switch_to_opponent_turn(&mut tx, &mut game, user_id, opponent_id, category) -> Result<(), StatusCode> — Lines 483-516. Question ID collection, state update.

The post-commit response building (lines 527-576) could also be extracted into build_progression_response().

submit_leg then becomes: validate → record scores → match on state → call the right transition function → commit → build response. The flow is readable at a glance.

Effort: Medium. Each extracted function needs the transaction, game, and several IDs. The conclude_game extraction is the highest-value since it's the longest and most complex path. The other two are straightforward.

Risk of not refactoring: Functional but fragile. Adding a new game mode or state transition means inserting into already-deep nesting. The notification assembly for game-end is interleaved with DB operations, making it hard to test notification content independently.

---

Tier 2 — Refactor opportunistically

These functions are complex but well-organized. Refactoring would improve readability but is not urgent. Recommended only when actively modifying these areas.

---

3. Condition::is_valid — cognitive complexity 41

File: src/notifications/types/condition/mod.rs:23 (75 lines)

What it does: Evaluates a parsed condition expression against a context map using the shunting-yard algorithm.

Why the score is high but the function is fine: The shunting-yard algorithm requires nested loops with break conditions — while let over the operator stack inside a for over the parts, with early breaks on parentheses and precedence comparisons. This is inherently nested control flow that cognitive complexity penalizes.

Recommendation: Leave as-is. The function is 75 lines, well-commented by its structure, and implements a well-known algorithm. Splitting it into pieces would scatter the two-stack invariant across multiple functions, making correctness harder to verify. The apply_op helper is already extracted. The algorithm doesn't benefit from further decomposition.

Rationale: Cognitive complexity is a heuristic. It measures nesting depth, not actual difficulty. A shunting-yard implementation looks complex to the metric but is straightforward to anyone familiar with the algorithm. Refactoring to satisfy the metric would make the code harder to understand by hiding the algorithm's structure behind function call indirection.

---

4. run_season_processing — cognitive complexity 35

File: src/season/mod.rs:160 (112 lines)

What it does: Stepped state machine for season processing: SetProcessing → GracePeriod → ProcessRewards → SetProcessed. Each step checkpoints to the job payload for crash recovery.

Why the score is high: The loop { match payload.step { ... } } pattern with four arms, each containing DB operations, shutdown checks, and payload checkpointing. The GracePeriod arm has a nested sleep loop with tokio::select! for shutdown handling.

Recommendation: Leave as-is, with one minor suggestion. The state machine is already well-factored — process_rewards is extracted into its own function. Each match arm is 15-30 lines and self-contained. The function reads top-to-bottom as a clear sequence of steps.

The one improvement: the GracePeriod sleep loop (lines 222-234) could be extracted to async fn wait_with_shutdown(duration, shutdown) -> bool since the "sleep in chunks, checking shutdown" pattern appears elsewhere in the codebase (job queue, reaper). But this is minor.

Rationale: State machines are inherently branchy. The match arms are the right abstraction — they make each state's behavior visible in one place. Extracting each arm into a separate function would just move the code without reducing cognitive load.

---

5. send_rendered — cognitive complexity 28

File: src/notifications/aws_sns.rs:98 (104 lines)

What it does: Spawns a task to send a push notification: fetch push tokens → persist notification to DB → serialize per platform (iOS/Android) → send in parallel → handle errors (delete disabled endpoints) → log aggregate errors.

Why the score is high: Sequential operations with error handling at each step, plus a for loop over results with a match on error types. The error handling loop (lines 167-191) is the main contributor — it pattern-matches on AWS SNS error types to decide whether to delete the token or just log.

Refactoring approach: Extract handle_send_results(results, push_tokens, db) -> Vec<String> for the error-handling loop. This isolates the AWS error type matching and token cleanup from the main send flow.

Effort: Low. Clean boundary, no shared mutable state.

Rationale for moderate priority: The function works fine. The error handling is the only part that's hard to follow at a glance. If you need to add new error types or change the retry behavior, extracting it first would help.

---

6. daily_notifications::run — cognitive complexity 27

File: src/notifications/daily_notifications.rs:29 (120 lines)

What it does: Batch job that sends daily reminder notifications to inactive users. Paginates through users by timezone, renders notifications, sends concurrently, cleans up rejected tokens, checkpoints progress for crash recovery.

Why the score is high: The main loop contains: shutdown check → DB pagination → stream processing with buffer_unordered(256) → result collection → rejected token cleanup → payload save → break condition check. Seven sequential concerns in one loop body.

Recommendation: Leave as-is. Despite the score, the function reads linearly top-to-bottom. Each step is 5-15 lines. The complexity comes from the batch processing pattern (pagination + shutdown + checkpointing), not from tangled logic. This is infrastructure code that's written once and rarely modified.

Rationale: Extracting pieces would require passing the payload, shutdown receiver, notification service, and DB pool into each helper. The function signatures would be wider than the extracted bodies. The cure is worse than the disease.

---

Tier 3 — No action needed

These functions score 10-23. They were reviewed individually and all represent appropriate complexity for their problem domain.

Score	Function	File	Rationale for leaving as-is
23	progress_mission	mission/types/category_unlock.rs	Category selection with three fallback tiers (new → batch-reuse → owned-reuse). The branching is the business logic — each tier is a deliberate design choice for handling category exhaustion. Collapsing tiers would lose the priority ordering.
23	dev_token	http/api/public/reg.rs	Registration with retry loop for username collisions. The retry logic generates candidates with increasing suffix complexity (2-digit → 3-digit → alphanumeric). Already at its natural complexity floor.
20	coins_for_percentile	season/mod.rs	if/else chain mapping percentile ranges to coin rewards. This is a business rules lookup table, not algorithmic complexity. A match or array-based approach wouldn't be meaningfully clearer.
18	render	notifications/templates.rs	Template rendering with weighted random selection and Tera integration. The branching handles template variants (with/without body, different rendering paths). Standard template engine glue.
16	execute_job	services/job_queue.rs	match on job result (Completed, ShutdownInterrupt, Error with retries left, Error exhausted). Four arms, each 10-15 lines of DB update + logging. This is the textbook pattern for job result handling. Splitting would scatter the four outcomes across files.
16	get (session)	http/session.rs	Redis session lookup with tombstone check and GETEX for sliding expiration. The branching handles Redis errors, UUID parsing, tombstone hits. Standard Redis session management.
15	redis_subscriber_loop	season/cache.rs	Outer reconnection loop + inner message loop with tokio::select! for shutdown. Three error handling points (client creation, pubsub connection, subscription). This is the inherent shape of a resilient pub/sub subscriber — the reconnection with backoff requires the nested loop structure.
14	worker_loop	services/job_queue.rs	Claim-execute-drain loop with semaphore concurrency control and graceful shutdown. The tokio::select! for shutdown interleaves with semaphore acquisition and job claiming. Already well-structured with execute_job extracted.
13	update_username	db/users.rs	Username change with collision retry, validation, and history tracking. Business logic is inherently sequential with multiple failure modes.
12	add_with_code	http/api/protected/friends.rs	Friend-add with existing friendship detection (active, inactive, pending). Each prior state requires different handling. The branching maps directly to the friendship state machine.

---

Risk Areas (independent of complexity)

Complexity scores don't capture all risks. Based on the code review, these areas deserve attention regardless of their scores:

1. The condition engine (condition/mod.rs) — 1,468 lines implementing a custom expression language. The parser, evaluator, and operator implementations are all hand-rolled. This is the most bug-prone area because expression language bugs silently affect notification delivery for all users. It has good test coverage for basic cases but lacks edge case tests for the parser.

2. The async PvP module (async_pvp/mod.rs) — 1,468 lines, 6 functions in the top 25. This module manages a distributed game state machine across two players. State transitions happen across multiple endpoints (submit_leg, pick_category, start_random_game, challenge_friend, accept_friend_challenge). A bug in one endpoint can leave a game in a state that no other endpoint can advance.

3. Season processing (season/mod.rs) — Manages a weekly lifecycle with real-money-adjacent rewards (coins). The crash recovery via job payload checkpointing is critical — a bug in checkpoint ordering could cause duplicate rewards or skipped groups.

---

Summary

Tier	Functions	Action	Rationale
1 — Refactor	from_str (86), submit_leg (59)	Split when next modifying	Both significantly exceed the codebase norm. Clear seam points exist. Highest ROI for readability and future maintainability.
2 — Opportunistic	is_valid (41), run_season_processing (35), send_rendered (28), daily_notifications::run (27)	Leave or extract helpers if modifying	Scores are elevated but the code is well-organized. Refactoring would improve metrics but not meaningfully improve understanding.
3 — Leave alone	14 functions scoring 10-23	No action	Complexity matches the problem domain. Further decomposition would just redistribute complexity into function signatures and call sites.
Baseline	166 functions scoring 1-9	No action	Well within healthy norms.

Bottom line: Two functions (from_str and submit_leg) account for 145 of the codebase's combined cognitive complexity points. Refactoring just those two would bring every function in the codebase below 45 — well within maintainable range. Neither is urgent, but both should be split before their next significant modification.