feat: add grading_style and sub_group columns, fix GradingHelper bug, decouple grading logic - #369
Open
Mucunguzi256 wants to merge 15 commits into
Open
feat: add grading_style and sub_group columns, fix GradingHelper bug, decouple grading logic#369Mucunguzi256 wants to merge 15 commits into
Mucunguzi256 wants to merge 15 commits into
Conversation
… principle Add standing rule #17 to AGENTS.md: when a step's data model is genuinely configurable, the UI should still prefill sensible real-world defaults rather than presenting an empty form. Example: saveTerms accepts any number of terms, but the UI should prefill 3 standard UNEB terms. Add matching §7 (UI/UX design principles) to docs/onboarding-engine-plan.md with a cross-link to AGENTS.md rule #17. Flagged as 'documented, not yet implemented' in both places.
…nciple Promotes AGENTS.md rule #17 ('Configurable, but never blank') into a reusable skill document with concrete defaults per onboarding step, implementation checklist, and cross-references. - New: docs/onboarding-defaults-skill.md — the canonical skill doc, tracked in the repo so it's version-controlled and discoverable. Covers: core principle, why, known defaults per step (terms, classes, subjects, fees, academic year), implementation checklist, and status. - Updated: AGENTS.md rule #17 — added pointer to the new skill doc. - Also created local-only SKILL.md files in .agents/skills/, .claude/skills/, .cursor/skills/, .junie/skills/ for agent auto-discovery (these directories are gitignored and won't appear in the PR diff). Approach chosen: dedicated docs/onboarding-defaults-skill.md rather than extending DESIGN_SYSTEM.md, because DESIGN_SYSTEM.md is a component API reference (props, CSS classes, migration patterns) and a behavioral defaults principle is a different concern. The local SKILL.md convention is established but gitignored — it supplements agent auto-discovery without replacing the version-controlled doc.
Replace inline Standard::create, Section::firstOrCreate, StandardLink::firstOrCreate, Subject::firstOrCreate, AcademicTerm::create/firstOrCreate, and FeesCategories::create/firstOrCreate in both create-mode and complete-mode paths of AgentToshi::commitAll() with delegation calls to OnboardingEngine::saveStandards(), saveSubjects(), saveTerms(), and saveFees(). This fixes two real, previously-live bugs: 1. Single-Standard mapping for mixed-level schools: the old code created ONE $phase Standard (named after $this->schoolType) and mapped ALL classes to it. For a mixed-level school (e.g. nursery + primary + o-level), subjects and fees were invisible to tiers that didn't match the one Standard. OnboardingEngine creates per-class tier Standards correctly. 2. SchoolCategorySeeder never ran for Toshi: canonical defaults (core subjects, Standard rows for each tier) were missing because the old code never called SchoolCategorySeeder. OnboardingEngine runs it when school_category is set. Also adds feesForEngine() helper that transforms Toshi's string[] fee names into the structured array format expected by OnboardingEngine::saveFees(). Test changes: - ToshiCommitAllCompleteModeTest: updated to assert CORRECT per-class tier mapping (primary Standard for P1-P3) instead of old buggy single-Standard assertion - ToshiContentStepDelegationTest: 5 new tests verifying delegation and bug fixes - toshi_creates_per_class_tier_standards_not_single_phase (Bug 1 regression) - toshi_delegates_subjects_to_engine (uses DB::table() to avoid Subject name accessor) - toshi_delegates_terms_to_engine - toshi_delegates_fees_to_engine_whole_school_spread (Bug 1 + pattern #7 regression) - toshi_complete_mode_also_delegates_to_engine
Reference research for future grading-system redesign, not yet implemented. Documents three levels at different reform stages: - O-Level (UCE): REFORMED, CA-weighted 20/80, letter grades A-E - A-Level (UACE): NOT yet reformed, old points system still live - Primary (PLE): traditional Division 1-4, purely exam-based Also updates Current Status header with sub-grouping/grading_style next step.
Two follow-up items from the 2026-08-26 sub_group investigation:
F1. Security: all 3 teacher-creation paths hardcode literal "password"
with no forced reset — real security gap, not fixed yet.
F2. Cosmetic: 2 blade templates hardcode 3-term name map instead of
building from school's real AcademicTerm records — shows "-"
for schools with different term names.
… decouple grading logic
PR 1: Schema + GradingHelper fix + report-card decoupling + OnboardingEngine extension
- Add migration: grading_style enum('aggregate','total_marks') nullable on
standards, sub_group varchar nullable on standards_link
- Fix GradingHelper::levelTypeForStandard() bug: str_starts_with now
also matches underscore-prefixed variants (primary_lower → 'primary')
- Decouple ReportCardsController showAgg logic: use grading_style when
set, fall back to isNursery + in_array name check when NULL
- Decouple ReportCardCommentService: new resolveGroup() method uses
grading_style when set (total_marks→'lower', aggregate→'upper'),
falls back to in_array name check when NULL. Optional ?Standard
parameter added to commentFor/headTeacherCommentFor for backward
compatibility
- Extend OnboardingEngine::saveStandards(): optional 'sub_group' key in
classes array, passed to StandardLink::firstOrCreate via array_filter
to exclude NULL values (avoids SQL NULL!=NULL issues)
- All new columns are nullable, all new params are optional — existing
callers unaffected
Tests:
- GradingHelperLevelTypeTest: 13 tests (underscore-prefix regression)
- GradingStyleResolutionTest: 10 tests (grading_style precedence, fallback)
- ContentStepsTest: 4 new sub_group tests (stored, null default, streams,
mixed)
- Existing tests: ReportCardHeadTeacherCommentTest 8 PASS,
ReportTotalExcludesNonContributingExamsTest 7 PASS,
DashboardStudentCountExcludesInactiveTest 2 PASS
PR 2: KabaleSetGradingStyle Artisan command + tests
- New command: kabale:backfill-grading-style (--school=, --dry-run)
- Backfills grading_style on standards: nursery/primary_lower →
total_marks, primary/primary_upper/o-level/a-level → aggregate
- Backfills sub_group on standards_link: primary_lower → 'lower',
primary_upper → 'upper', others → NULL (no sub-group)
- Idempotent: skips already-correct values
- Fails gracefully for non-existent school IDs
- Step 6 (remove hardcoded in_array checks) is already done:
ReportCardsController and ReportCardCommentService already use
grading_style as primary source with in_array only as NULL fallback
Tests: BackfillKabaleGradingStyleTest 10 tests, 31 assertions
…d expanded follow-up items - New session log: 2026-08-26 sub-group/grading_style implementation (two PRs) - Expanded F1 (hardcoded teacher password) and F2 (hardcoded 3 terms) from truncated one-liners to full descriptions
…rgence report to knowledge.md
…boundaries in knowledge.md
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds
grading_styleenum onstandardsandsub_groupvarchar onstandards_link, fixes a GradingHelper underscore-prefix bug, and decouples report-card grading logic from hardcoded standard-name checks.Depends on: #367 (prefill defaults principle), #368 (Toshi content-step delegation) — merge those first.
Changes
Schema (PR 1)
grading_styleenum('aggregate','total_marks') nullable onstandards,sub_groupstring nullable onstandards_linkgrading_styleto$fillablesub_groupto$fillableGradingHelper bug fix
levelTypeForStandard(): addedstr_starts_with($name, $keyword.'_')alongside existing space-prefix match. Bug:primary_lower→nullbecausestr_starts_with('primary_lower', 'primary ')is false. Now correctly matches underscore-prefixed variants.Report-card grading decoupling
showAggreads$standard->grading_stylefirst; falls back to!$isNursery && !in_array($standardName, ['primary_lower'])when NULL — zero behavior change for rows wheregrading_styleis still NULL.resolveGroup()method usesgrading_stylewhen set (total_marks→'lower',aggregate→'upper'); falls back toin_arrayname check when NULL. Optional?Standard $standardparameter added tocommentFor()andheadTeacherCommentFor()— fully backward-compatible.OnboardingEngine extension
saveStandards(): accepts optionalsub_groupkey in$classesarray, passed toStandardLink::firstOrCreateviaarray_filter(excludes NULL values from search/create array — SQL NULL ≠ NULL comparison).Backfill command (PR 2)
kabale:backfill-grading-styleArtisan command with--dry-runand--school=optionstotal_marks, primary/primary_upper/o-level/a-level →aggregatesub_group='lower', primary_upper →sub_group='upper', others → NULLTest results
Backward compatibility
grading_styleis NULL) produces identical behavior to the current hardcoded checksKnown follow-ups (logged, not in this PR)
"password") — needs its own fix