Skip to content

audit: Create CLAUDE.md#19

Open
AlexaVJ wants to merge 1 commit into
masterfrom
audit/code-quality
Open

audit: Create CLAUDE.md#19
AlexaVJ wants to merge 1 commit into
masterfrom
audit/code-quality

Conversation

@AlexaVJ

@AlexaVJ AlexaVJ commented May 6, 2026

Copy link
Copy Markdown

Summary by CodeRabbit

  • Documentation
    • Added developer guidance documentation to support the development workflow and repository conventions.

@coderabbitai

This comment was marked as resolved.

@deepsource-io

This comment was marked as resolved.

@AlexaVJ

AlexaVJ commented May 6, 2026

Copy link
Copy Markdown
Author

@coderabbitai what do you think about the architecture of this project?

@coderabbitai

coderabbitai Bot commented May 6, 2026

Copy link
Copy Markdown

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Read CLAUDE.md
cat CLAUDE.md

echo "---TREE---"
# Get top-level structure
tree -L 3 --dirsfirst -I 'node_modules|dist|.git' 2>/dev/null || find . -not -path '*/node_modules/*' -not -path '*/.git/*' -not -path '*/dist/*' -maxdepth 3 | sort

echo "---PACKAGE---"
cat package.json 2>/dev/null || true

echo "---AURELIA CONFIG---"
cat aurelia_project/aurelia.json 2>/dev/null || true

echo "---SRC TREE---"
find src -type f | sort 2>/dev/null || true

Length of output: 31521


@AlexaVJ Here's a breakdown of the project's architecture along with some observations:


🏗️ Overall Architecture

This is an Aurelia.js Single Page Application (SPA) bundled with Webpack 4. The entry point is src/main.js, which bootstraps Aurelia and sets src/app.js as the root component.

graph TD
    main["src/main.js<br/>(Bootstrap)"] --> app["src/app.js<br/>(Root + Router)"]
    app --> auth_step["src/authorize-step.js<br/>(Role Guard)"]
    app --> modules["src/modules/<feature>/<br/>(Feature Modules)"]
    modules --> services["src/services/<br/>(Business Logic / HTTP)"]
    modules --> models["src/models/<br/>(DTOs)"]
    modules --> resources["src/resources/<br/>(Custom Elements & Attributes)"]
    services --> config["src/config/api.js<br/>(Endpoint Constants)"]
    services --> auth["src/services/auth.js<br/>(JWT + Socket.IO)"]
Loading

✅ Strengths

  1. Clear Separation of Concerns: Services, models, modules, resources, and config are each in their own directories with well-defined responsibilities.
  2. Centralized Configuration: All API endpoints live in src/config/api.js and all user-facing strings (in Spanish) in src/config/messages.js, making them easy to maintain.
  3. Role-based Routing: src/authorize-step.js enforces access control by checking settings.roles on each route before navigation, which is a clean pattern.
  4. Service Singletons via DI: All 13 services (alert, auth, categories, contests, http, jwt, materials, problems, rankings, syllabuses, etc.) are registered as singletons through Aurelia's @inject() decorator, which is idiomatic and avoids manual wiring.
  5. Real-time Support: Socket.IO is integrated in src/services/auth.js post-login, with event listeners in src/app.js for submission results.
  6. Feature Modularity: 18+ feature modules (contest, syllabus, problems, material, ranking, etc.) each define their own nested router, keeping concerns scoped.

⚠️ Areas of Concern

  1. Duplicate Environment Files: There are two environment files — src/enviroment.js (likely a typo) and src/environment.js. This could lead to subtle bugs if they diverge.

  2. @ng-bootstrap in a Non-Angular App: @ng-bootstrap/ng-bootstrap is listed as a dependency, but this is an Angular-specific package and should not function in an Aurelia app. It may be unused dead weight in package.json.

  3. Test Runner Mismatch: aurelia_project/aurelia.json still declares Karma + Jasmine as the test runner, but the actual test infrastructure in package.json and test/ uses Jest + babel-jest. This config drift could confuse contributors.

  4. Very Thin Test Coverage: There is only a single test file, test/unit/app.spec.js. Given the size of the codebase (18+ modules, 13+ services), this is a significant gap in test coverage.

  5. Naming Convention Inconsistency: One module is named SubmissionsStudents (PascalCase directory) while all others use kebab-case (e.g., contest, syllabus). This is a minor but noticeable inconsistency.

  6. Mixed-Language Codebase: Code is in English while comments, UI strings, and route names are in Spanish. This is noted as intentional in CLAUDE.md, but it could be a barrier for international contributors.


