docs: correct stale type names in prqlc ARCHITECTURE.md - #6254
Conversation
prql-bot
left a comment
There was a problem hiding this comment.
The four renames hold up against the tree: RootModule and its module field, Expr::target_id, Lineage, and AnchorContext in sql::pq::context — whose relation_instances, column_decls, and column_names do line up one-to-one with the three bullets the doc already had, so leaving that list alone was right. The frame → lineage swap in the type-inference bullet is grounded too: the resolver derives the type from the lineage via ty_of_lineage in semantic/resolver/expr.rs.
One stale name in the same file that this PR leaves in place. The stage table expands RQ as Resolved Query (the semantic | lowering row), but the crate calls it Relational Query: prqlc/prqlc/src/ir/rq/mod.rs opens with //! Relational Query AST, and web/website/content/roadmap.md writes "RQ (Relational Query intermediate". "Resolved Query" appears nowhere else in the repo. That's the same defect class as the four being corrected here, ten lines above the first hunk, so a reader who follows the table into ir/rq/ hits the same dead end this PR is closing elsewhere — worth folding in rather than leaving the table contradicting the module it names.
It's outside the diff, so no inline suggestion; the replacement row keeps the column width at 39 characters, so the table stays aligned:
-| semantic | lowering | PL -> _RQ — Resolved Query_ |
+| semantic | lowering | PL -> _RQ — Relational Query_ |Nothing else: the reflowed paragraphs match prettier's 80-column proseWrap: always, and the rest of the file's identifiers (Flattener for the flatten sub-stage, chumsky for the lexer, the PL/PQ/LR/PR expansions) all still resolve.
The crate calls it Relational Query (ir/rq/mod.rs opens with "//! Relational Query AST"), as does web/website/content/roadmap.md. "Resolved Query" appeared nowhere else in the repo.
prql-bot
left a comment
There was a problem hiding this comment.
Resolved. 4b99469 is the one-line row this review asked for and the only change since f4d5f30: the stage table now expands RQ as Relational Query, matching //! Relational Query AST at the top of prqlc/prqlc/src/ir/rq/mod.rs, and "Resolved Query" no longer appears anywhere in the repo. The replacement keeps every row of the stage table at 67 characters, so the alignment prettier expects still holds.
Nothing else outstanding — the four renames from the first review already checked out. Recording this as a comment rather than an approval only because GitHub rejects self-approval; a maintainer merge is all that's left.
prqlc/prqlc/ARCHITECTURE.mdnames five types, fields, and expansions that no longer match the source, so a reader following it into the crate finds nothing. This updates them to the current names. Verified by grepping each identifier against the tree at16fda77; no prose beyond the renames is changed.Context(semantic stage)RootModule— its own doc comment still reads "Context of the pipeline"Context::root_modRootModule::moduleExpr::targetExpr::target_idLineage— noFrametype existssql::contextAnchorContextinsql::pq::contextir/rq/mod.rs)How each was checked
Contextstruct left in the crate is a private one insql/mod.rs— a different thing from the semantic-stage type the doc means, so leaving the name as-is points a reader at the wrong struct. The rename landed in internal: Combineprql-compilerintoprqlc#4083;ARCHITECTURE.mdhas had no substantive edit since it was moved in docs: Move architecture file to satisfycargo publish#4954.grep -rn 'struct Frame\b\|pub type Frame\b' prqlc/prqlc/src/returns nothing.sql/context.rs;sql/containspq/context.rs, whoseAnchorContextfields match the doc's existing bullet list one-to-one —relation_instances/table_decls(table instances),column_decls(column definitions),column_names(column names). Only the module path and type name needed changing, not the description.ir/rq/mod.rsopens with//! Relational Query AST,sql/pq/ast.rswith//! Sql Relational Query AST, andweb/website/content/roadmap.mdwrites "RQ (Relational Query intermediate". A repo-wide grep for "Resolved Query" matched only theARCHITECTURE.mdrow this PR changes.No regression test: documentation-only, with no behavior to assert against. The identifiers were each verified by grep rather than by a test.