From 7d46d4e2a6d46aa056871015b749e916d1294756 Mon Sep 17 00:00:00 2001 From: Stefan Date: Tue, 29 Sep 2026 14:49:54 +0100 Subject: [PATCH 1/2] Add a Jev code grader for source and spec files `bin/grade-code` scores every Ruby, ERB and JavaScript file under app, spec and lib against the house rules in CLAUDE.md, using one Jev systemone call per file with all applicable questions in that call. There are no mechanical checks: CodeStandardsChecker, standardrb and the custom cops already cover line length, method length and hardcoded strings. What is left is judgement, and that is what the rubric asks Jev about - idiomatic Rails, small single-purpose methods, short comments that explain WHY and carry no history notes, plain technical English, no dead code or defensive fallbacks, I18n for user-facing strings, class-free semantic HTML, the form partial conventions, and for specs: not tautological, succinct names, a clear story and real edge cases. Files are graded one at a time. A single file prints the full per-check report; a batch prints a worst-first table with the most-failed checks across the run, and can write CSV or JSON. Discovery skips vendored and non-source trees (app/assets, *.min.js, the upstream highlight.js). flake.nix declares python3 in the dev shell - it was only present transitively - and the shell banner lists the command. --- CLAUDE.md | 28 ++ bin/grade-code | 936 +++++++++++++++++++++++++++++++++++++++++++++++++ flake.nix | 4 +- 3 files changed, 966 insertions(+), 2 deletions(-) create mode 100755 bin/grade-code diff --git a/CLAUDE.md b/CLAUDE.md index c56e2e12..de441098 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -456,6 +456,34 @@ bin/coverage_check app/controllers/users_controller.rb - **Priority files**: Controllers, services, models with business logic - **Low coverage files**: Immediate attention required for < 80% coverage +### Code Quality Grading + +**Jev Grader** (`bin/grade-code`) - scores source and spec files against the +house rules with Jev, one file per call. No mechanical checks: those are +CodeStandardsChecker, standardrb and `bin/lint`. + +- **Usage**: `bin/grade-code [files or dirs]` - defaults to `app spec lib` +- **Single file**: prints the full per-check report; **2+ files**: worst-first + ranked table with the most-failed checks across the batch + +```bash +bin/grade-code # whole source and test tree +bin/grade-code app/models --workers 6 # parallel sweep of a directory +bin/grade-code --kinds model,controller # only these kinds +bin/grade-code app/models/inspection.rb # one file in detail +bin/grade-code spec --limit 20 --csv tmp/grades.csv # first 20, written to CSV +bin/grade-code --list-checks # the rubric +``` + +- **What it scores**: idiomatic Rails, nice simple methods, short comments that + explain WHY with no history notes, plain technical English, no dead code or + defensive fallbacks, I18n for user-facing strings, semantic HTML, form + partial conventions, and for specs: not tautological, succinct names, happy + path plus edge cases +- **Needs** `OPENCODE_API_KEY` (or `/run/secrets/opencode_api_key`, or `.env`) +- **Cost**: ~80s for the whole 560-file tree at `--workers 6`; advisory only, + nothing is blocked on the score + ### Code Organization - **Create partials for repeated code** - DRY principle diff --git a/bin/grade-code b/bin/grade-code new file mode 100755 index 00000000..e6d6633a --- /dev/null +++ b/bin/grade-code @@ -0,0 +1,936 @@ +#!/usr/bin/env python3 +""" +grade-code — grade this repo's source and spec files against the house coding +rules in CLAUDE.md using Jev (TypeSafe System One) via the OpenCode zen API. + +Every check is a judgement call, not a grep: one Jev systemone call per file, +one question per check, all questions for that file asked in a single call. +The script carries no mechanical checks — the repo already runs those +(CodeStandardsChecker, standardrb, the custom cops, bin/lint). What is left is +the taste: idiomatic Rails, small single-purpose methods, short comments that +explain WHY and carry no history notes, plain technical English, no dead code +or defensive fallbacks, I18n for every user-facing string, semantic HTML, and +specs that are not tautological and read as a clear story. + +Targets are files or directories; the default is the whole of app, spec and +lib. Files are graded one at a time. + +Usage: + ./bin/grade-code # app, spec, lib + ./bin/grade-code app/models spec/features # chosen dirs + ./bin/grade-code app/models/inspection.rb # full single-file report + ./bin/grade-code --kinds model,controller,service # only these kinds + ./bin/grade-code --limit 20 # first 20 discovered + ./bin/grade-code spec --workers 6 # parallel (default 1) + ./bin/grade-code --csv grades.csv --json + ./bin/grade-code --list-checks # print the rubric + +The Jev API key is read from OPENCODE_API_KEY, then /run/secrets/opencode_api_key, +then a .env file in the repo root. Batch mode always exits 0 (it is a report); +single-file mode exits 2 when the file could not be graded. +""" + +import argparse +import csv +import json +import os +import re +import sys +import time +import urllib.error +import urllib.request +from collections import Counter, OrderedDict +from concurrent.futures import ThreadPoolExecutor, as_completed + +ZEN_SYSTEMONE_URL = "https://opencode.ai/zen/v1/systemone" +DEFAULT_MODEL = "jev-1.13" +DEFAULT_KEY_FILE = "/run/secrets/opencode_api_key" + +REPO_ROOT = os.path.dirname(os.path.dirname(os.path.abspath(__file__))) +DEFAULT_TARGETS = ["app", "spec", "lib"] + +MAX_SOURCE_CHARS = 60000 +GRADABLE_EXTS = (".rb", ".erb", ".js") +SKIP_DIR_NAMES = { + ".bundle", ".git", "coverage", "gems", "log", "node_modules", + "public", "sorbet", "spencers_app", "storage", "tmp", "vendor", +} +SKIP_PATH_PREFIXES = ("app/assets/",) +# third-party code, not house code: the upstream highlight.js distribution +VENDORED_FILES = ("app/javascript/highlight.js",) + +KIND_LABELS = { + "controller": "Rails controller", + "helper": "Rails helper", + "job": "Rails job", + "lib": "library code", + "model": "Rails model", + "serializer": "Rails serializer", + "service": "Rails service object", + "spec": "RSpec spec", + "spec_feature": "RSpec feature spec", + "spec_support": "RSpec support code (helper, factory, harness)", + "view": "ERB template", + "javascript": "JavaScript", +} + +RAILS_KINDS = { + "controller", "helper", "job", "model", "serializer", "service", +} +RUBY_KINDS = RAILS_KINDS | {"lib", "spec", "spec_feature", "spec_support"} +APP_KINDS = RAILS_KINDS | {"lib"} +TEST_KINDS = {"spec", "spec_feature"} +ANY_KINDS = RUBY_KINDS | {"javascript", "view"} + +FORM_RE = re.compile(r"form_with|form_context|simple_form| gradable files with a kind +# --------------------------------------------------------------------------- + + +def classify(rel_path): + """Map a repo-relative path to a file kind, or None if not gradable. + + Only the app, spec and lib trees carry house coding rules worth grading: + config, migrations, seeds data and vendored assets do not. + """ + if rel_path.startswith(SKIP_PATH_PREFIXES): + return None + if not rel_path.endswith(GRADABLE_EXTS): + return None + if rel_path.endswith(".min.js") or rel_path in VENDORED_FILES: + return None + if rel_path.startswith("app/views/"): + return "view" + if rel_path.startswith("app/javascript/"): + return "javascript" + if rel_path.startswith("spec/"): + if rel_path.startswith("spec/features/"): + return "spec_feature" + support = ("spec/support/", "spec/factories/", "spec/fixtures/") + if rel_path.startswith(support): + return "spec_support" + if rel_path in ("spec/rails_helper.rb", "spec/spec_helper.rb"): + return "spec_support" + return "spec" + for prefix, kind in ( + ("app/models/", "model"), + ("app/controllers/", "controller"), + ("app/services/", "service"), + ("app/helpers/", "helper"), + ("app/jobs/", "job"), + ("app/serializers/", "serializer"), + ): + if rel_path.startswith(prefix): + return kind + if rel_path.startswith("lib/"): + return "lib" + return None + + +def discover(targets, kinds=None): + """Expand file and dir targets into (abs_path, rel_path, kind) triples.""" + found = [] + for target in targets: + path = target if os.path.isabs(target) else os.path.join(REPO_ROOT, target) + if os.path.isfile(path): + candidates = [path] + elif os.path.isdir(path): + candidates = [] + for root, dirs, files in os.walk(path): + dirs[:] = [d for d in dirs if d not in SKIP_DIR_NAMES] + candidates.extend(os.path.join(root, f) for f in files) + else: + print(f"no such file or directory: {target}", file=sys.stderr) + continue + for candidate in candidates: + rel = os.path.relpath(candidate, REPO_ROOT) + kind = classify(rel) + if kind and (not kinds or kind in kinds): + found.append((candidate, rel, kind)) + return sorted(set(found)) + + +def read_source(abs_path): + """Read a file for grading. Returns (source, truncated).""" + with open(abs_path, encoding="utf-8", errors="replace") as f: + text = f.read() + if len(text) <= MAX_SOURCE_CHARS: + return text, False + return text[:MAX_SOURCE_CHARS], True + + +# --------------------------------------------------------------------------- +# The rubric +# +# engine "jev" -> the question is sent to Jev (one call per file, all +# questions together) and the answer is graded against the thresholds below. +# +# For noul checks the probability is the goodness, unless invert is set: then +# the question asks for a *bad* thing and a low probability is good. +# score checks grade a 0..N level against score_pass / score_warn. +# --------------------------------------------------------------------------- + +CHECKS = [ + dict(id="idiomatic_rails", label="Idiomatic Rails", types=RAILS_KINDS, weight=6, + question=dict( + type="score", + instructions=( + "Is this Rails code idiomatic? House standards: scopes for " + "non-trivial queries, `find_by` rather than `where(...).first`, " + "validations for data integrity, meaningful associations, " + "RESTful actions, business logic in models or services rather " + "than controllers, service objects for work that spans models, " + "delegation where it earns its place, and ActiveRecord idioms " + "instead of hand-rolled SQL." + ), + criteria=[ + "Not idiomatic - hand-rolled SQL, non-RESTful actions, logic in the wrong layer", + "Mostly conventional with a few unidiomatic spots", + "Solid, idiomatic Rails throughout", + "Textbook - conventions, scopes, validations and delegation used exactly where they belong", + ], + ), + score_pass=2, score_warn=1), + dict(id="method_simplicity", label="Nice simple methods", types=RUBY_KINDS, weight=6, + question=dict( + type="score", + instructions=( + "Are the methods nice and simple? Each should do one thing, stay " + "short (house limit is 20 lines), use guard clauses instead of " + "nesting, and carry a descriptive name. Complex conditions belong " + "in a named predicate method rather than inline in a branch." + ), + criteria=[ + "Long multi-purpose methods, deep nesting, cryptic names", + "One or two methods are too long or do too much", + "Methods are short, single-purpose and well named", + "Every method is a small obvious unit whose name says what it does", + ], + ), + score_pass=2, score_warn=1), + dict(id="comments_why_not_what", label="Comments explain WHY", types=ANY_KINDS, weight=4, + question=dict( + type="noul", + instructions=( + "Do the comments in `source` earn their place - do they explain " + "WHY (business context, a regulatory requirement, a non-obvious " + "decision a reader could not infer) rather than restating WHAT " + "the next line of code already says? Judge only the comments that " + "are there: a file with no comments passes, because it neither " + "explains nor misleads. Comments that merely narrate the code - " + "'# increment the counter', '# verify we're on the right tab' - " + "count against it." + ), + criteria={ + "true": "Comments that exist carry real reasons, or there are none", + "false": "Comments narrate what the code does, or repeat it back", + }, + ), + threshold=0.5), + dict(id="comments_short_plain", label="Short comments, plain English", types=ANY_KINDS, weight=3, + question=dict( + type="noul", + instructions=( + "Are the comments short and in simple technical English - a " + "sentence or two, plain words, British spelling - rather than " + "essays, chatty prose, jargon-stacking or American spelling? " + "Judge only the comments that are there: a file with no comments " + "passes." + ), + criteria={ + "true": "Comments are brief, plain and use British spelling", + "false": "Comments are long-winded, chatty, flowery or American", + }, + ), + threshold=0.5), + dict(id="no_history_notes", label="No history notes or stale TODOs", types=ANY_KINDS, weight=4, + question=dict( + type="noul", + instructions=( + "Are the comments free of history notes - 'changed this to that', " + "'previously we did X', notes about who changed what and when, " + "dated or stale TODO/FIXME notes, and commentary that the next " + "refactor will make wrong?" + ), + criteria={ + "true": "No history notes, changelog-style comments or stale TODOs", + "false": "Changelog-style notes, dated commentary or stale TODOs", + }, + ), + threshold=0.5), + dict(id="no_dead_code", label="No dead code or speculative abstraction", types=ANY_KINDS, weight=4, + question=dict( + type="noul", + instructions=( + "Does `source` carry dead weight - methods nothing calls, " + "commented-out code, speculative abstractions, or a fallback for " + "a case that cannot happen?" + ), + criteria={ + "true": "Dead code, commented-out blocks or needless abstraction", + "false": "Nothing unused - every method earns its place", + }, + ), + invert=True, threshold=0.5), + dict(id="no_defensive_coding", label="No defensive coding or silent fallbacks", types=RUBY_KINDS, weight=4, + question=dict( + type="noul", + instructions=( + "Does `source` defend against problems that cannot happen - " + "`respond_to?` checks against interfaces the app owns, rescues " + "that swallow errors, nil-guards on values the app guarantees, " + "or `||` fallbacks that hide missing data? House style prefers " + "crashing loudly over quietly failing." + ), + criteria={ + "true": "Defensive checks, silent rescues or fallbacks that hide failures", + "false": "Trusts its own interfaces and lets errors bubble up", + }, + ), + invert=True, threshold=0.5), + dict(id="modern_ruby", label="Modern, tidy Ruby", types=RUBY_KINDS, weight=3, + question=dict( + type="score", + instructions=( + "How modern and tidy is the Ruby? House style prefers endless " + "methods for simple computation, shorthand hashes (`{width:, " + "height:}`), numbered block parameters (`map { _1.upcase }`), " + "pattern matching for dispatch, `filter_map`, rightward " + "assignment where it reads well, and no backslash continuations " + "or parameter-alignment padding." + ), + criteria=[ + "Dated Ruby throughout - old syntax the language no longer needs", + "Mostly legacy syntax; modern idioms only where the old ones bite", + "Acceptable - modern idioms where they help, some dated syntax left", + "Consistently the newest tidy syntax, in the right places", + ], + ), + score_pass=2, score_warn=1), + dict(id="naming_british", label="British, descriptive naming", types=ANY_KINDS, weight=3, + question=dict( + type="noul", + instructions=( + "Are the identifiers in `source` British English and idiomatic " + "for their language - snake_case in Ruby, camelCase in " + "JavaScript - with no American spellings (color, organize, " + "initialize) and no opaque abbreviations? Ordinary names such as " + "`name`, `email` or `user` count as fine; only genuinely cryptic " + "or misleading names count against it." + ), + criteria={ + "true": "Names are British, idiomatic for the language and readable", + "false": "American spellings or opaque abbreviations", + }, + ), + threshold=0.5), + dict(id="sorbet_sigs", label="Sorbet sigil and sigs", types=APP_KINDS, weight=3, + question=dict( + type="noul", + instructions=( + "Does `source` follow the repo's Sorbet convention: a `# typed:` " + "sigil at the top of the file, and `sig` blocks on the methods of " + "a file marked `# typed: strict` or `# typed: strong`?" + ), + criteria={ + "true": "Typed sigil present, and sigs wherever the file type requires them", + "false": "Missing the typed sigil, or methods in a strict/strong file lack sigs", + }, + ), + threshold=0.5), + dict(id="i18n_strings", label="I18n for user-facing strings", types=RAILS_KINDS | {"view"}, weight=6, + question=dict( + type="noul", + instructions=( + "Ignoring strings nobody sees (log messages, file paths, class " + "names, error classes, format strings), is every user-facing " + "string routed through I18n - `t(...)` in views and helpers, " + "locale keys or `I18n.t` in models, controllers, services and " + "serializers - with no hardcoded English the user could read?" + ), + criteria={ + "true": "All user-facing strings come from locale files", + "false": "Hardcoded user-facing strings", + }, + ), + threshold=0.5), + dict(id="skinny_controller", label="Thin controller", types={"controller"}, weight=5, + question=dict( + type="noul", + instructions=( + "Does this controller stay thin - find the record, set instance " + "variables, delegate to a model or service, use semantic params, " + "and choose the right HTTP method - rather than holding business " + "logic, queries or data munging of its own?" + ), + criteria={ + "true": "Thin: sets up and delegates, logic lives elsewhere", + "false": "Business logic or heavy data handling in the controller", + }, + ), + threshold=0.5), + dict(id="no_css_classes", label="No class or style attributes", types={"view"}, weight=4, + question=dict( + type="noul", + instructions=( + "Does this template contain any `class` or `style` attribute at " + "all? House rule: zero class attributes, zero inline styles - " + "all styling comes from the MVP.css framework's semantic " + "selectors, so a class here is always a breach." + ), + criteria={ + "true": "Every class and style attribute absent - markup carries no styling hooks", + "false": "One or more class or style attributes", + }, + ), + threshold=0.5), + dict(id="semantic_structure", label="Semantic HTML structure", types={"view"}, weight=3, + question=dict( + type="noul", + instructions=( + "Is the structure of this template semantic - `
`, " + "`
`, `