📊 Module Count Summary

Layer Count
Feature modules 18+
Services 13
Models 10
Custom elements 8
Custom attributes 2
Test spec files 1

Overall, the architecture is solid and well-organized for its size, with the main risks being the low test coverage and a few configuration inconsistencies worth cleaning up.

@AlexaVJ

AlexaVJ commented May 6, 2026

Copy link
Copy Markdown
Author

@coderabbitai what are the main maintainability risks in this codebase?

@coderabbitai

coderabbitai Bot commented May 6, 2026

Copy link
Copy Markdown

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
echo "=== TODO/FIXME/HACK comments ==="
rg -n 'TODO|FIXME|HACK|XXX|WORKAROUND' --type js -g '!node_modules' 2>/dev/null | head -60

echo ""
echo "=== Hardcoded URLs or credentials ==="
rg -n 'http://|https://' src/ --type js | grep -v 'node_modules' | head -40

echo ""
echo "=== Files by line count (largest first) ==="
find src -name '*.js' | xargs wc -l 2>/dev/null | sort -rn | head -30

echo ""
echo "=== Direct console.log usage ==="
rg -n 'console\.log' src/ --type js | head -30

echo ""
echo "=== Error handling - catch blocks ==="
rg -n '\.catch\(' src/ --type js | head -30

echo ""
echo "=== Pinned vs range dependencies in package.json ==="
cat package.json | python3 -c "
import json, sys
data = json.load(sys.stdin)
deps = {**data.get('dependencies', {}), **data.get('devDependencies', {})}
for k, v in sorted(deps.items()):
    print(f'{k}: {v}')
"

echo ""
echo "=== Any use of localStorage/sessionStorage directly ==="
rg -n 'localStorage|sessionStorage' src/ --type js | head -20

echo ""
echo "=== Services that are injected - checking coupling ==="
rg -n '@inject' src/ --type js | head -30

echo ""
echo "=== Any inline styles or magic numbers ==="
rg -n 'style=' src/ | grep -v node_modules | head -20

echo ""
echo "=== Check for any try/catch blocks ==="
rg -n 'try {|try{' src/ --type js | head -20

echo ""
echo "=== Check environment file difference ==="
cat src/environment.js 2>/dev/null || true
echo "---"
cat src/enviroment.js 2>/dev/null || true

echo ""
echo "=== Number of lines per module ==="
for dir in src/modules/*/; do
  count=$(find "$dir" -name '*.js' | xargs wc -l 2>/dev/null | tail -1 | awk '{print $1}')
  echo "$count $dir"
done | sort -rn

Length of output: 261967


@AlexaVJ Here is a detailed breakdown of the main maintainability risks I identified from analyzing the codebase:


🔴 Critical Risks

1. Severe Code Duplication — contest-school-problem & schools-view-problem

The two largest modules in the codebase are near-copies of each other:

Signal Evidence
Same size 552 vs 459 lines
Identical TODOs Both have // TODO: Assemble Python into code variable. and // TODO: Change ORDER_NONE to the correct strength.
Same hardcoded CDN URL Both reference https://blockly-demo.appspot.com/static/media/
Same sessionStorage pattern Both use sessionStorage.getItem("xmlCode") / sessionStorage.setItem("xmlCode", ...) directly

Any bug fix or feature change in one must be manually replicated in the other — a classic maintenance trap.


2. Divergent Environment Files (Typo Creates a Logic Fork)

Two environment files exist and they are not equivalent:

// src/environment.js (correct spelling — minimal)
export default { debug: true, testing: true };

// src/enviroment.js (typo — has the actual backend URL!)
export default {
  debug: true, testing: true,
  API_BACKEND_URL_DEVELOPMENT: "http://146.148.49.88:9191/api/"
};

The misspelled file carries the hardcoded production backend IP. If a developer imports the correctly-spelled file, they silently miss the API URL. This is a latent configuration bug.


3. Hardcoded Infrastructure IP Address

src/enviroment.js contains a hardcoded development server IP (http://146.148.49.88:9191/api/). If this server changes, every developer has to update the file manually, and the address is currently exposed in version control.


🟠 High Risks

4. Near-Zero Test Coverage

Only 1 test file (test/unit/app.spec.js) exists for 18+ feature modules, 13 services, and 10 models (>13,000 lines of source code). Refactoring any service or module carries a very high regression risk with no safety net.

5. Severely Outdated Toolchain

Package Current in repo Latest
webpack ^4.46.0 v5
socket.io-client ^2.0.4 v4
aurelia-fetch-client ^1.1.0 ~5 years old
bootstrap ^4.5.3 v5

Webpack 4 and socket.io v2 both have known security and compatibility issues. Upgrading socket.io from v2 to v4 is a breaking change that would require code changes in src/services/auth.js and src/app.js.

6. Unresolved TODOs Blocking Features

The Blockly Python code assembly is explicitly left incomplete in both duplicated modules:

// TODO: Assemble Python into code variable.
// TODO: Change ORDER_NONE to the correct strength.

These represent known gaps in functionality that have no tracking mechanism beyond the comment itself.


🟡 Medium Risks

7. sessionStorage Accessed Without Abstraction

While JWT token storage is cleanly abstracted through src/services/jwt.js, sessionStorage is accessed raw in two module files:

// In both schools-view-problem.js and contest-school-problem.js
sessionStorage.setItem("xmlCode", xml_text);
sessionStorage.getItem("xmlCode");

This bypasses the service layer, making it impossible to mock in tests and hard to change the storage strategy.

8. Incompatible Dependency (@ng-bootstrap)

@ng-bootstrap/ng-bootstrap: ^10.0.0 is an Angular-specific library that cannot function in an Aurelia app. It clutters node_modules, inflates bundle size, and could mislead future developers about the framework being used.

9. File Size / Complexity Hotspots

The following files are likely to need splitting:

File Lines
contest-school-problem.js 552
schools-view-problem.js 459
problems-editor.js 438
syllabuses.js (service) 433
auth.js (service) 382
app.js 313

Large files with mixed concerns (UI logic + API calls + DOM manipulation) are harder to test, review, and modify safely.

10. No Linting or Test Enforcement in CI

ESLint and Jest are configured but there is no evidence of CI enforcement. Without gates, code style drift and broken tests can accumulate silently.


📊 Risk Summary

quadrantChart
    title Maintainability Risks
    x-axis Low Impact --> High Impact
    y-axis Low Likelihood --> High Likelihood
    quadrant-1 Critical
    quadrant-2 Monitor
    quadrant-3 Low Priority
    quadrant-4 Address Soon
    Code Duplication: [0.85, 0.9]
    Zero Test Coverage: [0.8, 0.95]
    Env File Divergence: [0.7, 0.8]
    Outdated Toolchain: [0.75, 0.6]
    sessionStorage Leakage: [0.4, 0.85]
    ng-bootstrap Dep: [0.3, 0.7]
    Hardcoded IP: [0.6, 0.75]
    Large Files: [0.5, 0.7]
Loading

The top 3 items to address first for maintainability would be:

  1. Resolve the env file typo and centralize the backend URL into a proper config system.
  2. Extract a shared Blockly module from the two duplicated files.
  3. Establish a baseline test suite for at least the services layer before any further refactoring.

@AlexaVJ

AlexaVJ commented May 6, 2026

Copy link
Copy Markdown
Author

@coderabbitai what are the main performance risks in this codebase?

@coderabbitai

coderabbitai Bot commented May 6, 2026

Copy link
Copy Markdown

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
echo "=== Webpack bundle config ==="
cat webpack.config.js 2>/dev/null || cat webpack.config.ts 2>/dev/null || echo "Not found at root"
find . -name 'webpack*.js' -not -path '*/node_modules/*' | xargs cat 2>/dev/null

echo ""
echo "=== Aurelia bundle config ==="
cat aurelia_project/aurelia.json 2>/dev/null | python3 -c "import json,sys; d=json.load(sys.stdin); print(json.dumps(d.get('build',{}), indent=2))" 2>/dev/null || true

echo ""
echo "=== Route lazy loading / activationStrategy ==="
rg -n 'activationStrategy|moduleId|route|lazy|navigationStrategy' src/ --type js | grep -v node_modules | head -60

echo ""
echo "=== HTTP calls - are there any N+1 patterns? ==="
rg -n '\.get\(|\.post\(|\.put\(|\.delete\(|httpClient\|fetch(' src/services/ --type js | head -60

echo ""
echo "=== Loops that contain HTTP calls ==="
rg -n 'for\s*\(|forEach\|\.map\(' src/modules/ --type js -A 3 | grep -A2 'fetch\|\.get\(\|\.post\(' | head -40

echo ""
echo "=== Event listeners that may not be cleaned up ==="
rg -n 'addEventListener|socket\.on\|\.on(' src/ --type js | grep -v node_modules | head -40

echo ""
echo "=== detached() / unbind() / deactivate() cleanup hooks ==="
rg -n 'detached\(\|unbind\(\|deactivate\(' src/ --type js | head -30

echo ""
echo "=== Blockly and CDN references ==="
rg -n 'blockly|Blockly|appspot|cdn' src/ --type js -i | head -30

echo ""
echo "=== Image handling / large assets ==="
find src -name '*.jpg' -o -name '*.png' -o -name '*.gif' -o -name '*.svg' 2>/dev/null | head -20
find . -not -path '*/node_modules/*' -name '*.jpg' -o -name '*.png' 2>/dev/null | head -20

echo ""
echo "=== Any setInterval or setTimeout usage ==="
rg -n 'setInterval|setTimeout' src/ --type js | head -20

echo ""
echo "=== Polling patterns ==="
rg -n 'poll|interval|repeat\.for' src/ --type js -i | head -20

echo ""
echo "=== repeat.for in HTML templates (potential large list rendering) ==="
find src -name '*.html' | xargs rg -n 'repeat\.for' 2>/dev/null | head -30

echo ""
echo "=== Virtual scrolling or pagination ==="
rg -n 'virtual|pagination|page|limit|offset' src/ --type js -i | grep -v node_modules | head -30

echo ""
echo "=== Socket.IO event subscriptions in app.js ==="
cat src/app.js 2>/dev/null || true

echo ""
echo "=== Auth service for socket handling ==="
cat src/services/auth.js 2>/dev/null | head -100

echo ""
echo "=== Check if there is code splitting / dynamic imports ==="
rg -n 'import(' src/ --type js | head -20

echo ""
echo "=== Check for any memory-intensive data structures ==="
rg -n 'new Array|new Map|new Set|new WeakMap' src/ --type js | head -20

echo ""
echo "=== Check contest-school-problem for Blockly workspace ==="
cat src/modules/contest-school-problem/contest-school-problem.js 2>/dev/null | head -100

echo ""
echo "=== Check for any debounce/throttle usage ==="
rg -n 'debounce|throttle' src/ --type js -i | head -20

echo ""
echo "=== Check for any observeArray or collection-observer usage ==="
rg -n 'observeArray|collectionObserver|propertyObserver' src/ --type js | head -20

echo ""
echo "=== Check rankings and large data fetches ==="
cat src/services/rankings.js 2>/dev/null || true
cat src/modules/ranking/ 2>/dev/null || true
find src/modules/ranking -type f | xargs cat 2>/dev/null | head -100

Length of output: 279614


@AlexaVJ Here are the main performance risks I see, prioritized, with where they show up and how to fix them.

High impact, high likelihood

  1. Intervals and animations without cleanup (CPU + memory leaks on navigation)
  • Evidence:
    • setInterval in:
      • src/modules/syllabus/view-problem/view-problem.js Line [47]
      • src/modules/contest/contest-problem/contest-problem.js Line [75]
      • src/modules/contest/contest-school-problem/contest-school-problem.js Line [288]
      • src/modules/principal/principal-high-school/transicion.js Line [25]
      • src/resources/elements/clock.js Line [34]
      • src/resources/elements/canvas-principal.js Lines [258], [262]
      • src/modules/problems/problems-creator/problems-editor.js Lines [75], [86]
    • No detach/unbind/deactivate lifecycle hooks found to clear them.
  • Fix:
    • Store interval IDs and clear them in detached() or deactivate(), and remove any DOM/event listeners.
    • Example (Aurelia 1):
      export class ViewProblem {
        intervalId;
        attached() {
          this.intervalId = setInterval(() => { /* work */ }, 1000);
        }
        detached() {
          if (this.intervalId) clearInterval(this.intervalId);
        }
      }
    • For canvas (canvas-principal.js), prefer requestAnimationFrame + cancelAnimationFrame and throttle expensive draws.
  1. Heavy libraries loaded for everyone (large initial bundle)
  • Evidence:
    • webpack.config.js has entry.vendors = ['jquery','bootstrap'] so these ship on every page.
    • splitChunks top-level uses chunks: "initial" (HTTP/1.1 profile). performance.hints are disabled.
  • Risk:
    • Slower first paint on routes that don’t need jQuery/Bootstrap JS.
  • Fix:
    • Remove the explicit vendors entry and let SplitChunks build vendor chunks automatically.
    • Switch splitChunks to HTTP/2-friendly settings (per-package vendor split) and allow async splitting:
      • Set splitChunks.chunks: 'all'.
      • Enable the commented “HTTP/2 optimized” cacheGroups (vendorSplit/vendorAsyncSplit/commonAsync) or similar.
    • Only load Bootstrap JS if you actually use its JS components (Bootstrap CSS can stay extracted).
  1. Blockly statically imported (very large payload when not split)
  • Evidence:
    • src/modules/problems/schools-view-problem/schools-view-problem.js imports 'blockly/core', 'blockly/blocks', 'blockly/python', 'blockly/javascript', 'blockly/msg/es'.
  • Risk:
    • Blockly is big; if webpack doesn’t carve it into an async chunk, non‑Blockly routes pay the cost.
  • Fix:
    • Lazy-load Blockly only on the page that needs it (dynamic import + code-split). Example:
      async attached() {
        const core = await import(/* webpackChunkName: "blockly" */ 'blockly/core');
        await Promise.all([
          import(/* webpackChunkName: "blockly" */ 'blockly/blocks'),
          import(/* webpackChunkName: "blockly" */ 'blockly/python'),
          import(/* webpackChunkName: "blockly" */ 'blockly/javascript'),
          import(/* webpackChunkName: "blockly" */ 'blockly/msg/es'),
        ]);
        // init Blockly with core.default or core as appropriate
      }
    • Keep using PLATFORM.moduleName for routes, but verify with the bundle analyzer that Blockly sits in an async chunk.

