Hebbian-Robotics / Hebbian-Robotics/hflow
The curation gate parses every query twice, and the second parse refuses nothing
- Dominant language
- Python
- Stars
- 269
- Forks
- 150
- Avg merge
- 8h 18m
- Merged PRs (30d)
- 246
Description
`reject_non_single_select` runs three checks in order (`src/hflow/curation.py:604`):
1. exactly one statement, of type `SELECT` (`:637`)
2. the leading keyword is not `pragma` / `describe` / `show` / `summarize` (`:643`)
3. the SQL parses inside `SELECT * FROM ()`, the shape preview interpolates (`:651`)
The third one refuses nothing that the first two have not already refused.
## Measured
Deleting the whole third block leaves every curation test green:
```
### wrapper parse removed
100 passed in 6.24s
(nothing noticed)
```
So I went looking for an input it catches on its own: one that parses standalone as a single `StatementType.SELECT`, is not headed by one of the four keywords, and still fails to parse inside the wrapper. Across 27 shapes there is none. Every case where the wrapped parse fails, something earlier has already refused it:
| sql | standalone | wrapped |
| --- | --- | --- |
| `PRAGMA database_list` | `1xSELECT` | `ERR` (already refused at step 2) |
| `(PRAGMA database_list)` | `ERR ParserException` | `ERR` (already refused at step 1) |
| `COPY (SELECT 1) TO 'x.csv'` | `1xCOPY` | `ERR` (already refused at step 1) |
| `SELECT 1 /* unterminated` | `ERR ParserException` | `ERR` (already refused at step 1) |
| everything else tried | `1xSELECT` | parses |
Shapes tried that all parse both ways: `SELECT 1;`, `SELECT 1;;`, trailing line comment, `ORDER BY`, `LIMIT`, `UNION`, `UNION ALL ... ORDER BY ... LIMIT`, CTE, `FROM episodes`, `VALUES`, `(SELECT 1)`, `(DESCRIBE SELECT 1)`, `SELECT * FROM (SHOW TABLES)`, `SELECT * FROM pragma_version()`, `TABLE episodes`, `* EXCLUDE`, `QUALIFY`, `unnest`, `UNPIVOT`, `GROUP BY ALL`, `SELECT 1 UNION (SELECT 2)`.
## What it costs
It parses every query a second time, and its only demonstrated behaviour is almost refusing valid SQL. Two workarounds exist purely to stop that: the appended `\n` at `:651`, so a trailing `--` comment cannot swallow the wrapper's `)`, and the `.rstrip(';')`, so `SELECT 1;` survives. Both are load-bearing only for the block itself. Neither is needed once it goes.
That is the same failure direction as the regression this gate already had once: refusing legal read-only SELECTs.
## What to do
Delete the block at `:647-659` and the `wrapped_input` line, keeping steps 1 and 2. Then delete whichever of the two supporting tests no longer describe anything real, and keep the ones that still pin behaviour:
- `test_reject_non_single_select_accepts_trailing_line_comment_without_newline` keeps its assertion but its comment stops being true, so rewrite the comment.
- `test_reject_non_single_select_accepts_a_trailing_semicolon` stays as-is; the behaviour it pins must survive.
If you find an input the wrapper parse refuses correctly, say what it is and close this instead. That is the more useful outcome and I would rather be wrong here.
## Definition of done
1. The wrapper parse is gone, and `PRAGMA`/`DESCRIBE`/`SHOW`/`SUMMARIZE` are still refused by both `/curation/preview` and `/curation/pin` with the same message.
2. FROM-first, parenthesized, `VALUES`, trailing-semicolon and trailing-comment queries are all still accepted.
3. Mutation: delete the leading-keyword refusal, confirm a test goes red. It is now the only thing standing between the endpoints and what they advertise.
4. Both suites green, including `packages/hflow-server`.
## Validation
```bash
uv sync --locked --all-extras
uv run ruff check
uv run ruff format --check
uv run ty check
uv run pytest -q
cd packages/hflow-server && uv run --project . pytest -q
```
Contributor guide
Research direction
Start in src/hflow/curation.py at reject_non_single_select around lines 604-659, then locate the two supporting tests named in the issue. Run the curation tests before and after removing the wrapper parse, and run both the repository and packages/hflow-server validation suites. Done means the redundant parse is gone, required keyword refusals remain, listed valid query forms still pass, and mutation confirms those refusals are covered.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- python, sql
- Domain
- backend-api-design, data-engineering, testing
- Issue type
- Refactor
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Active
- Clarity
- Clearly specified
- Newbie friendliness
- 76/100