Add PLpgSQL language support#157
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis pull request adds PL/pgSQL support by introducing and exporting Sequence Diagram(s)sequenceDiagram
actor User
participant Parser
participant LanguageRegistry
participant TreeSitterSQL
participant PlpgsqlConfig
User->>Parser: parseFile("tests/fixtures/plpgsql.sql")
Parser->>LanguageRegistry: getLanguageConfig("plpgsql")
LanguageRegistry-->>Parser: plpgsqlConfig
Parser->>TreeSitterSQL: parse(source, plpgsqlConfig.parser)
TreeSitterSQL-->>Parser: syntaxTree
Parser->>PlpgsqlConfig: run symbolQueries & exportQueries on syntaxTree
PlpgsqlConfig-->>Parser: symbols & exports
Parser-->>User: parsed chunks with language 'plpgsql', symbols, exports
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tests/unit/parser.test.ts (1)
344-369:⚠️ Potential issue | 🟡 MinorDuplicate test block for C fixtures.
Lines 344-369 are exact duplicates of lines 240-265 (both test "should parse C fixtures correctly" and "should extract symbols from C fixtures correctly"). This appears to be an unintentional copy-paste error.
🧹 Remove duplicate tests
- it('should parse C fixtures correctly', () => { - const filePath = path.resolve(__dirname, '../fixtures/c.c'); - const result = parser.parseFile(filePath, 'main', 'tests/fixtures/c.c'); - expect(cleanTimestamps(result.chunks)).toMatchSnapshot(); - }); - - it('should extract symbols from C fixtures correctly', () => { - const filePath = path.resolve(__dirname, '../fixtures/c.c'); - const result = parser.parseFile(filePath, 'main', 'tests/fixtures/c.c'); - const allSymbols = result.chunks.flatMap((chunk) => chunk.symbols); - expect(allSymbols).toEqual( - expect.arrayContaining([ - expect.objectContaining({ name: 'add', kind: 'function.name' }), - expect.objectContaining({ name: 'test_function', kind: 'function.name' }), - expect.objectContaining({ name: 'main', kind: 'function.name' }), - expect.objectContaining({ name: 'add', kind: 'function.call' }), - expect.objectContaining({ name: 'printf', kind: 'function.call' }), - expect.objectContaining({ name: 'Point', kind: 'struct.name' }), - expect.objectContaining({ name: 'Data', kind: 'union.name' }), - expect.objectContaining({ name: 'Color', kind: 'enum.name' }), - expect.objectContaining({ name: 'Point_t', kind: 'type.name' }), - expect.objectContaining({ name: 'global_var', kind: 'variable.name' }), - expect.objectContaining({ name: 'point', kind: 'variable.name' }), - ]) - ); - }); -
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 3a727b07-03c8-4fda-8739-1b1d143f7528
⛔ Files ignored due to path filters (2)
package-lock.jsonis excluded by!**/package-lock.jsontests/unit/__snapshots__/parser.test.ts.snapis excluded by!**/*.snap
📒 Files selected for processing (7)
package.jsonsrc/languages/index.tssrc/languages/plpgsql.tssrc/types/tree-sitter-sql.d.tstests/fixtures/plpgsql.sqltests/unit/languages.test.tstests/unit/parser.test.ts
|
Rebased this branch onto current upstream/main to resolve merge conflicts after Scala/HCL support landed in #155.\n\nWhat changed in the rebase:\n- preserved upstream Scala + HCL additions\n- merged in PL/pgSQL support on top\n- reran dependency install/native rebuilds under Node 22\n- revalidated build, test, and lint on the rebased branch\n\nValidation after rebase:\n- npm run build\n- npm test\n- npm run lint\n\nThis force-push only updates branch history to keep the PR mergeable against current main. |
92fc459 to
fc455f6
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
package.json (1)
39-39:⚠️ Potential issue | 🔴 CriticalRe-verify tree-sitter compatibility before merge.
Line 39 adds
@derekstride/tree-sitter-sql, while the repo pinstree-sitter@^0.25.0(Lines 36 and 67). If the grammar’s peer range is still^0.21.x, this can break dependency resolution or runtime parser loading.#!/bin/bash # Verify published peer dependency constraints for the added parser npm view `@derekstride/tree-sitter-sql`@0.3.11 peerDependencies --json npm view `@derekstride/tree-sitter-sql`@0.3.11 engines --json # Verify effective tree-sitter requirement in this repo manifest jq '.overrides["tree-sitter"], .dependencies["tree-sitter"]' package.json
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: ASSERTIVE
Plan: Pro
Run ID: bb20f383-a6c0-428e-98ba-002c2f581ca0
⛔ Files ignored due to path filters (2)
package-lock.jsonis excluded by!**/package-lock.jsontests/unit/__snapshots__/parser.test.ts.snapis excluded by!**/*.snap
📒 Files selected for processing (7)
package.jsonsrc/languages/index.tssrc/languages/plpgsql.tssrc/types/tree-sitter-sql.d.tstests/fixtures/plpgsql.sqltests/unit/languages.test.tstests/unit/parser.test.ts
|
@coderabbitai review Addressed the remaining parser-test feedback by replacing the PL/pgSQL snapshot-only assertion with targeted invariant assertions and removing the obsolete snapshot entry. |
|
✅ Actions performedReview triggered.
|
|
@coderabbitai resolve |
✅ Actions performedComments resolved and changes approved. |
|
Tested PR #157 with custom image
Results:
Observed indices / aliases:
Language detection seen in
Example verification:
One unrelated infra bug surfaced during testing: repo names containing Net: PR looks good from an indexing/search validation standpoint. |
The @derekstride/tree-sitter-sql package downloads a glibc-linked tree-sitter binary during install, which fails on Alpine's musl libc. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Dockerfile (1)
32-32: 🧹 Nitpick | 🔵 TrivialRedundant
chownoperation.Lines 29-31 already copy files with
--chown=indexer:nodejs, making this recursive chown unnecessary. Removing it reduces build time and image layer size.♻️ Proposed fix to remove redundant chown
COPY --from=builder --chown=indexer:nodejs /app/dist ./dist COPY --from=builder --chown=indexer:nodejs /app/node_modules ./node_modules COPY --from=builder --chown=indexer:nodejs /app/package*.json ./ -RUN chown -R indexer:nodejs . USER indexer
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 8fd11bf7-838b-49dd-ad7f-1d88b6f94383
📒 Files selected for processing (1)
Dockerfile
CodeRabbit Review ResponseNode type names (Critical — False Positive)CodeRabbit flagged that the queries in The grammar uses Other comments
|
|
@coderabbitai resolve |
✅ Actions performedComments resolved and changes approved. |
There was a problem hiding this comment.
stripping off comments happens all the time with rushing models. They are not all self-evident at all. Some are self-evident and should be removed others should stay. I would instruct it to not do this in your system prompt if you use those models.
These comments are also helpful for the context for next agents themselves. Let me know wdyt?
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The original PLpgSQL commit inadvertently removed every inline comment from the test file. This restores the file to main's state and cleanly re-applies only the PLpgSQL test additions. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2 participants
Summary
Validation