Skip to content

feat: database bindings (SQLite/PostgreSQL/MariaDB), web route parameters, and web server fixes - #540

Merged
logbie merged 6 commits into
mainfrom
claude/vibrant-fermi-srm8k1
Jun 12, 2026
Merged

feat: database bindings (SQLite/PostgreSQL/MariaDB), web route parameters, and web server fixes#540
logbie merged 6 commits into
mainfrom
claude/vibrant-fermi-srm8k1

Conversation

@logbie

@logbie logbie commented Jun 12, 2026

Copy link
Copy Markdown
Collaborator

Closes the primitive-level runtime gaps identified in the framework-vs-language discussion: database access first, then route parameters, plus web server fixes from the archived FRAMEWORK_FINAL_REPORT.md. Built TDD throughout — every feature and fix started from a failing test.

Database bindings

Exposes the long-dormant sqlx dependency to WFL programs:

open database at "postgres://localhost/mydb" as db        // alias: connect to database at ... as ...
store users as query db with "SELECT * FROM users WHERE age > ?" and parameters [21]
store result as execute db with "INSERT INTO users (name) VALUES (?)" and parameters ["Alice"]
close database db
  • Backends: SQLite (sqlite://, sqlite::memory:), PostgreSQL (postgres://, postgresql://), MariaDB/MySQL (mariadb://, mysql://) — routed by URL scheme via an explicit DbPool enum (not sqlx Any, which has lossy type mapping). Pools live in IoClient behind string handles, mirroring file handles.
  • Results: query returns a list of row objects keyed by column name; execute returns {affected_rows, last_insert_id} (last_insert_id is nothing on PostgreSQL — RETURNING is the documented idiom there).
  • Type mapping: integers/floats/decimals → number, NULLnothing, BOOLEAN → boolean, BLOB/BYTEA → binary, DATE/TIME/TIMESTAMP → date/time/datetime (chrono).
  • Security: parameters always bind through driver-level .bind() — never string interpolation — so SQL injection via values is structurally impossible (covered by a test that round-trips a '; DROP TABLE payload). Placeholders are driver-native (? for SQLite/MariaDB, $1 for PostgreSQL); SQL text is never rewritten.
  • Backward compatibility: zero new lexer keywords. connect and query are contextual with strict lookahead; characterization tests prove programs using query as a variable parse unchanged, and the reserved statement shapes are documented in both keyword references.

Web route parameters

New stdlib helpers (src/stdlib/web.rs):

store params as path_params of path and "/users/:id"
check if params is nothing:
    respond to req with "Not Found" and status 404
otherwise:
    respond to req with params["id"]
end check

:name captures one percent-decoded segment, trailing *name captures the rest, query strings are ignored; path_matches gives a boolean for routing conditionals.

Web server fixes (FRAMEWORK_FINAL_REPORT follow-up)

The report's headline blocker (wait for request comes in on ... failing to parse) no longer reproduces and is locked in with parser regression tests. Verifying against live servers exposed two real bugs, both fixed:

  1. respond ... and status 404 and content_type "text/plain" never responded — the status parsed as the boolean expression 404 and content_type, failing at runtime and leaving the request unanswered (every 404 path in the comprehensive demo was silently broken). Status/content_type values now parse as primary expressions.
  2. header "User-Agent" of req always returned nothing on real requests — warp lowercases header names but the lookup was exact-match. Now case-insensitive; absent headers compare equal to nothing.

Also: the static analyzer now marks variables inside list literals as used, and scripts/run_web_tests.sh no longer dies on its first ((var++)) under set -e.

Testing

  • ~50 new Rust tests: database_parser_test.rs, database_test.rs, route_params_test.rs, respond_statement_parser_test.rs, header_access_runtime_test.rs (real warp round-trips), plus main-loop regression tests.
  • PostgreSQL/MariaDB suites are env-gated (WFL_TEST_POSTGRES_URL / WFL_TEST_MYSQL_URL) and were verified against live PostgreSQL 16 and MariaDB 10.11 servers during development — which caught a real INT4-vs-i64 decode bug SQLite alone would have missed. SQLite suites run everywhere, including Windows.
  • E2E: TestPrograms/database_sqlite_test.wfl (full CRUD, injection resistance, NULL handling, catchable errors) and TestPrograms/web_route_params_test.wfl driven by scripts/run_web_tests.sh|ps1 with curl assertions.
  • New CI database-tests job with postgres:16 + mariadb:11 service containers; existing jobs untouched.
  • cargo fmt --check, clippy --all-targets --all-features -D warnings, and the full test suite are green; all CI-relevant TestPrograms pass.

Docs

New Docs/04-advanced-features/databases.md (every example parse-validated with the release binary; the complete example runs end to end), route-parameters section in web-servers.md, interoperability.md updated from "Planned" to real syntax, keyword reference notes, CHANGELOG, and a Dev diary entry.

https://claude.ai/code/session_01Ah2j9WqDFZwpVGw4oEJxsG


Generated by Claude Code

Summary by CodeRabbit

  • New Features

    • Added built-in database connectivity for SQLite, PostgreSQL, and MariaDB/MySQL with parameterized query support
    • Added web route parameter extraction for dynamic URL path segments
  • Bug Fixes

    • Fixed header name lookup to be case-insensitive
    • Fixed respond statement parsing when combining status and content-type
    • Improved variable usage detection within list expressions
  • Documentation

    • New database integration guide with examples
    • New web route parameters guide

claude added 5 commits June 12, 2026 15:27
Adds web route template matching to the stdlib: path_params extracts
:name segment captures (and trailing *name wildcards) from request
paths, returning an object or nothing; path_matches returns a boolean
for use in check if routing. Captures are percent-decoded and query
strings are ignored. Also enables sqlx's chrono feature ahead of the
database bindings work.

https://claude.ai/code/session_01Ah2j9WqDFZwpVGw4oEJxsG
Adds AST variants OpenDatabaseStatement, DatabaseQueryStatement, and
CloseDatabaseStatement with natural-language parsing:

  open database at "sqlite://./app.db" as db
  connect to database at "postgres://localhost/mydb" as db
  store users as query db with "SELECT ..." and parameters [age]
  store result as execute db with "INSERT ..." and parameters [name]
  close database db

No new lexer keywords: 'connect' and 'query' are contextual with strict
lookahead so existing programs (including variables named query) parse
unchanged; interpreter arms are stubs until the runtime lands. Includes
typechecker rules, transpiler warning, and backward-compatibility
characterization tests.

https://claude.ai/code/session_01Ah2j9WqDFZwpVGw4oEJxsG
Adds src/interpreter/database.rs with an explicit DbPool enum over
sqlx's SqlitePool/PgPool/MySqlPool (MariaDB via the MySQL protocol,
including the mariadb:// scheme alias). IoClient manages pooled
connections behind string handles like file handles.

Rows decode to lists of objects keyed by column name with type-aware
mapping (integers/floats -> number, NULL -> nothing, BOOLEAN -> boolean,
BLOB/BYTEA -> binary, DATE/TIME/TIMESTAMP -> date/time/datetime via
chrono). Execute returns {affected_rows, last_insert_id}; last_insert_id
is nothing on PostgreSQL where RETURNING is the idiom. Parameters always
go through sqlx .bind() so SQL injection via values is not possible;
errors are catchable with try/when error.

Verified against live PostgreSQL 16 and MariaDB 10.11 servers (env-gated
tests via WFL_TEST_POSTGRES_URL / WFL_TEST_MYSQL_URL) plus 12 SQLite
tests that run everywhere with no services.

https://claude.ai/code/session_01Ah2j9WqDFZwpVGw4oEJxsG
Adds TestPrograms/database_sqlite_test.wfl (full CRUD, bound-parameter
injection resistance, NULL handling, catchable errors) and a CI
database-tests job running the env-gated PostgreSQL 16 and MariaDB 11
suites against service containers.

Web server fixes verified against live servers with regression tests:
- respond: 'and status 404 and content_type ...' previously parsed the
  status as the boolean expression '404 and content_type', failing at
  runtime and leaving requests unanswered; status/content_type values
  now parse as primary expressions (tests/respond_statement_parser_test.rs)
- header access is now case-insensitive (warp lowercases header names,
  so 'header "User-Agent" of req' always returned nothing on real
  requests); absent headers now compare equal to the nothing literal
  (tests/header_access_runtime_test.rs)
- main-loop try/catch + wait-for-request shapes from the archived
  FRAMEWORK_FINAL_REPORT are locked in as parser regression tests

Also adds TestPrograms/web_route_params_test.wfl driven by
scripts/run_web_tests.sh|ps1 (route params, percent-decoding, 404
branch, header echo, request counter), fixes the latent set -e
arithmetic-increment bug that made run_web_tests.sh exit before
running any test, marks list-literal elements as variable usages in
the static analyzer, types query results as lists of text-keyed maps,
and maps SQL NULL / no-match results to the nothing literal's runtime
value.

https://claude.ai/code/session_01Ah2j9WqDFZwpVGw4oEJxsG
… diary

Adds Docs/04-advanced-features/databases.md (all examples parse-validated
with the release binary; the complete example runs end to end), replaces
the 'Planned' database section in interoperability.md, documents
path_params/path_matches route templates in web-servers.md, notes the
reserved database statement shapes in both keyword references, and
records the work in CHANGELOG.md and a Dev diary entry.

https://claude.ai/code/session_01Ah2j9WqDFZwpVGw4oEJxsG
Copilot AI review requested due to automatic review settings June 12, 2026 16:41
@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

@coderabbitai

coderabbitai Bot commented Jun 12, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Warning

Review limit reached

@logbie, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 33 minutes and 48 seconds. Learn how PR review limits work.

Your organization has run out of usage credits. Purchase more credits in the billing tab to continue.

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 7ba2b37c-809d-4007-ab52-4686951428c8

📥 Commits

Reviewing files that changed from the base of the PR and between 27b05f0 and 41a1acd.

📒 Files selected for processing (13)
  • CHANGELOG.md
  • Docs/04-advanced-features/databases.md
  • Docs/04-advanced-features/interoperability.md
  • Docs/04-advanced-features/web-servers.md
  • Docs/reference/reserved-keywords.md
  • scripts/run_web_tests.ps1
  • src/analyzer/mod.rs
  • src/interpreter/database.rs
  • src/interpreter/mod.rs
  • src/stdlib/web.rs
  • src/transpiler/javascript.rs
  • src/typechecker/mod.rs
  • tests/database_analyzer_test.rs
📝 Walkthrough

Walkthrough

This PR introduces database connectivity (SQLite, PostgreSQL, MariaDB, MySQL) with sqlx-backed connection pooling and parameterized queries; adds route-parameter extraction helpers (path_params, path_matches); fixes header lookup to be case-insensitive; corrects respond-statement parsing for status and content_type clauses; and establishes CI testing against live database services.

Changes

Database Bindings and Web Enhancements

Layer / File(s) Summary
Database AST and parser implementation
src/parser/ast.rs, src/parser/stmt/database.rs, src/parser/mod.rs, src/parser/stmt/io.rs, src/parser/stmt/variables.rs, src/parser/stmt/mod.rs
Database statement variants (OpenDatabaseStatement, DatabaseQueryStatement with DatabaseQueryKind, CloseDatabaseStatement) are defined in the AST; parser logic recognizes open database at, connect to database at, store ... as query/execute, and close database syntax with full error handling and lookahead for query-vs-execute forms.
Database runtime execution and connection management
src/interpreter/database.rs, src/interpreter/mod.rs
DbPool enum holds backend-specific pooled connections (Postgres/MySQL/SQLite), SqlParam enum represents bind parameters with type conversion, and connect/run_query/run_execute/close functions execute database operations with row decoding, NULL-to-nothing mapping, and async I/O management via IoClient handles.
Database type checking and static analysis
src/typechecker/mod.rs, src/analyzer/mod.rs, src/analyzer/static_analyzer.rs
Type checker enforces Text URLs, Text SQL, list parameters, and Custom("Database") handles; assigns result types based on Query (list of row objects) vs Execute (single row object); static analyzer marks database-related variables as used and traverses list-literal elements for variable detection.
JavaScript transpiler database support
src/transpiler/javascript.rs
Database statements emit unsupported warnings and JavaScript comment placeholders consistent with other async/IO operations.
Web route parameter extraction helpers
src/stdlib/web.rs, src/builtins.rs, src/stdlib/mod.rs, src/stdlib/typechecker.rs, src/stdlib/text.rs
path_params extracts named captures from URLs (:param single-segment, *wildcard rest-of-path) with percent-decoding; path_matches returns boolean match result; both are registered as 2-argument stdlib functions returning Map<Text, Text> and Boolean respectively; percent_decode made crate-visible.
Header access case-insensitivity and respond statement parsing fix
src/interpreter/mod.rs, src/parser/stmt/web.rs
Header lookup now performs case-folded scan returning null for missing headers; respond statement parser uses parse_primary_expression() for status/content_type clauses to prevent consuming downstream and clauses.
Parser and runtime tests for database and route features
tests/database_parser_test.rs, tests/database_test.rs, tests/header_access_runtime_test.rs, tests/main_loop_parser_test.rs, tests/respond_statement_parser_test.rs, tests/route_params_test.rs
Parser tests validate open/connect/close/query/execute syntax, backward-compatibility for query identifier, and nested wait-for handling; runtime tests cover SQLite/PostgreSQL/MariaDB CRUD, parameter binding safety, NULL mapping, error handling, route matching semantics (captures, wildcards, percent-decoding, query-string ignoring).
End-to-end test programs and integration harness updates
TestPrograms/database_sqlite_test.wfl, TestPrograms/web_route_params_test.wfl, scripts/run_integration_tests.ps1, scripts/run_integration_tests.sh, scripts/run_web_tests.ps1, scripts/run_web_tests.sh
WFL E2E programs exercise database DDL/DML/error handling and route parameter extraction; integration test scripts skip route-params from main loop and add Test 3 harness with port polling, endpoint validation (route extraction, 404 handling, header echo), and proper server lifecycle management.
Documentation, CI setup, and changelog
Docs/04-advanced-features/databases.md, Docs/04-advanced-features/web-servers.md, Docs/04-advanced-features/interoperability.md, Docs/reference/keyword-reference.md, Docs/reference/reserved-keywords.md, Dev diary/2026-06-12-database-bindings-and-route-parameters.md, .github/workflows/ci.yml, CHANGELOG.md, Cargo.toml
Comprehensive docs cover database backends/URLs, connection/query/execute/error semantics, route parameter extraction with examples, keyword reservation, database interoperability status; dev diary documents feature scope and testing; CI job runs cargo test --test database_test with PostgreSQL 16 and MariaDB 11 services; sqlx dependency adds chrono feature; CHANGELOG lists additions and fixes.

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

  • WebFirstLanguage/wfl#424: Modifies percent_decode visibility and behavior in src/stdlib/text.rs, directly overlapping with this PR's use of percent-decoding in route parameter matching.
  • WebFirstLanguage/wfl#191: Refactors integration test runner scripts (scripts/run_integration_tests.ps1/.sh) for skip/timeout handling, providing foundational infrastructure this PR builds on.
  • WebFirstLanguage/wfl#169: Introduced HTTP server request/header scaffolding and HeaderAccess expression, which this PR extends with case-insensitive lookup and route parameter extraction.

🐰 The database connects with flair,
Route params dance through the air,
Headers case-blind and free,
A web feature symphony! 🎵

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title comprehensively describes all three major feature additions (database bindings with three backends, web route parameters, and web server fixes).
Docstring Coverage ✅ Passed Docstring coverage is 86.40% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch claude/vibrant-fermi-srm8k1

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds first-class database access and web routing helpers to WFL, plus parser/runtime fixes for the web server path described in FRAMEWORK_FINAL_REPORT.md, backed by extensive new tests, docs, and CI coverage.

Changes:

  • Introduces database statements (open/connect database, query, execute, close database) with sqlx-backed pools and per-backend behavior.
  • Adds path_params / path_matches stdlib helpers for route parameter extraction and matching.
  • Fixes web-server parsing/runtime regressions (respond status/content_type parsing; case-insensitive header lookup) and improves tooling/scripts + CI.

Reviewed changes

Copilot reviewed 40 out of 41 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
tests/route_params_test.rs Adds TDD coverage for path_params / path_matches behavior and edge cases.
tests/respond_statement_parser_test.rs Regression tests ensuring respond ... status ... content_type ... parses clauses correctly.
tests/main_loop_parser_test.rs Regression coverage for parsing request loops with try/catch around wait for request.
tests/header_access_runtime_test.rs Runtime round-trip tests validating case-insensitive header access against a real server/client.
tests/database_test.rs Runtime tests for DB open/query/execute/close across SQLite + env-gated Postgres/MariaDB.
tests/database_parser_test.rs Parser tests for the new DB statement shapes + backward-compat characterization.
TestPrograms/web_route_params_test.wfl E2E web server program exercising route params + regressions.
TestPrograms/database_sqlite_test.wfl E2E SQLite CRUD + injection-resistance + NULL/error handling.
src/typechecker/mod.rs Adds typing rules for DB statements (URL/SQL/params/result typing).
src/transpiler/javascript.rs Warns and no-ops DB statements in JS transpilation output.
src/stdlib/web.rs Implements path_params / path_matches with percent-decoding and wildcard support.
src/stdlib/typechecker.rs Registers stdlib types for path_params / path_matches.
src/stdlib/text.rs Exposes percent_decode for internal stdlib reuse.
src/stdlib/mod.rs Registers new web stdlib module.
src/parser/stmt/web.rs Fixes respond parsing by restricting status/content_type values to primary expressions.
src/parser/stmt/variables.rs Adds DB query/execute lookahead in variable declarations (store ... as query/execute ...).
src/parser/stmt/mod.rs Wires in new database statement parser module.
src/parser/stmt/io.rs Extends open parsing to support open database ....
src/parser/stmt/database.rs New parser for database statements and reserved statement shapes.
src/parser/mod.rs Routes connect to database ... and close database ... in the main statement parser.
src/parser/ast.rs Adds AST nodes for DB statements and DatabaseQueryKind.
src/interpreter/mod.rs Adds IoClient DB handle storage + interpreter execution for DB statements + header lookup fix.
src/interpreter/database.rs Implements sqlx-backed pooled DB connectivity, binding, and row/execute result mapping.
src/builtins.rs Registers routing helper builtins and arity info.
src/analyzer/static_analyzer.rs Marks DB statements’ expressions as used; fixes list-literal element usage tracking.
src/analyzer/mod.rs Adds analyzer handling for DB statements (scope/symbol tracking).
scripts/run_web_tests.sh Fixes set -e arithmetic increment behavior; adds route params E2E web test.
scripts/run_web_tests.ps1 Adds route params E2E web test for Windows.
scripts/run_integration_tests.sh Skips the new route-params web E2E program in the generic suite (run separately).
scripts/run_integration_tests.ps1 Same skip behavior for Windows integration runner.
google_index.html Updates tracked output file content (appears to be generated artifact).
Docs/reference/reserved-keywords.md Documents reserved statement shapes for DB syntax without new keywords.
Docs/reference/keyword-reference.md Adds examples clarifying contextual query/connect database shapes.
Docs/04-advanced-features/web-servers.md Documents new route parameter helpers and templates.
Docs/04-advanced-features/interoperability.md Updates DB integration from “planned” to implemented, links to DB guide.
Docs/04-advanced-features/databases.md New user guide for database feature set, syntax, and type mapping.
Dev diary/2026-06-12-database-bindings-and-route-parameters.md Development diary entry summarizing features/fixes and testing.
CHANGELOG.md Records new DB + routing features and the web/server/analyzer fixes.
Cargo.toml Enables sqlx chrono feature for date/time mappings.
Cargo.lock Locks new chrono dependency via sqlx features.
.github/workflows/ci.yml Adds a DB integration job using Postgres/MariaDB service containers; gates bump-version on it.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/analyzer/mod.rs Outdated
Comment thread src/analyzer/mod.rs
Comment thread src/analyzer/mod.rs
Comment thread src/typechecker/mod.rs Outdated
Comment thread google_index.html

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 7

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
Docs/04-advanced-features/interoperability.md (1)

108-124: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Move Database Integration into the current-feature section.

This subsection says databases are a current capability, but it still sits under ## Planned Interoperability. That makes the document internally inconsistent; move it above the planned section or rename the section to match the status.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@Docs/04-advanced-features/interoperability.md` around lines 108 - 124, The
"### Database Integration" subsection is incorrectly placed under "## Planned
Interoperability"; move the entire "### Database Integration" block (including
the WFL code example and the Databases guide link) out from under the planned
section and place it inside the current-features section (or rename "## Planned
Interoperability" if you prefer to make it accurate), ensuring the heading "###
Database Integration" now appears among other current-feature headings so the
document's status is consistent.
src/interpreter/mod.rs (1)

435-489: ⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift

Database pools can become unreachable but still stay open.

db_handles lives for the full interpreter lifetime, and close_database() is the only removal path. If open database runs in a child scope that exits on return/error before an explicit close, the handle string disappears with the environment while the pool stays registered in IoClient. In a long-running server, repeated scoped opens will accumulate idle pools and exhaust database connections. Tie handle cleanup to scope exit, or add a tracked fallback cleanup path instead of relying on user code to always close explicitly.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/interpreter/mod.rs` around lines 435 - 489, The code leaks DB pools
because open_database() returns only a string handle and removal is only via
explicit close_database(); fix by returning an RAII guard that removes the pool
on scope exit: introduce a new DbHandle struct (e.g., struct DbHandle { id:
String, client: Arc<IoClient> }) and change IoClient::open_database to return
DbHandle (or Result<DbHandle, String>), implement Drop for DbHandle to call a
private cleanup method on IoClient that removes the entry from db_handles and
closes the pool (reusing IoClient::close_database logic), keep
IoClient::get_database and close_database behavior but update call sites to
accept DbHandle.id or the guard itself; this ensures pools are automatically
removed when the guard is dropped and prevents accumulation of unreachable
pools.
🧹 Nitpick comments (4)
google_index.html (1)

1-1: 💤 Low value

Consider excluding this test artifact from version control.

This file appears to be output generated by TestPrograms/test_web_request.wfl when the HTTP request to google.com is blocked. Committing test output artifacts can cause confusion and unnecessary diffs when tests run in different network environments.

Consider adding google_index.html to .gitignore and removing it from the repository. The test can still create this file at runtime without it being tracked.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@google_index.html` at line 1, The file google_index.html is a generated test
artifact from TestPrograms/test_web_request.wfl and should be excluded from VCS;
remove google_index.html from the repository and add its filename (or a suitable
pattern like TestPrograms/*.html or the specific path) to .gitignore so the test
can still create the file at runtime without it being tracked, and update any
CI/test docs to note that the artifact is intentionally untracked.
Docs/reference/reserved-keywords.md (1)

434-434: 💤 Low value

Minor markdown formatting: remove spaces inside code spans.

Line 434 contains spaces inside the inline code span backticks. Markdownlint flags this as a style issue.

✨ Proposed fix
-- `query `.
+`query`.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@Docs/reference/reserved-keywords.md` at line 434, The inline code span
contains extraneous spaces (e.g., `query `) causing markdownlint warnings;
locate the occurrence of the backtick-wrapped token `query ` in
reserved-keywords.md and remove the spaces inside the backticks so it reads
`query` (ensure no leading/trailing spaces remain inside the code span).

Source: Linters/SAST tools

src/stdlib/web.rs (1)

23-64: Consider documenting path traversal risks when using captured parameters.

The wildcard and named parameter captures correctly percent-decode segments but preserve relative path components like .. and .. If captured values are used for file system access (e.g., "/static/*filepath" capturing "../../etc/passwd"), applications must sanitize these values to prevent directory traversal attacks.

Consider adding a security note in the documentation or function docstrings warning that captured parameters should be validated before use in file paths.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/stdlib/web.rs` around lines 23 - 64, The match_path_template function
currently percent-decodes captured segments (both ':' params and '*' wildcard)
but does not sanitize relative path components; add a docstring/top-level
comment on match_path_template warning callers that captured values may contain
".." or "." and must not be used directly for filesystem access, and include a
short remediation note recommending canonicalization or validation (e.g., reject
segments containing "..", normalize with std::fs::canonicalize and verify the
result is inside an allowed base directory, or strip/deny path separators)
before using Value::Text captures for file paths; reference the captured-symbol
handling in match_path_template (the ':' named-parameter branch and the '*'
wildcard branch that inserts Value::Text) so callers know which outputs require
sanitization.
scripts/run_web_tests.ps1 (1)

144-151: 💤 Low value

Empty catch block is acceptable but could be clearer.

The empty catch block at lines 149-151 is intentional (server not ready yet, continue polling). Consider adding a comment to explain this to future maintainers and silence static analysis warnings.

📝 Optional clarifying comment
             try {
                 $rootResponse = Invoke-WebRequest -Uri "http://localhost:8096/" -TimeoutSec 2 -UseBasicParsing -ErrorAction Stop
                 if ($rootResponse.Content -like "*Route server ready*") {
                     $serverReady = $true
                 }
             } catch {
-                # Server not ready yet
+                # Server not ready yet, continue polling (empty catch intentional)
             }
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@scripts/run_web_tests.ps1` around lines 144 - 151, The empty catch block in
the polling logic (the try/catch that invokes Invoke-WebRequest and checks
$rootResponse.Content to set $serverReady) should include a brief explanatory
comment clarifying that the catch is intentional because the server may not be
ready yet and polling should continue; update the catch after the
Invoke-WebRequest block to add that comment (or a no-op like `# intentionally
empty - server not ready yet, continue polling`) so future maintainers and
static analysis tools understand the intent.

Source: Linters/SAST tools

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@CHANGELOG.md`:
- Line 24: The inline code span contains a trailing space (`query `) which
triggers markdownlint; edit the CHANGELOG sentence that mentions the statement
shapes (`store <name> as query <handle> with ...` and `store <name> as execute
<handle> with ...`) to remove the trailing space inside the code span—e.g., use
`query` (no trailing space) or place the required following space outside the
backticks so the inline code contains no internal whitespace while preserving
the intended wording.

In `@Docs/04-advanced-features/databases.md`:
- Around line 53-55: Update the sentence in
Docs/04-advanced-features/databases.md that currently reads “Parameters are
bound by the database driver, which makes SQL injection through values
impossible” to a softened, accurate statement: explain that parameter binding
prevents SQL injection via bound values but does not make SQL injection
impossible overall, and explicitly warn that SQL text/fragments and identifiers
must never be constructed from untrusted input; keep the existing example about
passing values with parameters but replace the absolute wording with this
clarified phrasing.

In `@src/analyzer/mod.rs`:
- Around line 783-796: You’re defining the same database variable twice which
causes Scope::define duplicate errors: for Statement::OpenDatabaseStatement and
Statement::DatabaseQueryStatement a Symbol for variable_name is created before
calling self.analyze_statement(inner) and then defined again later; fix by
removing the early define and only call self.current_scope.define(...) once
after analyzing the inner statement (or check for existing symbol before
defining) so the symbol is created/registered in one place (refer to Symbol
construction, self.current_scope.define, self.analyze_statement(inner), and the
two Statement::... arms).

In `@src/interpreter/database.rs`:
- Around line 49-54: The match arm in value_to_sql_param currently casts
whole-number serde_json::Value::Number to i64 which can silently saturate for
magnitudes > i64::MAX; update value_to_sql_param to check bounds
(i64::MIN..=i64::MAX) before returning SqlParam::Int and, if out of range,
either return an Err or fall through to SqlParam::Float (preferred) so large
whole numbers are preserved as floats; specifically, use the Number's
as_f64()/as_i64() or compare n against i64::MIN as f64 / i64::MAX as f64 and
only produce SqlParam::Int when within bounds, otherwise produce SqlParam::Float
or an error depending on desired behavior.

In `@src/interpreter/mod.rs`:
- Around line 2809-2819: Preflight the destination binding (variable_name)
before performing external side-effects or, if preflight is not possible, ensure
cleanup on define failure: either call a validation helper on env (e.g.,
env.borrow_mut().can_define(variable_name) or attempt a temporary reserve) prior
to calling self.io_client.open_database(&url_str).await, or if you must open
first, then after obtaining handle call env.borrow_mut().define(variable_name,
Value::Text(handle.clone().into())) and on Err immediately release the opened
resource by calling the appropriate cleanup on the IO client (e.g.,
self.io_client.close_database(handle) or drop/dispose the pool) so no live dbN
pool is left unattached; apply the same change to the comparable block around
lines 2891-2904 that uses env.define and open/execute semantics.

In `@src/transpiler/javascript.rs`:
- Around line 611-623: Currently the match arm for
Statement::OpenDatabaseStatement / Statement::DatabaseQueryStatement /
Statement::CloseDatabaseStatement only warns and emits a commented no-op;
instead fail the transpilation with a clear error so callers don't get silently
broken JS. Replace the warn+Ok(...) branch in the match (the arm handling
OpenDatabaseStatement, DatabaseQueryStatement, CloseDatabaseStatement) with code
that returns an Err describing the unsupported database statement (include the
statement kind and the line/column), e.g. produce a Diagnostic or Error from the
transpiler with a message like "Database statements (open/query/close) are not
supported in JavaScript output" and include *line and *column; do not emit a
commented no-op or continue successfully.

In `@src/typechecker/mod.rs`:
- Around line 968-969: The code currently only calls infer_expression_type(db)
for the `db` expression (via infer_expression_type) but does not validate that
`db` is actually a Database, so `query`/`execute` can compile with a wrong type
and only fail at runtime; apply the same Database-type enforcement you already
use for `close database` (the checks at the close-database handling around lines
1015–1019) to the query/execute handling: after infer_expression_type(db) in the
query/execute branch, perform the explicit Database type check (the same
logic/utility used for `close database`) and emit the same type error if it is
not a Database so `query`/`execute` fails at compile time.

---

Outside diff comments:
In `@Docs/04-advanced-features/interoperability.md`:
- Around line 108-124: The "### Database Integration" subsection is incorrectly
placed under "## Planned Interoperability"; move the entire "### Database
Integration" block (including the WFL code example and the Databases guide link)
out from under the planned section and place it inside the current-features
section (or rename "## Planned Interoperability" if you prefer to make it
accurate), ensuring the heading "### Database Integration" now appears among
other current-feature headings so the document's status is consistent.

In `@src/interpreter/mod.rs`:
- Around line 435-489: The code leaks DB pools because open_database() returns
only a string handle and removal is only via explicit close_database(); fix by
returning an RAII guard that removes the pool on scope exit: introduce a new
DbHandle struct (e.g., struct DbHandle { id: String, client: Arc<IoClient> })
and change IoClient::open_database to return DbHandle (or Result<DbHandle,
String>), implement Drop for DbHandle to call a private cleanup method on
IoClient that removes the entry from db_handles and closes the pool (reusing
IoClient::close_database logic), keep IoClient::get_database and close_database
behavior but update call sites to accept DbHandle.id or the guard itself; this
ensures pools are automatically removed when the guard is dropped and prevents
accumulation of unreachable pools.

---

Nitpick comments:
In `@Docs/reference/reserved-keywords.md`:
- Line 434: The inline code span contains extraneous spaces (e.g., `query `)
causing markdownlint warnings; locate the occurrence of the backtick-wrapped
token `query ` in reserved-keywords.md and remove the spaces inside the
backticks so it reads `query` (ensure no leading/trailing spaces remain inside
the code span).

In `@google_index.html`:
- Line 1: The file google_index.html is a generated test artifact from
TestPrograms/test_web_request.wfl and should be excluded from VCS; remove
google_index.html from the repository and add its filename (or a suitable
pattern like TestPrograms/*.html or the specific path) to .gitignore so the test
can still create the file at runtime without it being tracked, and update any
CI/test docs to note that the artifact is intentionally untracked.

In `@scripts/run_web_tests.ps1`:
- Around line 144-151: The empty catch block in the polling logic (the try/catch
that invokes Invoke-WebRequest and checks $rootResponse.Content to set
$serverReady) should include a brief explanatory comment clarifying that the
catch is intentional because the server may not be ready yet and polling should
continue; update the catch after the Invoke-WebRequest block to add that comment
(or a no-op like `# intentionally empty - server not ready yet, continue
polling`) so future maintainers and static analysis tools understand the intent.

In `@src/stdlib/web.rs`:
- Around line 23-64: The match_path_template function currently percent-decodes
captured segments (both ':' params and '*' wildcard) but does not sanitize
relative path components; add a docstring/top-level comment on
match_path_template warning callers that captured values may contain ".." or "."
and must not be used directly for filesystem access, and include a short
remediation note recommending canonicalization or validation (e.g., reject
segments containing "..", normalize with std::fs::canonicalize and verify the
result is inside an allowed base directory, or strip/deny path separators)
before using Value::Text captures for file paths; reference the captured-symbol
handling in match_path_template (the ':' named-parameter branch and the '*'
wildcard branch that inserts Value::Text) so callers know which outputs require
sanitization.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 2823f1a2-19e0-4c36-b10d-6aadb6c2b1e6

📥 Commits

Reviewing files that changed from the base of the PR and between 334bcef and 27b05f0.

⛔ Files ignored due to path filters (1)
  • Cargo.lock is excluded by !**/*.lock
📒 Files selected for processing (40)
  • .github/workflows/ci.yml
  • CHANGELOG.md
  • Cargo.toml
  • Dev diary/2026-06-12-database-bindings-and-route-parameters.md
  • Docs/04-advanced-features/databases.md
  • Docs/04-advanced-features/interoperability.md
  • Docs/04-advanced-features/web-servers.md
  • Docs/reference/keyword-reference.md
  • Docs/reference/reserved-keywords.md
  • TestPrograms/database_sqlite_test.wfl
  • TestPrograms/web_route_params_test.wfl
  • google_index.html
  • scripts/run_integration_tests.ps1
  • scripts/run_integration_tests.sh
  • scripts/run_web_tests.ps1
  • scripts/run_web_tests.sh
  • src/analyzer/mod.rs
  • src/analyzer/static_analyzer.rs
  • src/builtins.rs
  • src/interpreter/database.rs
  • src/interpreter/mod.rs
  • src/parser/ast.rs
  • src/parser/mod.rs
  • src/parser/stmt/database.rs
  • src/parser/stmt/io.rs
  • src/parser/stmt/mod.rs
  • src/parser/stmt/variables.rs
  • src/parser/stmt/web.rs
  • src/stdlib/mod.rs
  • src/stdlib/text.rs
  • src/stdlib/typechecker.rs
  • src/stdlib/web.rs
  • src/transpiler/javascript.rs
  • src/typechecker/mod.rs
  • tests/database_parser_test.rs
  • tests/database_test.rs
  • tests/header_access_runtime_test.rs
  • tests/main_loop_parser_test.rs
  • tests/respond_statement_parser_test.rs
  • tests/route_params_test.rs

Comment thread CHANGELOG.md Outdated
Comment thread Docs/04-advanced-features/databases.md Outdated
Comment thread src/analyzer/mod.rs Outdated
Comment thread src/interpreter/database.rs
Comment thread src/interpreter/mod.rs
Comment thread src/transpiler/javascript.rs
Comment thread src/typechecker/mod.rs Outdated
- analyzer: remove the wait-for pre-define block that double-defined
  variables introduced by inner statements; 'wait for store rows as
  query db with ...' (and the pre-existing 'wait for open file ... as'
  case) no longer report spurious 'already been defined' errors
  (tests/database_analyzer_test.rs)
- analyzer: database symbols now carry the statement's real line/column
  instead of 0,0 so redeclaration diagnostics point at the right place
- typechecker: query/execute now validate the handle is a Database
  connection, matching close database
- database: whole numbers beyond i64 range fall back to float binding
  instead of silently saturating in the cast (unit tests added)
- interpreter: close the freshly opened pool if binding the handle
  variable fails, so no unreachable pool is left registered
- transpiler: database statements now fail JavaScript transpilation
  with a clear error like other interpreter-only statements, instead of
  silently emitting no-ops
- stdlib/docs: document that route-parameter captures are untrusted and
  must be validated before filesystem use; soften the docs' SQL
  injection wording (binding protects values, not SQL text built from
  untrusted input); move Database Integration out of the 'Planned'
  interoperability section; fix markdownlint code-span spacing; comment
  the intentional empty catch in run_web_tests.ps1

https://claude.ai/code/session_01Ah2j9WqDFZwpVGw4oEJxsG
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants