Skip to content

Enforce cognitive<=15 / cyclomatic<=20 complexity and <=1200-line file limit #860

Open

Description

@alex-mextner

What

The repository (alex-mextner/HyperCalendarBot) has no enforced limit on function complexity or file length, and the tree already exceeds the file-length policy we want to adopt.

Policy to enforce: physical text files <= 1200 lines OK, > 1200 warning, > 2400 error. The only default text-file exemption is .json; lockfiles are not exempt. Git-ignored files are not enumerated; binary files, symlinks and submodules are reported as skipped rather than interpreted as text. Tests, docs and source all count; untracked non-ignored files count. Functions: cognitive complexity <= 15 and cyclomatic complexity <= 20, measured by moznion/cccc. The 15/20 complexity defaults were selected during implementation; the user specified the 1200/2400 file-length thresholds.

EVIDENCE (captured at creation time). Measured 2026-10-26 at main commit 2327f650 over 1049 non-.json text files from git ls-files --cached --others --exclude-standard: 36 files exceed 1200 lines = 10 errors (> 2400) + 26 warnings.

Errors (> 2400): docs/intents/README.md 5961, docs/plans/2026-03-13-google-calendar-sync.md 3477, docs/plans/2026-03-25-connect-telegram.md 3388, docs/intents/index.html 3339, docs/plans/2026-03-12-parser-recurrence-holidays.md 2897, docs/plans/2026-03-13-notifications.md 2853, docs/plans/2026-03-13-ai-agent.md 2801, test/services/ai/agent-run.test.ts 2684, docs/plans/2026-03-20-ai-assistant-bot-server.md 2534, src/bot/handlers/callback.handler.ts 2469.

