Repository navigation
feat!: name the project file's loosening flags allow_, off by default - #187
colinhacks wants to merge 2 commits into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
ℹ️ No critical issues. Two rough edges worth a look.
Reviewed changes
The whole PR at 5ac4ff0: the strict_params → allow_untyped_params rename and the new allow_nested_arrays flag, followed through the core config, the CLI, the type check, the build step, the run time and the docs.
allow_untyped_params: replacesstrict_paramsand reads the other way round.strictandstrict_paramsare unknown keys with no alias, and each error says what to write instead (engine/core/src/config.rs).allow_nested_arrays:analysisWithNestedArraysmarks eachany_deptharray column afteranalysisWithBases. The mapper renders it asNestedArray<T>, the loader emitsnestedArrays: true(Postgres only) along with a nesting output schema, and.run()and the Standard Query decoder skipwithinDimensions.coercealready recurses into nested arrays, so values decode at any depth.- Cache identity: the flag is part of both the mapper's
configIdentityand the build session's key, so editing it reaches a running type check or watch process. - Rendering: an array of arrays now reads
(number | null)[][].
I ran tsc --noEmit --runExternalCode and it passes. tests/nested-arrays.test.ts and the array test in tests/standard-query.test.ts also pass. The live tests did not run here because there is no engine binary in this environment.
ℹ️ reylo play doesn't read allow_nested_arrays
reylo play takes strictParams from the project but not the new flag. With allow_nested_arrays: true, its result pane still shows (number | null)[] for an array column while the type check shows NestedArray<number | null>. The PR body says allow_nested_arrays types the column at any depth, so reylo play disagrees with the editor for the same project.
Technical details
# `reylo play` ignores `allow_nested_arrays`
## Affected sites
- src/play.ts:167 — `playResult(..., { strictParams: project.strictParams, width, ms })` passes no nested-arrays option
- src/play-result.ts:33,41 — `options` has no such field, and `moduleTypes(sql, outcome.analysis, dialect, outcome.lexed, options.strictParams ?? true)` omits the 7th argument
## Required outcome
- On a Postgres project with `allow_nested_arrays: true`, `reylo play` renders array columns as the type check does (`NestedArray<T>`), the same way it already follows `allow_untyped_params`.
## Suggested approach
- Add `allowNestedArrays?: boolean` to `playResult`'s options, pass `project.allowNestedArrays && dialect === "postgres"` from src/play.ts, and forward it to `moduleTypes`, as src/mapper-server.ts:151 does.claude-opus-5-5 ｜ 𝕏
|
On the review's |
There was a problem hiding this comment.
✅ No new issues found. Both points from the last review are fixed.
Reviewed changes
This run covers 04c7daf, the only commit since the last review at 5ac4ff0.
reylo playfollowsallow_nested_arrays:playResulttakes a newallowNestedArraysoption and passes it tomoduleTypes.src/play.tssuppliesproject.allowNestedArrays && dialect === "postgres", the same expression assrc/mapper-server.ts:151, so the play pane and the type check now show the same types. A new test intests/nested-arrays.test.tscovers this.- Corrected doc comment: the parenthetical in
src/module-types.ts:102now says the error is the default. Before, it read as ifallow_untyped_paramsdefaulted to true.
I ran tests/nested-arrays.test.ts (3 passed) and tsc --noEmit --runExternalCode (clean).
claude-opus-5-5 ｜ 𝕏
e0cfbdb to
7d1e2a6
Compare
There was a problem hiding this comment.
✅ No new issues found. The rebase onto
js.codecs(#193) brings both features together correctly.
Reviewed changes
The diff since e0cfbdb is the branch rebased onto main at d15b515. It moves allow_nested_arrays onto the code that #193 added for js.codecs.
moduleTypessignature:codecsnow comes beforeallowNestedArrays. Every caller was updated:src/mapper-server.ts,src/play-result.ts(which passesundefined, undefined, flag) andtests/nested-arrays.test.ts.- Mapper project state:
ProjectandconfigIdentityhold bothcodecsandallowNestedArrays. A change to either one still reaches a running type check. - Loader: the module now carries the
codecsplan andnestedArrays: truetogether.querySchemasgets the marked analysis along with the project's codecs. - Run time: in
src/standard-query.ts, the codec plan wraps thepostgresDecoderthat knows about nesting. Insrc/execute.ts,withCodecswraps the path that skips the dimension check.decodePlanandencodePlanalready handle arrays of any depth, so a codec still applies to the elements of aNestedArray<T>column. - Render and schema: the array branches now pass
codecsthrough and still checknestsAnyDeep. - Docs and skill: the
postgres.mdxlimits table and theSKILL.mddata-types bullet now include #193's wording alongside theallow_nested_arraysnote.
tsc --noEmit --runExternalCode passes, and so do tests/nested-arrays.test.ts, decode, standard-query and render. Some codecs and SQLite-driver tests fail here, but only because of this environment: Node 24 has no global Temporal, and with no engine binary, js.codecs resolves to {}.
claude-opus-5-5 ｜ 𝕏
7d1e2a6 to
3f71bff
Compare
The project file's two switches that loosen what Reylo accepts now read the same way: each is named
allow_and is false by default.allow_untyped_paramsreplacesstrict_paramsand reads the other way round:allow_untyped_params: trueis whatstrict_params: falsewas. There is no alias.strict_paramsandstrictare unknown keys, and the error for each says what to write instead.allow_nested_arraysis new. Since #181, a Postgres array column is typed with one dimension and.run()throws on a value of more, since Postgres doesn't track an array's dimensions. With this set:NestedArray<T>, whichreylonow exports.run()and the Standard Query decoder skip the dimension checkreylo playshows the same types as the type checkThe run-time refusal now names the key as its other way out.
Also
(number | null)[][], not((number | null)[])[].Verified
cargo build --workspace --all-targetswith no warning; core's and thereylocrate's tests.nub run typecheck, andnub run testagainst Postgres 18 and MySQL 8.4 on the release binary: 368 passed, none skipped.nub run buildinsite/.