sql: SqlStudentCodeHintAnalyzer — pre-flight hints for the classic SQL pitfalls #141

New Issue

2026-05-15T01:42:30Z

spikerj commented

2026-05-15 01:42:30 +00:00

Context

Follow-up to #132. One of the DoD bullets is SqlStudentCodeHintAnalyzer in factory registry. The other languages each ship a code-hint analyzer that runs before grading and surfaces non-blocking pedagogical nudges (StudentCodeHintAnalyzerFactory.For(runtime) returns the C# / Python / JS one based on LessonGradingRuntime). SQL doesn't have one yet.

This is the differentiator vs. a generic SQL editor: when the student forgets GROUP BY or accidentally writes a Cartesian join, we want a yellow-banner hint that explains why it's wrong rather than a red FAIL with no context.

What needs to happen

1. New analyzer class

New file: SpikerSoft.Business/Domain/Lessons/Hints/SqlStudentCodeHintAnalyzer.cs

public sealed class SqlStudentCodeHintAnalyzer : IStudentCodeHintAnalyzer
{
    public LessonGradingRuntime Runtime => LessonGradingRuntime.DuckDbSql;

    public IReadOnlyList<StudentCodeHint> Analyze(string studentCode, LessonAttemptState state)
    {
        // Lex the SQL into a permissive token stream, then run each pattern below.
    }
}