Warnings (1200-2400): src/config/constants.ts 2288, test/services/ai/tool-handlers/sharing.test.ts 2247, src/services/ai/agent.ts 1873, src/services/ai/streaming.ts 1418, src/index.ts 1342, test/services/ai/tool-handlers/events.test.ts 1486, test/bot/handlers/message.handler.test.ts 1253, test/regressions/canonical-basis-behavior.test.ts 1249, src/bot/handlers/message.handler.ts 1241, src/services/ai/tool-handlers/events.ts 1225, test/services/ai/tool-handlers/meta.test.ts 1215, docs/specs/05-image-generation.md 1277, docs/specs/03-google-calendar-sync.md 1234, uv.lock 1343 (generated lockfile: NOT exempt; the user's only exemption is .json, so it stays a reported warning), and 12 more files under docs/plans/ (1258-2199 lines; full list in the plan).

Complexity baseline: at ticket creation the shared Rig checker was not available; the original pending-baseline statement is historical, not a claim about current tool availability. Subsequent baseline and verification evidence must identify the exact source revision and count nested functions as well as top-level functions.

ORIGIN: Alex's request for a complexity and oversized-file cleanup, run as a staged multi-agent refactor in worktree cccc-complexity. A search of existing issues for complexity, cccc and oversized found nothing relevant. Plan: docs/plans/2026-10-26-complexity-size-cleanup.md (wave 1 = file-size splits per area; wave 2 = complexity findings by directory).

PSEUDOCODE:
wave 1: for each oversized file in an owner's disjoint set -> split by cohesive responsibility into modules <= 1200 lines (aim <= 1000), keep exports and import paths stable, record before/after test-case counts in Results.
wave 2: run cccc via the Rig checker -> findings per directory -> extract helpers until cognitive <= 15 and cyclomatic <= 20 -> append final measured numbers to Results.

LINKS: repo alex-mextner/HyperCalendarBot; baseline commit 2327f650; tool moznion/cccc. Commits and PRs for this work must say Refs with this issue's number, never Closes/Fixes.

CORRECTION (2026-10-27): an earlier version of this ticket wrongly said lockfiles are exempt and credited that to the user. The user exempted JSON from the file-length rule. uv.lock (1343 lines) is therefore not exempt and remains a warning: it is not fixed, hidden or excluded, and the 1200/2400 limits are unchanged.

ACCEPTANCE AUDIT CORRECTION (2026-10-27): criterion 3 was prematurely checked with a machine-local /tmp path. That check is now reverted. The available comparison normalises anchor links and reports four retargeted links, so it does not prove verbatim preservation. A reviewable comparison and an explicit account of link changes are still owed; no local path is presented as a publicly accessible image proof. Criteria 1, 2, 3, 4 and 5 remain unchecked. A post-merge acceptance reason does not authorize merging a known failing regression suite.

Why (motivation)

Files and functions have grown unchecked because nothing measures them. Giant files (a 5961-line intent doc, a 2469-line callback handler, a 2684-line test) are hard to review, hard to edit safely and expensive for AI agents to load, so changes to them are slower and more error-prone. Enforcing a 1200-line file limit and cognitive<=15 / cyclomatic<=20 per function keeps future changes reviewable.

User impact

Internal refactor, no change visible to people using the calendar bot: no command, reply, reminder or screen changes. Maintainers are affected: today a 2400-line handler or 6000-line document is hard to read and review, and easy to break while editing. After the cleanup, future fixes to things users do notice (reminders, invitations, AI replies) are faster and less likely to introduce new bugs.

Cost of inaction

The limits stay unenforced, so files and functions keep growing; reviews of the biggest handlers and the AI agent get slower and shallower, merge conflicts in giant files grow, and the eventual cleanup gets more expensive. A regression hidden in a long handler becomes more likely to ship unnoticed, which users would experience as the bot misreplying or missing a reminder.

Acceptance criteria

  • Re-running the baseline command over tracked and untracked non-ignored non-.json text files reports 0 files > 1200 lines, with no lockfile exemption - proof: physical-line counts, including an unterminated final line, recorded with the exact source revision. The current uv.lock warning remains unresolved.
  • Every split keeps behavior and public API (no export name or import path used by other code changes; every original test case keeps its name and assertions) - proof: before/after test( and it( counts per split file plus a passing full bun test, lint and typecheck run on server 65.21.40.51
  • Doc-only splits preserve content verbatim - proof: a reviewable byte/line comparison recorded in the PR description; any required link retargeting must be identified explicitly rather than hidden by normalisation
  • Cognitive complexity <= 15 and cyclomatic <= 20 per function are enforced by moznion/cccc via the Rig shared checker with zero complexity findings on main - proof: checker output recorded in the plan's Results table plus a green CI gate run; file-length warnings are reported separately — proof: proof
  • The file-size rule runs automatically: 1201 through 2400 lines warn, above 2400 lines fails CI, and JSON is exempt - proof: negative CI run on a deliberately >2400-line scratch text file, positive run without it, and warning-boundary evidence

Screenshots

(none)

Deploy

Deployed: no

Links

Skipped gates

  • post-merge-acceptance: Final published-main enforcement and the negative/positive generated-CI proof (a CI run failing on a deliberately oversized or over-complex scratch file and passing without it, with the installed Rig on main) can only exist after the rig 775 release and the calendar 862 merge. The uv.lock warning cleanup stays explicitly unresolved. This does not waive pre-merge regression checks or permit closing the task before the remaining evidence exists.
  • screenshots: internal refactor, no rendered surface; evidence is the measured file-line baseline in the body

Activity

  1. alex-mextner commented on Oct 27, 2026

    @alex-mextner
    OwnerAuthor

    Deferred quality-only review findings from pull request 862

    Independent reviews of the startup, constants and worker split found no correctness problems, but listed behaviour-neutral follow-ups. They are not fixed in pull request 862 so that it stays a pure refactor. Impact: maintainability only. Next action: one small follow-up pull request after 862 merges. Source: review job 20261027T064206-7104f361 on the exact diff of commit 862715a.

    1. src/bootstrap/bot-tasks.ts builds BirthdayService before checking whether the MTProto service tier is enabled; check first, construct after.
    2. src/bootstrap/bot-tasks.ts imports worker/bot-tasks-queue.ts twice; import the cron setup functions together with createBotTasksQueue.
    3. The default-language lookup (db.users.findByTelegramId(id)?.language ?? 'en') as Lang appears four times (bot-tasks, google-sync); extract one helper.
    4. The "send through botRef and log failure" closure exists three times (bot-tasks, google-sync, voice-calls); reuse sendThroughBot from bot-ref.ts.
    5. The header of src/bootstrap/google-sync.ts claims Google modules load lazily, but InlineKeyboard, the calendar picker keyboard and ReminderMaterializer are static imports; fix the comment or the imports.
    6. agenda.repository.ts: buildRoster repeats the viewer filter that readBatch already applies; keep one.
    7. config/constants/messages/*/holidays-notify.ts also holds AI-error and notification-settings strings; rename or split by topic.
    8. config/env.ts: optionalNumberEnv (no validation) and parseIntegerEnv (throws) are easy to confuse; rename or document.
    9. The two canonical-basis SQLite test files share an identical database setup block; move it into canonical-basis-helpers.ts.
    10. The comment at src/web/server.ts:263 says route order matters; matching is by exact path, so it does not.
    11. src/worker/bot-tasks-queue.ts: hooks are now resolved once and awaited; type the six () => void hooks as void | Promise<void> so a future async hook is visible in the types.
    12. src/worker/playwright-pool.ts: shouldRetryAfterGenerationLoss mutates state and throws; rename it (for example noteGenerationLoss).
  2. alex-mextner commented on Oct 27, 2026

    @alex-mextner
    OwnerAuthor

    Acceptance status for PR 862 (written 2026-10-27)

    Not complete. This ticket stays open. It tracks the remaining deployment proof and the uv.lock warning. Pre-merge full regression CI still has to pass; main will not ship with code or test failures. Refs only, no closing keywords.

    Factual correction

    The ticket previously said lockfiles are exempt and credited that to the user. That was wrong. The user's only requirement was that JSON is exempt. uv.lock (1343 lines) is not exempt and remains a warning: it is not fixed, hidden, compacted or renamed, and the 1200 / 2400 limits are unchanged. The What section was corrected accordingly.

    Exact tree

    Head 912f5348, tree 545ae3b6af0ca7031103c232687a335d18c7856e (both re-verified with git rev-parse in the worktree).

    Fresh rig lint check (run 2026-10-27 in the worktree with the cccc-lint-check rig build; file /tmp/task860-proof/rig-check.txt)

    WARNING rig.yaml ancestor skipped: /home/ultra/xp/HyperCalendarBot is not owned by you or is writable by others
    uv.lock: warning file-lines/max-lines 1343 lines exceeds the warning limit 1200
    lint check: 0 error(s), 1 warning(s), 0 incomplete, 0 fixed; complexity=pass file_lines=pass (exit 0)
    

    Exit 0: zero errors, zero complexity findings, exactly one warning (uv.lock).

    Docs-split verbatim evidence

    Commit 84a60608 ("text kept verbatim"). Fresh read-only comparison (file /tmp/task860-proof/docs-split-verification.txt): for each of the 20 split docs, every non-blank line of the original (parent of 84a6060) is found, in order, in the index plus part files. 19 of 20 docs: 0 lines missing. docs/plans/2026-03-13-image-generation.md: 4 lines differ, all deliberate cross-reference retargets (spec line numbers such as "lines 758-1151" replaced by section names and part-file paths, because line numbers no longer exist after the split). Part-file anchor links are normalised in this comparison.

    Full suite on the exact tree (server run, read-only inspection)

    The run finished 2026-10-27T16:52:10Z (wall 434s, exit 1): 8213 pass, 16 skip, 7 fail, 8236 tests in 501 files. Failures: Python regression suite test_prebuilt_deploy.py; "the pending status posted when the build starts > posts nothing while stage-qa.yml does not exist on the default branch yet"; "synthesize converts WAV from HF API to OGG Opus buffer"; "synthesize throws if ffmpeg fails"; three SessionBridge temp-file tests (createTempSessionFile x2, cleanupTempFile). The criterion that needs a passing full suite is therefore NOT checked.

  3. alex-mextner commented on Oct 28, 2026

    @alex-mextner
    OwnerAuthor

    Post-merge file-size census on main 53ddfa3 (PR #883 merge), measured over all tracked regular non-JSON text blobs.

    #860 stays open: the criteria about the warning tier (3 files) and the docs original bytes are still unmet. Thresholds and exclusions are unchanged.

  4. alex-mextner commented on Oct 29, 2026

    @alex-mextner
    OwnerAuthor

    Complexity gate evidence on main bb7a9d4 (written 2026-10-29)

    Source: main commit bb7a9d47 (PR #904 merge), tree b487d260a352a3fadcf862aa5c4fdd5354e05ed0.

    Hosted gate: rig-lint-check run 40875271712, event push on main, headSha bb7a9d4, conclusion success. Step "rig lint check" printed ok: true, exit_code: 0, pinned cccc 1.8.4 (sha256 e09bc0ec6e9b8e55a57188b0137fdb38a11bba3cc115be8f6ef28df7f4a1d578), config max_cognitive 15, max_cyclomatic 20, warn_above 1200, error_above 2400, checks.complexity = pass, checks.file_lines = pass, summary 1472 files considered, 0 errors, 1 warning, 0 incomplete. The only finding is file-lines/max-lines warning on uv.lock (1343 lines). There are zero complexity findings.

    A local aggregate on the same tree (rig lint run: 8 of 8 applicable checks pass; rig lint check: 1473 files considered, 0 errors, 1 warning, complexity pass, the same uv.lock warning) agrees with the hosted run.

    The same gate also fails on a real violation: rig-lint-check run 40769298300 (pull request, branch google-event-id, head 81c8d90) exited 1 with checks.complexity = fail, reporting cognitive 43 and cyclomatic 27 in src/services/google/event-id-backfill.ts. That branch was fixed before merge.

    The plan's Results table (docs/plans/2026-10-26-complexity-size-cleanup.md on main) has the dated row "Complexity after the cleanup: 0 violations over 1109 files, nested functions included"; the row names no revision, so the revision-exact numbers are the ones above.

    Not claimed here: no hosted CI run on a deliberately over-2400-line scratch file exists, so the file-size automation criterion stays open; uv.lock (1343) stays a warning, so the 1200-line criterion stays open.

  5. alex-mextner commented on Oct 29, 2026

    @alex-mextner
    OwnerAuthor

    screenshot: 860#issuecomment-6497353091

  6. alex-mextner commented on Oct 29, 2026

    @alex-mextner
    OwnerAuthor

    screenshot: 40875271712

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

Metadata

Metadata

Assignees

No one assigned

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions