feat(gradebook): level contribution percent mode + parseable-formula guard

Render level contribution in percent mode, and enforce that an enabled level
contribution always has a parseable formula:

- Model: validate on :formula (not the hidden :formula_ast) that an enabled
  config with a formula carries a non-blank ast — the error surfaces on the
  field the user edits, and an enabled config can no longer silently evaluate
  to nothing.
- Dialog: parse the formula whenever level is enabled, so clearing it blocks
  Save and flags the field instead of round-tripping to a confusing 422.
- Controller: wrap the tab-weight and level-config writes in one transaction
  so a rejected formula rolls back the tab weights instead of a partial save.

Author: LWS49

app/controllers/course/gradebook_controller.rb

@@ -22,8 +22,13 @@ def index
   def update_weights
     authorize! :manage_gradebook_weights, current_course
     updates = (update_weights_params[:weights] || []).map { |entry| parse_weight_entry(entry) }
-    Course::Gradebook::TabContribution.bulk_update(course: current_course, updates: updates)
-    level_config = persist_level_contribution
+    level_config = nil
+    # One transaction so a rejected level formula rolls back the tab-weight writes too,
+    # rather than leaving a partial save.
+    ActiveRecord::Base.transaction do
+      Course::Gradebook::TabContribution.bulk_update(course: current_course, updates: updates)
+      level_config = persist_level_contribution
+    end
     response_body = { weights: serialize_weight_updates(updates) }
     response_body[:levelContribution] = serialize_level_contribution(level_config) if level_config
     render json: response_body

app/models/course/gradebook/level_config.rb

@@ -4,9 +4,9 @@ class Course::Gradebook::LevelConfig < ApplicationRecord # rubocop:disable Metri
 
   validates :weight, numericality: { greater_than_or_equal_to: 0, less_than_or_equal_to: 100 }
   validates :formula, presence: true, if: :enabled
-  validates :formula_ast, presence: true, if: :enabled
   validates :course_id, uniqueness: true
   validate :formula_ast_structure
+  validate :enabled_formula_must_parse
 
   # Upserts the singleton Level config for a course from a normalized attrs hash
   # (symbol keys, snake_case). Raises ActiveRecord::RecordInvalid on validation failure.
@@ -88,6 +88,18 @@ def formula_ast_structure
     errors.add(:formula_ast, 'has an invalid structure') unless self.class.valid_ast?(formula_ast)
   end
 
+  # An enabled contribution must carry a usable parse. formula_ast is derived client-side
+  # (there is no server-side parser), so a present formula with a blank ast means the formula
+  # did not parse — reject it on :formula (the field the user edits) rather than silently
+  # persisting an enabled config that evaluate_for returns nil for. The blank-formula case is
+  # already covered by the presence validation above, so guard on formula.present? to avoid a
+  # duplicate error.
+  def enabled_formula_must_parse
+    return unless enabled && formula.present?
+
+    errors.add(:formula, 'could not be parsed') if formula_ast.blank?
+  end
+
   def evaluate_node(node, level)
     case node['type']
     when 'num'   then node['value'].to_f

client/app/bundles/course/gradebook/__tests__/ConfigureWeightsPrompt.test.tsx

@@ -202,6 +202,19 @@ describe('<ConfigureWeightsPrompt />', () => {
     expect(screen.getByRole('button', { name: /save/i })).toBeDisabled();
   });
 