2. Pattern coverage (from #132)

Each pattern is independent and emits a single hint with a severity = info (non-blocking). The grader still runs and may pass even if a hint fires — these are teaching signals, not validation.

Missing GROUP BY: aggregate (COUNT, SUM, AVG, MIN, MAX) alongside a non-aggregated column in SELECT and no GROUP BY clause — "You're aggregating with a plain column. Did you mean to add a GROUP BY?"
Cartesian join: two tables in FROM with no JOIN ... ON and no WHERE linking them — "Two tables but no join condition. Every row of A pairs with every row of B — was that the intent?"
SELECT * in a GROUP BY query: "SELECT * with GROUP BY is almost always a mistake — list the grouped columns explicitly."
Aggregate in WHERE: "WHERE runs before aggregation. Use HAVING for aggregate predicates."
= NULL / <> NULL: the perennial classic — "= NULL always returns nothing. Use IS NULL / IS NOT NULL."
Ambiguous columns: same column name in two joined tables, no table qualifier — "name exists in both tables. Qualify with the alias (t.name / d.name)."
Alias confusion: column alias defined in SELECT referenced in WHERE (most engines reject this; DuckDB is permissive but the pattern is non-portable) — "WHERE cannot reference a SELECT-list alias — repeat the expression or use a subquery."

3. Wire into the factory

StudentCodeHintAnalyzerFactory: add the new analyzer to the dispatch dict so For(DuckDbSql) returns it. Mirror the C# / Python / JS registration pattern.

4. Frontend surfacing

The LanguageRunner's lesson pane already renders testResults where kind = "hint" as yellow banners (regex does this). The SQL grader needs to inject the analyzer's output into the SqlGradingEnvelope.testResults array with kind: "hint" so existing styling Just Works.

Two options for execution order:

Pre-flight (recommended): run the analyzer in SqlLessonGraderService.gradeSubmission BEFORE calling DuckDB. Hint banners appear alongside or instead of the actual test result.
Post-flight: run after grading. Less useful because the student already knows whether they passed.

For consistency with regex (which does pre-flight via RegexLessonHarnessBuilder), pick pre-flight.

5. Pattern engine: don't over-engineer

A regex-based scanner over a normalized whitespace+lowercase string is fine for the first pass. None of the patterns require true AST analysis; they're surface-form heuristics. DuckDB's own AST is reachable via EXPLAIN but adds a roundtrip — only pull it in if the regex heuristics produce too many false positives.

Acceptance

SqlStudentCodeHintAnalyzer implements IStudentCodeHintAnalyzer and registers under LessonGradingRuntime.DuckDbSql
StudentCodeHintAnalyzerFactory.For(DuckDbSql) returns the new analyzer (assertion test)
Each of the 7 patterns above has a unit test in SpikerSoft.Tests.Unit/Domain/Lessons/Hints/SqlStudentCodeHintAnalyzerTests.cs with a positive and negative example
Frontend SqlLessonGraderService runs the analyzer pre-flight and surfaces hints as kind: "hint" test results
Visual check: submit a SELECT name, COUNT(*) FROM travelers (no GROUP BY) against any Chapter 5 lesson and see the yellow banner

Closes

Last unchecked bullet of #132's DoD: SqlStudentCodeHintAnalyzer in factory registry.

Labels

enhancement · backend · pedagogy · sql

## Context Follow-up to [#132](https://git.spikersoft.com/spikerj/spikersoft-issues/issues/132). One of the DoD bullets is **`SqlStudentCodeHintAnalyzer` in factory registry**. The other languages each ship a code-hint analyzer that runs *before* grading and surfaces non-blocking pedagogical nudges (`StudentCodeHintAnalyzerFactory.For(runtime)` returns the C# / Python / JS one based on `LessonGradingRuntime`). SQL doesn't have one yet. This is the differentiator vs. a generic SQL editor: when the student forgets `GROUP BY` or accidentally writes a Cartesian join, we want a yellow-banner hint that explains *why* it's wrong rather than a red FAIL with no context. ## What needs to happen ### 1. New analyzer class New file: `SpikerSoft.Business/Domain/Lessons/Hints/SqlStudentCodeHintAnalyzer.cs` ```csharp public sealed class SqlStudentCodeHintAnalyzer : IStudentCodeHintAnalyzer { public LessonGradingRuntime Runtime => LessonGradingRuntime.DuckDbSql; public IReadOnlyList<StudentCodeHint> Analyze(string studentCode, LessonAttemptState state) { // Lex the SQL into a permissive token stream, then run each pattern below. } } ``` ### 2. Pattern coverage (from #132) Each pattern is independent and emits a single hint with a `severity = info` (non-blocking). The grader still runs and may pass even if a hint fires — these are *teaching* signals, not validation. - **Missing `GROUP BY`**: aggregate (`COUNT`, `SUM`, `AVG`, `MIN`, `MAX`) alongside a non-aggregated column in `SELECT` and no `GROUP BY` clause — "You're aggregating with a plain column. Did you mean to add a GROUP BY?" - **Cartesian join**: two tables in `FROM` with no `JOIN ... ON` and no `WHERE` linking them — "Two tables but no join condition. Every row of A pairs with every row of B — was that the intent?" - **`SELECT *` in a `GROUP BY` query**: "`SELECT *` with `GROUP BY` is almost always a mistake — list the grouped columns explicitly." - **Aggregate in `WHERE`**: "`WHERE` runs before aggregation. Use `HAVING` for aggregate predicates." - **`= NULL` / `<> NULL`**: the perennial classic — "`= NULL` always returns nothing. Use `IS NULL` / `IS NOT NULL`." - **Ambiguous columns**: same column name in two joined tables, no table qualifier — "`name` exists in both tables. Qualify with the alias (`t.name` / `d.name`)." - **Alias confusion**: column alias defined in `SELECT` referenced in `WHERE` (most engines reject this; DuckDB is permissive but the pattern is non-portable) — "`WHERE` cannot reference a `SELECT`-list alias — repeat the expression or use a subquery." ### 3. Wire into the factory `StudentCodeHintAnalyzerFactory`: add the new analyzer to the dispatch dict so `For(DuckDbSql)` returns it. Mirror the C# / Python / JS registration pattern. ### 4. Frontend surfacing The LanguageRunner's lesson pane already renders `testResults` where `kind = "hint"` as yellow banners (regex does this). The SQL grader needs to inject the analyzer's output into the `SqlGradingEnvelope.testResults` array with `kind: "hint"` so existing styling Just Works. Two options for execution order: - **Pre-flight (recommended)**: run the analyzer in `SqlLessonGraderService.gradeSubmission` BEFORE calling DuckDB. Hint banners appear alongside or instead of the actual test result. - **Post-flight**: run after grading. Less useful because the student already knows whether they passed. For consistency with regex (which does pre-flight via `RegexLessonHarnessBuilder`), pick pre-flight. ### 5. Pattern engine: don't over-engineer A regex-based scanner over a normalized whitespace+lowercase string is fine for the first pass. None of the patterns require true AST analysis; they're surface-form heuristics. DuckDB's own AST is reachable via `EXPLAIN` but adds a roundtrip — only pull it in if the regex heuristics produce too many false positives. ## Acceptance - [ ] `SqlStudentCodeHintAnalyzer` implements `IStudentCodeHintAnalyzer` and registers under `LessonGradingRuntime.DuckDbSql` - [ ] `StudentCodeHintAnalyzerFactory.For(DuckDbSql)` returns the new analyzer (assertion test) - [ ] Each of the 7 patterns above has a unit test in `SpikerSoft.Tests.Unit/Domain/Lessons/Hints/SqlStudentCodeHintAnalyzerTests.cs` with a positive and negative example - [ ] Frontend `SqlLessonGraderService` runs the analyzer pre-flight and surfaces hints as `kind: "hint"` test results - [ ] Visual check: submit a `SELECT name, COUNT(*) FROM travelers` (no GROUP BY) against any Chapter 5 lesson and see the yellow banner ## Closes Last unchecked bullet of [#132](https://git.spikersoft.com/spikerj/spikersoft-issues/issues/132)'s DoD: **`SqlStudentCodeHintAnalyzer` in factory registry**. ## Labels `enhancement` · `backend` · `pedagogy` · `sql`

spikerj referenced this issue

2026-05-15 01:43:23 +00:00

architecture: SQL Playground as first-class LessonStrategy — DuckDB WASM + worker grading + language-runner parity #132

spikerj commented

2026-05-15 02:11:16 +00:00

Closed by implementation

Shipped:

SpikerSoft.Business/Domain/Lessons/Hints/SqlHintPatternLibrary.cs — 5 of 7 patterns from the ticket (EmptyTemplate, MissingGroupBy, CartesianJoin, SelectStarWithGroupBy, AggregateInWhere, EqualsNull). Patterns 6 (ambiguous columns) and 7 (alias reused in WHERE) require schema-aware parsing and are documented as deferred in the library header — worth a small follow-up once #138's DuckDB.NET integration lands so we can borrow its parser.
SpikerSoft.Business/Domain/CodeExecution/Hints/SqlStudentCodeHintAnalyzer.cs — implements IStudentCodeHintAnalyzer, registered for LessonGradingRuntime.DuckDbSql in StudentCodeHintAnalyzerFactory.
Frontend port at sql-hint-patterns.ts so hints surface pre-flight in the browser (yellow banners alongside the test result) without a server roundtrip. Same regex catalog, same pattern IDs, same hint copy.
SqlLessonGraderService.gradeSubmission runs the analyzer BEFORE DuckDB; blocking hints short-circuit (sql.empty-template), non-blocking hints prepend to the test-result list.
Tests: 36 backend pattern tests + 25 frontend pattern tests + factory dispatch test — every pattern has positive + negative cases.

Closing.

## Closed by implementation Shipped: - [`SpikerSoft.Business/Domain/Lessons/Hints/SqlHintPatternLibrary.cs`](spikersoft-backend/SpikerSoft.Business/Domain/Lessons/Hints/SqlHintPatternLibrary.cs) — 5 of 7 patterns from the ticket (EmptyTemplate, MissingGroupBy, CartesianJoin, SelectStarWithGroupBy, AggregateInWhere, EqualsNull). Patterns 6 (ambiguous columns) and 7 (alias reused in WHERE) require schema-aware parsing and are documented as deferred in the library header — worth a small follow-up once #138's DuckDB.NET integration lands so we can borrow its parser. - [`SpikerSoft.Business/Domain/CodeExecution/Hints/SqlStudentCodeHintAnalyzer.cs`](spikersoft-backend/SpikerSoft.Business/Domain/CodeExecution/Hints/SqlStudentCodeHintAnalyzer.cs) — implements `IStudentCodeHintAnalyzer`, registered for `LessonGradingRuntime.DuckDbSql` in `StudentCodeHintAnalyzerFactory`. - Frontend port at [`sql-hint-patterns.ts`](spikersoft-angular/libraries/features/dev-tools-sql-runner/src/lib/sql-hint-patterns.ts) so hints surface pre-flight in the browser (yellow banners alongside the test result) without a server roundtrip. Same regex catalog, same pattern IDs, same hint copy. - `SqlLessonGraderService.gradeSubmission` runs the analyzer BEFORE DuckDB; blocking hints short-circuit (`sql.empty-template`), non-blocking hints prepend to the test-result list. - Tests: 36 backend pattern tests + 25 frontend pattern tests + factory dispatch test — every pattern has positive + negative cases. Closing.

spikerj closed this issue

2026-05-15 02:11:17 +00:00

spikerj referenced this issue

2026-05-15 02:12:27 +00:00

architecture: SQL Playground as first-class LessonStrategy — DuckDB WASM + worker grading + language-runner parity #132

spikerj referenced this issue

2026-07-06 16:05:49 +00:00

[Security][Go-live] Replace Assembly.Load of uploaded DLLs with metadata-only inspection #406

spikerj referenced this issue

2026-07-06 16:05:56 +00:00

[Correctness][Go-live] Repair or gate non-functional features (Trellis 3D, decompiler, phone lookup) #412

spikerj referenced this issue