fix(chessql): remove user-facing attack motif#1080
Conversation
…otif from SqlCompiler's VALID_MOTIFS and compileMotif\n- Strips 'attack' from the QueryView syntax help text\n- Removes 'hasAttack' badge configuration in GameTable\n- Drops 'hasAttack' from GameRow interface\n- Updates SqlCompilerTest to verify 'attack' compilation throws an error\n\nResolves #1076 Co-authored-by: google-labs-jules[bot] <161369871+google-labs-jules[bot]@users.noreply.github.com>
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
1d4-web | 5b4f27e | Commit Preview URL Branch Preview URL |
Feb 28 2026, 10:49 PM |
…oves `assertMotifDetected(url, "attack")` from `MotifE2ETest` to comply with the removal of `attack` from `VALID_MOTIFS` in `SqlCompiler.java`.\n- Ensures `attack` motif presence is still verified in the database via `getOccurrences()`, without expecting it to compile as a valid user-facing query motif. Co-authored-by: google-labs-jules[bot] <161369871+google-labs-jules[bot]@users.noreply.github.com>
|
rebase please |
Rebased. |
…otif from SqlCompiler's VALID_MOTIFS and compileMotif\n- Strips 'attack' from the QueryView syntax help text\n- Removes 'hasAttack' badge configuration in GameTable\n- Drops 'hasAttack' from GameRow interface\n- Updates SqlCompilerTest to verify 'attack' compilation throws an error\n- Removes test assertion for attack motif compilation from MotifE2ETest\n\nResolves #1076 Co-authored-by: google-labs-jules[bot] <161369871+google-labs-jules[bot]@users.noreply.github.com>
|
merge main in |
Merged |
…otif from SqlCompiler's VALID_MOTIFS and compileMotif\n- Strips 'attack' from the QueryView syntax help text\n- Removes 'hasAttack' badge configuration in GameTable\n- Drops 'hasAttack' from GameRow interface\n- Updates SqlCompilerTest to verify 'attack' compilation throws an error\n- Removes test assertion for attack motif compilation from MotifE2ETest\n\nResolves #1076 Co-authored-by: google-labs-jules[bot] <161369871+google-labs-jules[bot]@users.noreply.github.com>
|
merge main again -- there have been substantial changes there |
Merged |
…otif from SqlCompiler's VALID_MOTIFS and compileMotif\n- Strips 'attack' from the QueryView syntax help text\n- Removes 'hasAttack' badge configuration in GameTable\n- Drops 'hasAttack' from GameRow interface\n- Updates SqlCompilerTest to verify 'attack' compilation throws an error\n- Removes test assertion for attack motif compilation from MotifE2ETest\n\nResolves #1076 Co-authored-by: google-labs-jules[bot] <161369871+google-labs-jules[bot]@users.noreply.github.com>
…r approach Revise DATAFUSION_PARQUET.md based on architectural review findings: - Replace Substrait IR approach with a dual-compiler model: SqlCompiler (unchanged, PostgreSQL) + new DataFusionSqlCompiler (DataFusion SQL). datafusion-substrait 52.x still documents that correlated EXISTS + GROUP BY/HAVING is unsupported, which is required for all 5 ATTACK-derived motifs and sequence() queries. - Fix motif_occurrences Parquet schema: add attacker, target, is_discovered, is_mate columns (load-bearing for fork, checkmate, discovered_attack, discovered_check, double_check at query time). - Fix "boolean flags" narrative: all motif queries now use EXISTS subqueries on motif_occurrences, not boolean scans on game_features. - Add game_pgns Parquet table (Lichess only): written during initial ingest to enable re-analysis without re-downloading source dumps. Avoids 700+ GB re-download for detector fixes across historical months. - Fix export job: full partition overwrite (no per-game tracking needed); staleness detection via indexed_at > parquet_exported_at. - Add contract test suite design: same data, both backends, all 16 motifs + sequence + ORDER BY, result equivalence asserted in CI. - Reorder implementation phases: DataFusionSqlCompiler + motif_query scaffold first, then export + time-based routing, then shadow mode, then Lichess ingest. - Update DataFusion/arrow/parquet crate versions: 46/55 -> 52/57. - Add Future Enhancement: Substrait section explaining what would trigger a revisit once the toolchain matures. Fix CHESSQL.md: remove motif(attack) from directly-stored motifs table (removed in #1080); update count to 11 stored + 5 ATTACK-derived = 16.
…r approach (#1092) Revise DATAFUSION_PARQUET.md based on architectural review findings: - Replace Substrait IR approach with a dual-compiler model: SqlCompiler (unchanged, PostgreSQL) + new DataFusionSqlCompiler (DataFusion SQL). datafusion-substrait 52.x still documents that correlated EXISTS + GROUP BY/HAVING is unsupported, which is required for all 5 ATTACK-derived motifs and sequence() queries. - Fix motif_occurrences Parquet schema: add attacker, target, is_discovered, is_mate columns (load-bearing for fork, checkmate, discovered_attack, discovered_check, double_check at query time). - Fix "boolean flags" narrative: all motif queries now use EXISTS subqueries on motif_occurrences, not boolean scans on game_features. - Add game_pgns Parquet table (Lichess only): written during initial ingest to enable re-analysis without re-downloading source dumps. Avoids 700+ GB re-download for detector fixes across historical months. - Fix export job: full partition overwrite (no per-game tracking needed); staleness detection via indexed_at > parquet_exported_at. - Add contract test suite design: same data, both backends, all 16 motifs + sequence + ORDER BY, result equivalence asserted in CI. - Reorder implementation phases: DataFusionSqlCompiler + motif_query scaffold first, then export + time-based routing, then shadow mode, then Lichess ingest. - Update DataFusion/arrow/parquet crate versions: 46/55 -> 52/57. - Add Future Enhancement: Substrait section explaining what would trigger a revisit once the toolchain matures. Fix CHESSQL.md: remove motif(attack) from directly-stored motifs table (removed in #1080); update count to 11 stored + 5 ATTACK-derived = 16.
…1093) * docs(one_d4): replace Substrait plan with DataFusion SQL dual-compiler approach Revise DATAFUSION_PARQUET.md based on architectural review findings: - Replace Substrait IR approach with a dual-compiler model: SqlCompiler (unchanged, PostgreSQL) + new DataFusionSqlCompiler (DataFusion SQL). datafusion-substrait 52.x still documents that correlated EXISTS + GROUP BY/HAVING is unsupported, which is required for all 5 ATTACK-derived motifs and sequence() queries. - Fix motif_occurrences Parquet schema: add attacker, target, is_discovered, is_mate columns (load-bearing for fork, checkmate, discovered_attack, discovered_check, double_check at query time). - Fix "boolean flags" narrative: all motif queries now use EXISTS subqueries on motif_occurrences, not boolean scans on game_features. - Add game_pgns Parquet table (Lichess only): written during initial ingest to enable re-analysis without re-downloading source dumps. Avoids 700+ GB re-download for detector fixes across historical months. - Fix export job: full partition overwrite (no per-game tracking needed); staleness detection via indexed_at > parquet_exported_at. - Add contract test suite design: same data, both backends, all 16 motifs + sequence + ORDER BY, result equivalence asserted in CI. - Reorder implementation phases: DataFusionSqlCompiler + motif_query scaffold first, then export + time-based routing, then shadow mode, then Lichess ingest. - Update DataFusion/arrow/parquet crate versions: 46/55 -> 52/57. - Add Future Enhancement: Substrait section explaining what would trigger a revisit once the toolchain matures. Fix CHESSQL.md: remove motif(attack) from directly-stored motifs table (removed in #1080); update count to 11 stored + 5 ATTACK-derived = 16. * feat(chessql): implement DataFusionSqlCompiler with inline literals Adds DataFusionSqlCompiler, which compiles the same ChessQL AST as SqlCompiler but emits DataFusion SQL with all values inlined as SQL literals instead of JDBC bind parameters. parameters() is always empty. Covers comparison, IN, motif, sequence, and ORDER BY expressions. Mirrors the full SqlCompilerTest suite (39 tests) against the new compiler. This is Phase 1 of the DataFusion/Parquet migration and a prerequisite for the motif_query Rust service integration.
This PR resolves issue #1076 by hiding the
attackmotif from the user-facing UI and ChessQL query compiler, effectively treating it as a backend-only internal primitive.Changes:
"attack"from theVALID_MOTIFSset inSqlCompiler.javaand dropped the special-caseifblock for compiling it. Added a test ensuring it's rejected.motif(attack)from the syntax help text inQueryView.tsx.hasAttackfrom theMOTIF_KEYSarray inGameTable.tsxto prevent the badge from rendering.hasAttackproperty from theGameRowinterface intypes.tsas it's no longer necessary for the UI.The underlying backend mechanics (database, engine primitives, detectors) have not been altered, strictly adhering to the requirements.
All tests (backend and frontend) are passing locally. Screenshots have been taken locally via Playwright to ensure the
attackmotif has been successfully purged from the query helper text.PR created automatically by Jules for task 10998642611428147024 started by @aaylward