+  it('disables Save and flags the formula when Level contribution is enabled but the formula is cleared', () => {
+    setup({
+      gamificationEnabled: true,
+      levelContribution: enabledLevel(),
+      students,
+    });
+    fireEvent.change(screen.getByLabelText(/formula/i), {
+      target: { value: '' },
+    });
+    expect(screen.getByRole('button', { name: /save/i })).toBeDisabled();
+    expect(screen.getByText('Enter a formula.')).toBeInTheDocument();
+  });
+
   it('shows the running assessment-weight sum against the tab total', () => {
     setup();
     fireEvent.click(

client/app/bundles/course/gradebook/__tests__/WeightedGradebookTable.test.tsx

@@ -1780,6 +1780,71 @@ describe('level contribution columns', () => {
     ).not.toBeInTheDocument();
   });
 
+  // Percent lens: the Level Contribution column is stored in points but, like every
+  // other category column, renders as "% of this component earned" in percent mode —
+  // i.e. contribution / weight × 100, NOT the raw points with a stray "%" suffix.
+  it('renders the level contribution as a percentage of its weight in percent mode', async () => {
+    const user = userEvent.setup();
+    renderWeighted({
+      gamificationEnabled: true,
+      levelContribution: { ...levelOn, formula: 'level', weight: 30 },
+      tabs: [makeTab(10, 'Tab 1', 1, 100)],
+      assessments: [makeAssessment(100, 'Quiz 1', 10, 10)],
+      // BE-supplied contribution: 15 pts out of a 30-pt budget.
+      students: [
+        { ...makeStudent(1, 'Alice'), level: 8, levelContribution: 15 },
+      ],
+      submissions: [makeSub(1, 100, 8)],
+    });
+    await user.click(screen.getByRole('radio', { name: /percentage/i }));
+    const aliceRow = screen.getByText('Alice').closest('tr')!;
+    // 15 / 30 → 50%, mirroring how a tab cell shows the fraction of that tab earned.
+    expect(within(aliceRow).getByText('50%')).toBeInTheDocument();
+  });
+
+  it('renders the level breakdown row as a percentage of the weight in percent mode', async () => {
+    const user = userEvent.setup();
+    renderWeighted({
+      gamificationEnabled: true,
+      levelContribution: { ...levelOn, formula: 'level', weight: 30 },
+      tabs: [makeTab(10, 'Tab 1', 1, 100)],
+      assessments: [makeAssessment(100, 'Quiz 1', 10, 10)],
+      students: [
+        { ...makeStudent(1, 'Alice'), level: 8, levelContribution: 15 },
+      ],
+      submissions: [makeSub(1, 100, 8)],
+    });
+    await user.click(screen.getByRole('radio', { name: /percentage/i }));
+    await user.click(screen.getByRole('button', { name: /expand Alice/i }));
+    const levelRow = (await screen.findAllByTestId(/^breakdown-row-1--1-/))[0];
+    // The breakdown's level cell mirrors the summary cell: 15 / 30 → 50%.
+    expect(within(levelRow).getByText('50%')).toBeInTheDocument();
+  });
+
+  it('lets an over-budget level contribution read past 100% in percent mode', async () => {
+    const user = userEvent.setup();
+    renderWeighted({
+      gamificationEnabled: true,
+      // clamp off + a formula that overshoots: BE supplies the raw 45 pts.
+      levelContribution: {
+        ...levelOn,
+        formula: 'level',
+        weight: 30,
+        clamp: false,
+      },
+      tabs: [makeTab(10, 'Tab 1', 1, 100)],
+      assessments: [makeAssessment(100, 'Quiz 1', 10, 10)],
+      students: [
+        { ...makeStudent(1, 'Alice'), level: 45, levelContribution: 45 },
+      ],
+      submissions: [makeSub(1, 100, 8)],
+    });
+    await user.click(screen.getByRole('radio', { name: /percentage/i }));
+    const aliceRow = screen.getByText('Alice').closest('tr')!;
+    // 45 / 30 → 150%: the weight is a suggested max, never a cap.
+    expect(within(aliceRow).getByText('150%')).toBeInTheDocument();
+  });
+
   // Over-budget warning: the level weight is a suggested maximum, so a formula can
   // push a student's contribution past it or below 0. The Level Contribution subheader
   // shows a bound-aware message (above-only, below-only, or both).

client/app/bundles/course/gradebook/components/ConfigureWeightsPrompt.tsx

@@ -385,8 +385,11 @@ const ConfigureWeightsPrompt: FC<Props> = ({
   const effectiveWeight = (tabId: number): number =>
     isAllExcluded(tabId) ? 0 : weights[tabId] ?? 0;
 
-  const parsedLevel: ParsedFormula | null =
-    levelEnabled && levelFormula ? parseFormula(levelFormula) : null;
+  // Parse whenever enabled — including an empty string, which parseFormula reports as
+  // { ok: false, error: 'Enter a formula.' } so Save is blocked and the field flags it.
+  const parsedLevel: ParsedFormula | null = levelEnabled
+    ? parseFormula(levelFormula)
+    : null;
   const levelParseError =
     parsedLevel && !parsedLevel.ok ? parsedLevel.error : null;
   // Students whose contribution falls outside [0, levelWeight], split by bound —

client/app/bundles/course/gradebook/components/WeightedGradebookTable.tsx

@@ -309,6 +309,18 @@ const WeightedGradebookTable = ({
     return displayMode === 'percent' ? sub * 100 : sub * weight;
   };
 
+  // The Level Contribution is stored in points (already weight-scaled). Points mode
+  // shows it verbatim; percent mode shows the fraction of the level budget earned
+  // (points / weight × 100), mirroring how a tab cell reads in percent — a value can
+  // exceed 100% since the weight is a suggested max, never a cap. Weight 0 → the
+  // component carries no grade, so there's nothing meaningful to normalise against.
+  const levelDisplayValue = (points: number | null): number | null => {
+    if (points === null) return null;
+    if (displayMode !== 'percent') return points;
+    const weight = levelContribution.weight;
+    return weight > 0 ? (points / weight) * 100 : null;
+  };
+
   const allExcludedTabIds = useMemo(() => {
     const byTab = new Map<number, boolean>();
     assessments.forEach((a) => {
@@ -632,9 +644,11 @@ const WeightedGradebookTable = ({
     return {
       tabs: tabPrecs,
       total: columnPrecision(rows.map((r) => totalDisplayValue(r.total))),
-      level: columnPrecision(rows.map((r) => r.levelContribution ?? null)),
+      level: columnPrecision(
+        rows.map((r) => levelDisplayValue(r.levelContribution ?? null)),
+      ),
     };
-  }, [rows, resolvedTabs, displayMode, totalWeight]);
+  }, [rows, resolvedTabs, displayMode, totalWeight, levelContribution.weight]);
 
   const hasExternalIds = useMemo(
     () => students.some((s) => s.externalId != null && s.externalId !== ''),
@@ -689,9 +703,13 @@ const WeightedGradebookTable = ({
       cols.push({
         id: 'levelContribution',
         title: t(translations.levelContributionHeader),
-        accessorFn: (row) => fmtCsv(row.levelContribution ?? null),
+        accessorFn: (row) =>
+          fmtCsv(levelDisplayValue(row.levelContribution ?? null)),
         cell: (row) =>
-          fmtDisplay(row.levelContribution ?? null, columnPrecisions.level),
+          fmtDisplay(
+            levelDisplayValue(row.levelContribution ?? null),
+            columnPrecisions.level,
+          ),
         csvDownloadable: true,
       });
     }
@@ -1267,7 +1285,9 @@ const WeightedGradebookTable = ({
                         {showLevelContributionCol &&
                           ((): React.JSX.Element => {
                             const valueText = fmtDisplay(
-                              row.original.levelContribution ?? null,
+                              levelDisplayValue(
+                                row.original.levelContribution ?? null,
+                              ),
                               columnPrecisions.level,
                             );
                             const info = levelClampByStudent.get(
@@ -1479,7 +1499,7 @@ const WeightedGradebookTable = ({
                                   <TableCell align="right">
                                     {tb.tabId === LEVEL_TAB_ID
                                       ? fmtDisplay(
-                                          a.points,
+                                          levelDisplayValue(a.points),
                                           columnPrecisions.level,
                                         )
                                       : ''}

spec/controllers/course/gradebook_controller_spec.rb

@@ -378,6 +378,17 @@ def weight_for(tab)
           expect(weight_for(tab1)).to eq(10)
         end
 
+        it 'rolls back tab weights when the enabled level formula is invalid' do
+          create(:course_gradebook_tab_contribution, tab: tab1, course: course, weight: 10)
+          patch :update_weights, params: {
+            course_id: course.id,
+            weights: [tabId: tab1.id, weight: 50],
+            levelContribution: { enabled: true, formula: '', weight: 5 }
+          }, as: :json
+          expect(response).to have_http_status(:unprocessable_content)
+          expect(weight_for(tab1)).to eq(10)
+        end
+
         it 'rejects >100 with 422' do
           patch :update_weights,
                 params: { course_id: course.id, weights: [tabId: tab1.id, weight: 101] },

spec/models/course/gradebook/level_config_spec.rb

@@ -23,6 +23,13 @@
         expect(config).to be_valid
       end
 
+      it 'rejects an enabled config whose formula did not parse (blank ast), erroring on :formula' do
+        config = build(:course_gradebook_level_config, course: course,
+                                                       enabled: true, formula: 'level', formula_ast: nil)
+        expect(config).not_to be_valid
+        expect(config.errors[:formula]).to be_present
+      end
+
       it 'rejects a negative weight' do
         config = build(:course_gradebook_level_config, course: course, weight: -1)
         expect(config).not_to be_valid
@@ -43,7 +50,8 @@
       it 'creates a config from a camelCase-free attrs hash' do
         config = described_class.upsert_for(
           course: course,
-          attrs: { enabled: true, formula: 'min(level, 30) * 0.05', weight: 8, show: true }
+          attrs: { enabled: true, formula: 'min(level, 30) * 0.05',
+                   formula_ast: { 'type' => 'var', 'name' => 'level' }, weight: 8, show: true }
         )
         expect(config.enabled).to eq(true)
         expect(config.formula).to eq('min(level, 30) * 0.05')
@@ -52,7 +60,9 @@
       end
 
       it 'updates the existing singleton on a second call' do
-        described_class.upsert_for(course: course, attrs: { enabled: true, formula: 'level', weight: 2 })
+        described_class.upsert_for(course: course,
+                                   attrs: { enabled: true, formula: 'level',
+                                            formula_ast: { 'type' => 'var', 'name' => 'level' }, weight: 2 })
         described_class.upsert_for(course: course, attrs: { enabled: false, formula: '', weight: 0 })
         expect(described_class.where(course_id: course.id).count).to eq(1)
         expect(course.reload.gradebook_level_config.enabled).to eq(false)
@@ -253,14 +263,23 @@
         expect(config.formula_ast).to eq(valid_ast)
       end
 
-      it 'stores nil formula_ast when not provided' do
+      it 'stores nil formula_ast for a disabled config' do
         config = described_class.upsert_for(
           course: course,
-          attrs: { enabled: true, formula: 'level', weight: 5 }
+          attrs: { enabled: false, formula: '', weight: 0 }
         )
         expect(config.formula_ast).to be_nil
       end
 
+      it 'raises RecordInvalid when enabled with a formula but no parseable ast' do
+        expect do
+          described_class.upsert_for(
+            course: course,
+            attrs: { enabled: true, formula: 'level', weight: 5 }
+          )
+        end.to raise_error(ActiveRecord::RecordInvalid)
+      end
+
       it 'raises RecordInvalid for a malformed formula_ast' do
         expect do
           described_class.upsert_for(