Improved stdlib matrix app - #4349
Merged
Merged
Conversation
Now has both web and node modules, and they are better categorized
Also added "unnecessary" classification
It was an unreliable heuristic that is better achieved by just letting the LLM decide and provide documentation
If an agent doesn't know the exact name of something it can try to find items that talk about the concept.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
⏭️ Performance benchmarks were skippedPerf benchmarks (CodSpeed) are opt-in on pull requests — they no longer run on every push. They always run automatically after merge to To run them on this PR, do any of the following, then push a commit (or re-run CI):
|
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (11)
💤 Files with no reviewable changes (1)
🚧 Files skipped from review as they are similar to previous changes (7)
📝 WalkthroughWalkthroughThis PR adds full-text search to ChangesCLI symbol search
TypeScript-first stdlib matrix
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant TypeScriptExtractor
participant AlignmentSessions
participant BAMLToolbox
participant Compiler
participant MatrixReport
participant MatrixUI
TypeScriptExtractor->>AlignmentSessions: provide module-qualified symbols
AlignmentSessions->>BAMLToolbox: request symbol and documentation lookups
BAMLToolbox->>Compiler: compile returned examples
Compiler-->>BAMLToolbox: validation diagnostics
BAMLToolbox-->>AlignmentSessions: tool answers and validation results
AlignmentSessions->>MatrixReport: store TypeScript-keyed judgements
MatrixReport->>MatrixUI: provide states, counterparts, and examples
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (5)
tools/stdlib-matrix/baml_src/align.baml (2)
373-384: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUpdate the stale comment that names
session_final.No
session_finalfunction exists in this file. The final round calls the samesession_step, which returnsStep, so the model can still returnInvestigate. The shape of the call does not enforce an answer-only turn; only the prompt text does. Correct the comment so a reader does not assume a schema-level guarantee.🤖 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 `@tools/stdlib-matrix/baml_src/align.baml` around lines 373 - 384, Update the comment above the round == lookup_rounds() branch to remove the incorrect references to session_final and schema-enforced answer-only behavior. State that the final round still uses session_step and relies on the prompt text to request an answer without further lookups.
329-349: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove the duplicated documentation paragraph.
Lines 331-333 and lines 347-349 state the same rule twice. Keep one copy.
🤖 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 `@tools/stdlib-matrix/baml_src/align.baml` around lines 329 - 349, Remove the repeated “A session that will not answer is a failure…” documentation paragraph from the surrounding session documentation, keeping a single copy while preserving the other explanatory paragraphs unchanged..github/workflows/stdlib-matrix.yml (1)
197-202: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winThe coverage gate compares absolute counts across different TypeScript surfaces.
.counts.unjudgedis compared directly between the deployed report and this run. The two reports can describe different surfaces: this workflow runs after a release, and a TypeScript or@types/nodeupgrade adds symbols. New symbols are unjudged until a session reaches them, sounjudgedcan rise while coverage is equal or better. The gate then blocks publishing, and the deployed report stays stale until someone forces a run.Compare judged coverage as a proportion of
ts_symbols, or skip the comparison whents_symbolsdiffers between the two reports.🤖 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 @.github/workflows/stdlib-matrix.yml around lines 197 - 202, Update the coverage gate around the now and before jq reads to account for differing TypeScript surfaces: compare unjudged coverage as a proportion of each report’s ts_symbols, or skip the regression check when ts_symbols values differ. Preserve the existing failure message and exit behavior only for a genuine proportional regression.tools/stdlib-matrix/baml_src/tests.baml (2)
979-985: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueThe snippet-compilation test hard-fails when the debug CLI is not built.
baml.sys.panicruns when../../baml_language/target/debug/baml-cliis missing. Any contributor who runs the suite without first buildingbaml_cligets a failing test rather than a skipped one. The path also assumes a debug profile and the project directory as the working directory.If the test framework supports skipping, skip when the binary is absent. Otherwise document the
cargo build -p baml_cli --bin baml-cliprerequisite intools/stdlib-matrix/README.md.🤖 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 `@tools/stdlib-matrix/baml_src/tests.baml` around lines 979 - 985, Update the snippet-compilation test around the compiler existence check to skip the test when the debug baml-cli binary is absent, using the test framework’s supported skip mechanism instead of baml.sys.panic. Preserve the existing compiler path and build_toolbox flow when the binary exists; if skipping is unavailable, document the cargo build prerequisite in the stdlib-matrix README.
877-890: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAlign optional fields in
Findingliterals.BAML allows omitting optional struct fields, so
fixture_findingis valid. Addexample: nullhere if project style prefers all explicit optional assignments inFindingliterals.🤖 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 `@tools/stdlib-matrix/baml_src/tests.baml` around lines 877 - 890, Add the optional example field explicitly as null in the Finding literal returned by fixture_finding, matching the project’s convention for fully specifying optional Finding fields while preserving the existing values.
🤖 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 @.github/workflows/stdlib-matrix.yml:
- Around line 119-125: Update the previous.json parsing around the version
assignment in the matrix workflow to handle jq decode failures without
triggering set -e. Treat malformed or unreadable JSON exactly like an
unsupported report: set the have output to false, emit the existing cold-run
notice or equivalent, remove previous.json, and exit successfully.
In `@baml_language/crates/baml_cli/src/describe_search.rs`:
- Around line 137-149: Update the item path construction in the candidate
closure to prepend the item's package before its namespace and name, producing
the documented fully qualified path such as baml.sys.exec. Preserve the existing
namespace and name formatting while ensuring the package is included in every
Candidate.path result.
In `@tools/stdlib-matrix/baml_src/align.baml`:
- Around line 725-744: Update baml_finding_names so collisions for
strip_kind_prefix(symbol.id) are handled like display-name collisions: preserve
the existing mapping when the stripped path resolves to the same symbol, but set
it to "" when different symbols share that path. Do not allow the current direct
names.set call to overwrite an existing conflicting stripped-path entry.
- Around line 428-455: Move the held-findings drain from the attempts-exhaustion
branch to the post-loop path, using without_example for each entry in held and
marking its id covered. Ensure this cleanup runs whenever the loop ends,
including round-budget exhaustion, before the final !answered &&
collected.length() == 0 check; avoid draining the same findings twice.
In `@tools/stdlib-matrix/baml_src/entry.baml`:
- Around line 239-243: Update the ToolReport construction in dump_tools to avoid
calling load_skill, which can throw after answers have been computed. Read the
cached skill file directly for skill_bytes, or use 0 when the cache is
unavailable, while preserving the existing answers and other report fields.
- Around line 273-284: Update the subject-selection condition in the session
request loop to use the same case-insensitive substring matching as --only,
rather than exact equality. Ensure chunked labels such as "<subject> (part 1 of
2)" still populate sample and keep dump_plan selection consistent with --only.
In `@tools/stdlib-matrix/baml_src/references.baml`:
- Around line 299-318: Update attach_ts_references to resolve each node
container’s bare signature names within its module scope, using the symbol’s
module as the scope passed to resolve_reference/resolve_all. Keep global and web
resolution behavior unchanged, while ensuring names such as Readable resolve to
qualified ids like node:stream.Readable so references are recorded for freshness
checks.
In `@tools/stdlib-matrix/baml_src/toolbox.baml`:
- Around line 355-367: Update fetch_skill to pass an explicit finite timeout to
baml.http.fetch, matching the timeout parameter convention and duration used by
comparable calls elsewhere in the file. Preserve the existing error handling and
empty-string fallback behavior.
In `@typescript2/app-stdlib-matrix/src/components/matrix-symbol.ts`:
- Around line 318-325: Update the rendering flow in the matrix-symbol component
so far-side BAML IDs are rendered independently from judgement-level metadata.
Extend the endpoint rendering to include IDs from unnecessary judgements, while
rendering each judgement’s reason, example, and provenance only once rather than
once per ID. Adjust the counterparts/counterpart and unnecessary rendering paths
accordingly, preserving existing absence handling.
---
Nitpick comments:
In @.github/workflows/stdlib-matrix.yml:
- Around line 197-202: Update the coverage gate around the now and before jq
reads to account for differing TypeScript surfaces: compare unjudged coverage as
a proportion of each report’s ts_symbols, or skip the regression check when
ts_symbols values differ. Preserve the existing failure message and exit
behavior only for a genuine proportional regression.
In `@tools/stdlib-matrix/baml_src/align.baml`:
- Around line 373-384: Update the comment above the round == lookup_rounds()
branch to remove the incorrect references to session_final and schema-enforced
answer-only behavior. State that the final round still uses session_step and
relies on the prompt text to request an answer without further lookups.
- Around line 329-349: Remove the repeated “A session that will not answer is a
failure…” documentation paragraph from the surrounding session documentation,
keeping a single copy while preserving the other explanatory paragraphs
unchanged.
In `@tools/stdlib-matrix/baml_src/tests.baml`:
- Around line 979-985: Update the snippet-compilation test around the compiler
existence check to skip the test when the debug baml-cli binary is absent, using
the test framework’s supported skip mechanism instead of baml.sys.panic.
Preserve the existing compiler path and build_toolbox flow when the binary
exists; if skipping is unavailable, document the cargo build prerequisite in the
stdlib-matrix README.
- Around line 877-890: Add the optional example field explicitly as null in the
Finding literal returned by fixture_finding, matching the project’s convention
for fully specifying optional Finding fields while preserving the existing
values.
🪄 Autofix
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: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 97279ea9-132d-4785-9215-a2da66ca33e4
⛔ Files ignored due to path filters (1)
baml_language/crates/baml_cli/src/snapshots/baml_cli__help_command__tests__describe_detailed_help.snapis excluded by!**/*.snap
📒 Files selected for processing (27)
.github/workflows/stdlib-matrix.ymlbaml_language/crates/baml_cli/src/describe_command.rsbaml_language/crates/baml_cli/src/describe_search.rsbaml_language/crates/baml_cli/src/lib.rstools/stdlib-matrix/README.mdtools/stdlib-matrix/baml_src/align.bamltools/stdlib-matrix/baml_src/entry.bamltools/stdlib-matrix/baml_src/ingest.bamltools/stdlib-matrix/baml_src/models.bamltools/stdlib-matrix/baml_src/pools.bamltools/stdlib-matrix/baml_src/references.bamltools/stdlib-matrix/baml_src/render.bamltools/stdlib-matrix/baml_src/report.bamltools/stdlib-matrix/baml_src/revision.bamltools/stdlib-matrix/baml_src/surface.bamltools/stdlib-matrix/baml_src/tests.bamltools/stdlib-matrix/baml_src/toolbox.bamltools/stdlib-matrix/baml_src/ts_symbols.bamltools/stdlib-matrix/extractors/ts-surface.mjstools/stdlib-matrix/runtypescript2/app-stdlib-matrix/src/components/matrix-app.tstypescript2/app-stdlib-matrix/src/components/matrix-group.tstypescript2/app-stdlib-matrix/src/components/matrix-symbol.tstypescript2/app-stdlib-matrix/src/composition.tstypescript2/app-stdlib-matrix/src/index.csstypescript2/app-stdlib-matrix/src/signature.tstypescript2/app-stdlib-matrix/src/types.ts
💤 Files with no reviewable changes (1)
- tools/stdlib-matrix/baml_src/pools.baml
Binary size checks passed✅ 7 passed
Generated by |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
baml describegets a--searchflag which will also fuzzy-search doc commentsSummary by CodeRabbit
baml describe, with ranked results, limits, text/JSON output, and suggestions.