Medium impact
4) Large, likely unoptimized images

  • Evidence:
    • Many PNGs in src/assets/img/* (e.g., Maratón 2016/2018/2019, backgrounds).
  • Risk:
    • Big transfer + decode time, especially on mobile.
  • Fix:
    • Add image-webpack-loader (mozjpeg/optipng/svgo) in production.
    • Convert heavy PNGs to WebP/AVIF where possible; serve multiple sizes; add loading="lazy" to in templates.
  1. Spell-check dictionaries downloaded at runtime
  • Evidence:
    • simplemde.min.js (CodeMirror spell-checker) fetches en_US.aff/.dic from cdn.jsdelivr via XHR.
  • Risk:
    • Extra network roundtrips on editor load; fragile offline; may fire CORS/caching edge cases.
  • Fix:
    • Disable spellChecker where not needed, or bundle dictionaries locally and import them so they’re cached by your app.
  1. Production source maps and hidden perf warnings
  • Evidence:
    • devtool: 'nosources-source-map' in production; performance.hints: false.
  • Risk:
    • Larger build outputs; oversize assets go unnoticed.
  • Fix:
    • Consider devtool: false (or separate map upload) for production builds.
    • Enable performance.hints in prod to catch regressions.

Lower impact / nice-to-haves
7) Base64 inlined assets

  • Evidence:
    • url-loader inlines images <= 8kb, fonts <= 10kb into JS.
  • Risk:
    • Too many inlined assets inflate JS parse time; not individually cacheable.
  • Fix:
    • Keep thresholds small (they are), but audit if many assets are at the edge; prefer file-loader + HTTP/2 for most.

How to validate quickly

  • Run the analyzer to see real chunk sizes: build with analyze enabled (webpack has BundleAnalyzerPlugin wired).
    • Example: npm run build -- --env.production --env.analyze
  • While navigating between routes, use the browser Performance panel:
    • Watch for CPU spikes and detached DOM nodes when leaving pages with intervals/canvas; confirm the leaks go away after adding detached()/deactivate() cleanup.
  • Lighthouse/WebPageTest on a production build (gzip on) to quantify FCP and JS execution weight.

If you want, I can draft a focused patch plan:

  • Add lifecycle cleanup to the files listed in point 1.
  • Switch splitChunks to chunks: 'all' and remove the vendors entry; enable the HTTP/2 vendorSplit groups.
  • Convert Blockly imports to dynamic imports in the school-view/problem module.
  • Add image optimization to the webpack prod rules and suggest lazy image loading patterns.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant