stdlib similarity matrix - #4337
Merged
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
📝 WalkthroughWalkthroughThis PR adds a BAML–TypeScript standard-library matrix pipeline. It extracts symbols, aligns them with model-assisted passes, reuses prior judgements, renders reports, publishes them to GitHub Pages, and provides an interactive Lit-based web application. ChangesExported surface contract
Stdlib matrix pipeline
Publishing workflow and web application
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant Release as Release or manual dispatch
participant Workflow as stdlib-matrix workflow
participant Extractor as Surface extractors
participant Matrix as Matrix builder
participant Aligner as Alignment passes
participant App as Matrix web application
participant Pages as GitHub Pages
Release->>Workflow: Trigger matrix workflow
Workflow->>Extractor: Extract BAML and TypeScript surfaces
Extractor-->>Matrix: Return versioned symbol inputs
Matrix->>Aligner: Submit proposal, sweep, and verification requests
Aligner-->>Matrix: Return judgements and failures
Matrix-->>Workflow: Produce matrix.json and matrix.md
Workflow->>Pages: Build and deploy report site
App->>Pages: Load matrix.json
Pages-->>App: Return matrix report
App-->>App: Render grouped symbols and navigation
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 |
2kai2kai2
enabled auto-merge
August 6, 2026 21:24
![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: 20
🧹 Nitpick comments (9)
typescript2/app-stdlib-matrix/src/components/matrix-symbol.ts (3)
58-65: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
SWATCH.unjudgedTsis never returned.The
swatchgetter returnsSWATCH.tsfor the TypeScript side without a further test at Line 167, and the comment above it explains that the TypeScript side has no unjudged state. TheunjudgedTsentry is therefore unreachable. Remove it, and remove the matching CSS class ifsrc/index.cssdefinesswatch-unjudged-tsonly for this entry.🤖 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 `@typescript2/app-stdlib-matrix/src/components/matrix-symbol.ts` around lines 58 - 65, Remove the unreachable SWATCH.unjudgedTs entry from the SWATCH constant, and delete the matching swatch-unjudged-ts CSS class if it exists solely for this entry. Preserve the existing TypeScript swatch behavior in the swatch getter.
34-49: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winThe four-state swatch doc comment is attached to the wrong declaration.
The block comment at Lines 34-43 describes the swatch states, but the declaration that follows it is
SECTION, which has its own doc comment at Line 44. The swatch text therefore documents nothing. Move it aboveSWATCHat Line 58, or above theswatchgetter at Line 155.🤖 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 `@typescript2/app-stdlib-matrix/src/components/matrix-symbol.ts` around lines 34 - 49, Move the four-state swatch documentation comment from above SECTION to the SWATCH declaration or the swatch getter, so it documents the swatch states rather than the section-heading helper. Keep SECTION associated only with its existing heading comment.
341-349: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winReplace the linear counterpart lookup with an id index.
Line 347 scans the whole
matrix.tsormatrix.bamlarray for each counterpart. This runs on every render of every open row, so the cost grows with the number of open rows multiplied by the surface size.SymbolIndexis already memoized per report andtypes.tsalready builds id-to-index maps, so an exposed id-to-index lookup removes the scan.Add an
indexOfId(side, id)accessor toSymbolIndex, then use it here in place offindIndex.🤖 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 `@typescript2/app-stdlib-matrix/src/components/matrix-symbol.ts` around lines 341 - 349, Expose an indexOfId(side, id) accessor on the memoized SymbolIndex, reusing the existing id-to-index maps built in types.ts. Update matrix-symbol.ts counterpart to obtain the counterpart index through SymbolIndex.indexOfId instead of scanning the selected symbols array with findIndex, while preserving the existing missing-symbol nothing behavior.tools/stdlib-matrix/baml_src/report.baml (1)
308-319: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider removing the duplicated defaults and guard.
build_matrix_v2repeats the default data paths and thebaml_surface_sha256guard thatbuild_matrixintools/stdlib-matrix/baml_src/entry.bamlalready declares. If a path changes, the two entry points can diverge. Extract the defaults and the guard into one helper, or letbuild_matrix_v2delegate to a shared function.🤖 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/report.baml` around lines 308 - 319, Refactor build_matrix_v2 to reuse the existing defaults and baml_surface_sha256 validation from build_matrix instead of duplicating them. Extract the shared setup into a helper or delegate both entry points through one shared function, preserving the required-hash guard and ensuring both entry points use the same data-path defaults.typescript2/app-stdlib-matrix/src/types.ts (1)
243-262: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider making
buildGroupsreject a side/array mismatch at compile time.
sideandsymbolsare independent parameters, and the body casts on the value ofside(Lines 256, 268). A call that passes'ts'withmatrix.bamltype-checks and then misgroups every row. Function overloads, or a single discriminated parameter object, would make the pairing checkable.♻️ Proposed signature
+export function buildGroups( + side: 'baml', + symbols: BamlSymbol[], +): Array<[string, TreeNode[]]>; +export function buildGroups( + side: 'ts', + symbols: TsSymbol[], +): Array<[string, TreeNode[]]>; export function buildGroups( side: Side, symbols: Array<BamlSymbol | TsSymbol>, ): Array<[string, TreeNode[]]> {🤖 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 `@typescript2/app-stdlib-matrix/src/types.ts` around lines 243 - 262, Update buildGroups so the side discriminator is type-linked to the symbols array, preventing 'ts' from accepting BamlSymbol[] and vice versa. Use overloads or a discriminated parameter object while preserving the existing grouping behavior and return type for valid pairings.tools/stdlib-matrix/baml_src/pools.baml (2)
242-247: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider tying
ts_keystots_symbolsat the call boundary.
build_pool_with_keyspairsts_keystots_symbolsby position at Line 263. A caller that passes keys computed from a different array produces silently wrong pools, becauseat(index) ?? ""hides the mismatch. Add a length check, or pass pre-zipped pairs.🤖 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/pools.baml` around lines 242 - 247, Update build_pool_with_keys so ts_keys and ts_symbols cannot be mismatched: validate that both arrays have equal lengths at the call boundary and reject mismatches before positional pairing, or change the API to accept pre-zipped symbol/key pairs. Do not rely on the existing at(index) fallback to conceal invalid input.
262-283: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winStop the member-key scan after the first shared stem.
The loop at Line 269 keeps iterating over every member key after
sharesbecomes true. The file documents that this comparison is the hot path, so the wasted iterations matter for the 183-owner pass.♻️ Proposed change
let shares = false; for (let member_key in member_keys) { - if (!shares && stems_share(member_key, key)) { + if (stems_share(member_key, key)) { shares = true; + break; } }Confirm that
breakis available in BAMLforloops; if it is not, keep the guard as written.🤖 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/pools.baml` around lines 262 - 283, Update the inner member_keys loop in the symbol-processing flow to stop immediately after stems_share(member_key, key) sets shares to true by adding a break if BAML for loops support it; otherwise preserve the existing guarded scan.tools/stdlib-matrix/baml_src/align.baml (1)
835-848: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCount rulings that match no pairing.
apply_rulingsindexes every ruling bybaml\u{1f}ts, then only consults the index while walkingmatrix.judgements. A ruling that names a pair the matrix does not hold is dropped silently.AlignOutcome.rejectedis returned empty at Line 899, so verify-pass drift produces no count, unlikeapply_proposalsandapply_sweep. The file header states that an inventing prompt shows up as a count.Track the keys that were consumed and report the rest.
♻️ Proposed change
let upheld = 0; let refuted = 0; + let used: map<string, bool> = {}; let kept: Judgement[] = []; for (let judgement in matrix.judgements) { let key = `${judgement.baml}\u{1f}${judgement.ts ?? ""}`; match (rulings.get(key)) { null => { kept.push(judgement); }, let ruling: Ruling => { + let _ = used.set(key, true);Then build the returned
rejectedlist from the ruling keys absent fromused, and pass it in place of[]at Line 899.🤖 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 835 - 848, Update apply_rulings to track ruling keys consumed while processing matrix.judgements, then build AlignOutcome.rejected from indexed ruling keys absent from that used set. Replace the empty rejected list returned near the function’s outcome construction, preserving existing failure and matched-ruling handling.tools/stdlib-matrix/baml_src/symbols.baml (1)
350-372: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winThe required-method and default-method distinction is dropped.
Line 353 concatenates
item.methods,item.required_methods, anditem.default_methodsinto one loop. Every entry is emitted withorigin: origin_of_method(...), which yields onlymethodorstatic_method. A consumer cannot then tell a required interface method from an inherited default or from a class's inherent method.The export layer preserves this distinction deliberately:
export.rskeepsrequired_methodsanddefault_methodsas separate fields, andFunctionExport.from_defaultflags an inherited default.models.bamldecodesExportImplMethod.from_defaultat Line 124, but no provided file reads it. Consider extending theoriginvocabulary, or recordingfrom_defaultonBamlSymbol, so the report keeps the facet the export supplies.🤖 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/symbols.baml` around lines 350 - 372, Preserve required/default method provenance in the symbol generation loop around methods, required_methods, and default_methods instead of emitting all entries through origin_of_method alone. Use the export metadata, including ExportImplMethod.from_default, to distinguish inherent, required, and inherited default methods in BamlSymbol while retaining existing method origin behavior.
🤖 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 61-64: Add the conditional ref configuration to the Checkout step,
using github.event.workflow_run.head_sha for workflow_run events and github.sha
otherwise, while preserving persist-credentials: false.
In `@baml_language/crates/baml_surface/src/export_tests.rs`:
- Around line 51-57: Update the census key list in the export-tests uniqueness
check to include "default_methods" alongside the existing interface member keys.
Preserve the invariant that every exported id is counted exactly once, including
ids from ItemDetail::Interface default methods.
In `@tools/stdlib-matrix/baml_src/ratchet.baml`:
- Around line 94-100: Update compare_to_baseline so body_differs reflects
changes beyond count totals, including judgement relations, reasons, or
divergences; compare the complete report body against the baseline while
excluding provenance and other input metadata. Preserve the existing advisory
behavior for identical inputs whose report contents changed.
In `@tools/stdlib-matrix/baml_src/surface.baml`:
- Around line 45-62: Update normalize_ty_display so nominal BAML type heads are
normalized within composite displays, including arrays, optionals, and nested
map key/value types, rather than only when the entire display matches a known
name. Preserve unknown structure and existing generic formatting, and ensure
names such as baml.String and baml.Uint8Array normalize wherever they appear.
In `@tools/stdlib-matrix/baml_src/symbols.baml`:
- Around line 180-182: Preserve parameter declaration order by retaining and
iterating the ordered sig.params collection in the rendering logic, using kwargs
only to look up each parameter’s value; update both
tools/stdlib-matrix/baml_src/symbols.baml:180-182 and
tools/stdlib-matrix/baml_src/references.baml:231-233 accordingly so optional
parameters and referenced text follow the original signature order.
In `@tools/stdlib-matrix/baml_src/ts_symbols.baml`:
- Around line 36-46: Update ts_origin_of to test ts_is_callable(member) before
member.static so static non-callable members are classified as property while
static callable members remain static_method. Then adjust ts_id_of to receive
member.static and determine prototype addressing from staticness rather than
origin, preserving correct IDs for static properties.
In `@tools/stdlib-matrix/extractors/ts-surface.mjs`:
- Around line 31-35: Remove the redundant "PromiseConstructor" entry from
ECMA_CONTAINERS; keep "Promise" so walkLibFile continues folding the constructor
twin into the Promise container and scope.ecma reports only containers that are
actually compared.
- Around line 74-76: Validate the --repo-root option in the argument parsing
before calling path.resolve: when --repo-root is present without a following
value, report a clear usage error and terminate instead of passing undefined to
path.resolve. Preserve the default "." root behavior when the flag is absent,
using the existing CLI entrypoint’s error-handling convention.
- Around line 152-172: Ensure member metadata records the earliest introducing
lib rather than the first lexicographically visited file. Update addMember to
revise entry.since when a later declaration has an earlier lib rank, and add the
necessary lib-version ranking near sinceOf; preserve the existing
declaration-merging and signature behavior.
- Around line 253-258: Update the declaration-file discovery loop around
walkNodeModuleFile to record each expected `@types/node` file that is missing, and
expose those skipped paths in the generated document scope. Preserve processing
for existing files, and ensure consumers can distinguish skipped declarations
from declarations that were read but matched nothing.
- Around line 272-285: Replace the locale-dependent localeCompare comparators in
the container and member sorts within the document construction flow with
locale-independent string ordering, consistent with the existing bare sort
calls. Preserve the secondary static-member ordering and all other output fields
unchanged.
In `@tools/stdlib-matrix/README.md`:
- Around line 41-44: Update the --check description in the README to state that
it refreshes the data/ directory for a later --skip-extract build while
comparing only report inputs, including the stdlib content hash and TypeScript
release; remove the claim that it writes nothing and retain the existing
exit-status behavior.
In `@tools/stdlib-matrix/run`:
- Around line 219-231: Add combination validation near the existing option
checks to reject --check when combined with --llm or --concurrency, before the
target-selection logic. Ensure the validation exits with a clear error
consistent with other unusable combinations, while leaving independent --check
behavior unchanged.
- Around line 245-251: Keep the raw-result artifact-writing block involving
raw_result, mkdir, and cp inside the non-check else branch so --check continues
writing nothing, while preserving the existing artifact path and remaining
check-mode behavior.
In `@typescript2/app-stdlib-matrix/package.json`:
- Around line 7-20: Update the typecheck script in package.json to remove the
incompatible --noEmit flag from the tsc -b command. Configure noEmit in the
referenced tsconfig.json files so project-reference typechecking remains
emit-free while preserving the existing build script.
In `@typescript2/app-stdlib-matrix/README.md`:
- Around line 10-13: Restrict the src handling in
MatrixAppElement.connectedCallback() and `#load`() to relative same-origin report
paths before calling fetch(), rejecting absolute URLs and external origins while
preserving the default ./matrix.json and named report-file cases. Update the
README usage text to document that ?src= accepts only relative same-origin
paths.
In `@typescript2/app-stdlib-matrix/src/components/matrix-app.ts`:
- Line 84: Update disconnectedCallback to remove the GOTO_EVENT listener
registered by connectedCallback, using the same this.#onGoto reference and
EventListener cast as the addEventListener call. Ensure reconnecting the element
does not accumulate handlers while preserving the existing window-listener
cleanup.
In `@typescript2/app-stdlib-matrix/src/index.css`:
- Line 5: Update the Stylelint configuration for index.css so Tailwind v4
at-rules such as `@theme` and `@utility` are recognized without lint errors. Either
configure the file to use CSS syntax or extend scss/at-rule-no-unknown with the
required Tailwind directives, including theme, utility, source, apply, variant,
and custom-variant.
In `@typescript2/app-stdlib-matrix/src/signature.ts`:
- Around line 162-181: Update typeName’s generic-argument parsing so it does not
split on commas nested inside angle brackets. Track angle-bracket depth while
scanning the substring between the outer brackets, splitting only when depth is
zero, and preserve the existing rendering for each top-level argument.
- Around line 82-104: Update activate so the modifier-key check occurs before
event.preventDefault(): unmodified anchor activation must preserve the href
fragment navigation for keyboard Enter while modified clicks still prevent
default, stop propagation, and call dispatchGoto. Keep the existing row-click
behavior outside the anchor unchanged.
---
Nitpick comments:
In `@tools/stdlib-matrix/baml_src/align.baml`:
- Around line 835-848: Update apply_rulings to track ruling keys consumed while
processing matrix.judgements, then build AlignOutcome.rejected from indexed
ruling keys absent from that used set. Replace the empty rejected list returned
near the function’s outcome construction, preserving existing failure and
matched-ruling handling.
In `@tools/stdlib-matrix/baml_src/pools.baml`:
- Around line 242-247: Update build_pool_with_keys so ts_keys and ts_symbols
cannot be mismatched: validate that both arrays have equal lengths at the call
boundary and reject mismatches before positional pairing, or change the API to
accept pre-zipped symbol/key pairs. Do not rely on the existing at(index)
fallback to conceal invalid input.
- Around line 262-283: Update the inner member_keys loop in the
symbol-processing flow to stop immediately after stems_share(member_key, key)
sets shares to true by adding a break if BAML for loops support it; otherwise
preserve the existing guarded scan.
In `@tools/stdlib-matrix/baml_src/report.baml`:
- Around line 308-319: Refactor build_matrix_v2 to reuse the existing defaults
and baml_surface_sha256 validation from build_matrix instead of duplicating
them. Extract the shared setup into a helper or delegate both entry points
through one shared function, preserving the required-hash guard and ensuring
both entry points use the same data-path defaults.
In `@tools/stdlib-matrix/baml_src/symbols.baml`:
- Around line 350-372: Preserve required/default method provenance in the symbol
generation loop around methods, required_methods, and default_methods instead of
emitting all entries through origin_of_method alone. Use the export metadata,
including ExportImplMethod.from_default, to distinguish inherent, required, and
inherited default methods in BamlSymbol while retaining existing method origin
behavior.
In `@typescript2/app-stdlib-matrix/src/components/matrix-symbol.ts`:
- Around line 58-65: Remove the unreachable SWATCH.unjudgedTs entry from the
SWATCH constant, and delete the matching swatch-unjudged-ts CSS class if it
exists solely for this entry. Preserve the existing TypeScript swatch behavior
in the swatch getter.
- Around line 34-49: Move the four-state swatch documentation comment from above
SECTION to the SWATCH declaration or the swatch getter, so it documents the
swatch states rather than the section-heading helper. Keep SECTION associated
only with its existing heading comment.
- Around line 341-349: Expose an indexOfId(side, id) accessor on the memoized
SymbolIndex, reusing the existing id-to-index maps built in types.ts. Update
matrix-symbol.ts counterpart to obtain the counterpart index through
SymbolIndex.indexOfId instead of scanning the selected symbols array with
findIndex, while preserving the existing missing-symbol nothing behavior.
In `@typescript2/app-stdlib-matrix/src/types.ts`:
- Around line 243-262: Update buildGroups so the side discriminator is
type-linked to the symbols array, preventing 'ts' from accepting BamlSymbol[]
and vice versa. Use overloads or a discriminated parameter object while
preserving the existing grouping behavior and return type for valid pairings.
🪄 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: 47ea9850-1313-4837-bbed-9fbc1d12a3ca
⛔ Files ignored due to path filters (2)
baml_language/crates/baml_surface/src/snapshots/baml_surface__export_tests__assert_package_exports_fully.snapis excluded by!**/*.snaptypescript2/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (40)
.baml/.gitignore.github/workflows/stdlib-matrix.yml.gitignorebaml_language/crates/baml_surface/src/export.rsbaml_language/crates/baml_surface/src/export_tests.rsbaml_language/crates/baml_surface/src/facts.rsbaml_language/crates/baml_surface/src/handles.rstools/stdlib-matrix/.gitignoretools/stdlib-matrix/README.mdtools/stdlib-matrix/baml.tomltools/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/ratchet.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/symbols.bamltools/stdlib-matrix/baml_src/tests.bamltools/stdlib-matrix/baml_src/ts_symbols.bamltools/stdlib-matrix/extractors/ts-surface.mjstools/stdlib-matrix/runtypescript2/app-stdlib-matrix/.gitignoretypescript2/app-stdlib-matrix/README.mdtypescript2/app-stdlib-matrix/index.htmltypescript2/app-stdlib-matrix/package.jsontypescript2/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/index.csstypescript2/app-stdlib-matrix/src/main.tstypescript2/app-stdlib-matrix/src/navigation.tstypescript2/app-stdlib-matrix/src/signature.tstypescript2/app-stdlib-matrix/src/types.tstypescript2/app-stdlib-matrix/tsconfig.jsontypescript2/app-stdlib-matrix/vite.config.ts
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@typescript2/app-stdlib-matrix/src/components/matrix-app.ts`:
- Around line 64-67: Update the URL resolution logic used by reportSource to
catch malformed requested values passed to new URL, including invalid src query
parameters. Return "./matrix.json" for parsing failures so connectedCallback can
continue to the existing fetch failure handling.
🪄 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: 4c8735e3-c627-4949-8b59-e75655ceae3e
📒 Files selected for processing (13)
.github/workflows/stdlib-matrix.ymlbaml_language/crates/baml_surface/src/export_tests.rstools/stdlib-matrix/README.mdtools/stdlib-matrix/baml_src/models.bamltools/stdlib-matrix/baml_src/ratchet.bamltools/stdlib-matrix/baml_src/surface.bamltools/stdlib-matrix/baml_src/tests.bamltools/stdlib-matrix/baml_src/ts_symbols.bamltools/stdlib-matrix/extractors/ts-surface.mjstools/stdlib-matrix/runtypescript2/app-stdlib-matrix/README.mdtypescript2/app-stdlib-matrix/src/components/matrix-app.tstypescript2/app-stdlib-matrix/src/signature.ts
🚧 Files skipped from review as they are similar to previous changes (10)
- typescript2/app-stdlib-matrix/README.md
- tools/stdlib-matrix/run
- tools/stdlib-matrix/baml_src/models.baml
- tools/stdlib-matrix/README.md
- tools/stdlib-matrix/baml_src/ts_symbols.baml
- baml_language/crates/baml_surface/src/export_tests.rs
- tools/stdlib-matrix/baml_src/ratchet.baml
- tools/stdlib-matrix/baml_src/surface.baml
- .github/workflows/stdlib-matrix.yml
- tools/stdlib-matrix/baml_src/tests.baml
2kai2kai2
disabled auto-merge
August 6, 2026 22:32
2kai2kai2
enabled auto-merge
August 6, 2026 22:33
⏭️ 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):
|
2kai2kai2
force-pushed
the
kai/stdlib-similarity-matrix
branch
from
August 6, 2026 23:15
d084258 to
dcba401
Compare
Contributor
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
Contributor
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
typescript2/app-stdlib-matrix/src/navigation.ts (1)
123-136: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueExtract the
Refequality check into one helper.
registerandnameOfboth compare two refs bygrouppluspath.join(). Two copies of the same rule can drift. A single helper keeps the identity rule in one place.♻️ Proposed refactor
+function samePlace(a: Ref, b: Ref): boolean { + return a.group === b.group && a.path.join() === b.path.join(); +} + function register(names: NameMap, key: string, ref: Ref) { const existing = names.get(key); if (existing === undefined) { names.set(key, ref); return; } if (existing === null) return; - if ( - existing.group !== ref.group || - existing.path.join() !== ref.path.join() - ) { - names.set(key, null); - } + if (!samePlace(existing, ref)) names.set(key, null); }const resolved = this.resolve(side, dotted); - return resolved && - resolved.group === place.group && - resolved.path.join() === place.path.join() - ? dotted - : null; + return resolved && samePlace(resolved, place) ? dotted : null;Also applies to: 301-311
🤖 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 `@typescript2/app-stdlib-matrix/src/navigation.ts` around lines 123 - 136, Extract the shared Ref identity comparison used by register and nameOf into a single helper that compares group and path.join(). Replace both inline comparisons with this helper, preserving the existing name registration and lookup behavior.
🤖 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.
Nitpick comments:
In `@typescript2/app-stdlib-matrix/src/navigation.ts`:
- Around line 123-136: Extract the shared Ref identity comparison used by
register and nameOf into a single helper that compares group and path.join().
Replace both inline comparisons with this helper, preserving the existing name
registration and lookup behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 83f583be-b88a-428f-a033-2409a1a5e7b2
⛔ Files ignored due to path filters (2)
baml_language/crates/baml_surface/src/snapshots/baml_surface__export_tests__assert_package_exports_fully.snapis excluded by!**/*.snaptypescript2/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (40)
.baml/.gitignore.github/workflows/stdlib-matrix.yml.gitignorebaml_language/crates/baml_surface/src/export.rsbaml_language/crates/baml_surface/src/export_tests.rsbaml_language/crates/baml_surface/src/facts.rsbaml_language/crates/baml_surface/src/handles.rstools/stdlib-matrix/.gitignoretools/stdlib-matrix/README.mdtools/stdlib-matrix/baml.tomltools/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/ratchet.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/symbols.bamltools/stdlib-matrix/baml_src/tests.bamltools/stdlib-matrix/baml_src/ts_symbols.bamltools/stdlib-matrix/extractors/ts-surface.mjstools/stdlib-matrix/runtypescript2/app-stdlib-matrix/.gitignoretypescript2/app-stdlib-matrix/README.mdtypescript2/app-stdlib-matrix/index.htmltypescript2/app-stdlib-matrix/package.jsontypescript2/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/index.csstypescript2/app-stdlib-matrix/src/main.tstypescript2/app-stdlib-matrix/src/navigation.tstypescript2/app-stdlib-matrix/src/signature.tstypescript2/app-stdlib-matrix/src/types.tstypescript2/app-stdlib-matrix/tsconfig.jsontypescript2/app-stdlib-matrix/vite.config.ts
🚧 Files skipped from review as they are similar to previous changes (35)
- tools/stdlib-matrix/extractors/ts-surface.mjs
- typescript2/app-stdlib-matrix/README.md
- tools/stdlib-matrix/baml.toml
- typescript2/app-stdlib-matrix/.gitignore
- baml_language/crates/baml_surface/src/handles.rs
- tools/stdlib-matrix/baml_src/render.baml
- typescript2/app-stdlib-matrix/src/main.ts
- baml_language/crates/baml_surface/src/facts.rs
- baml_language/crates/baml_surface/src/export_tests.rs
- typescript2/app-stdlib-matrix/vite.config.ts
- typescript2/app-stdlib-matrix/tsconfig.json
- typescript2/app-stdlib-matrix/src/components/matrix-app.ts
- .gitignore
- tools/stdlib-matrix/baml_src/models.baml
- typescript2/app-stdlib-matrix/package.json
- tools/stdlib-matrix/baml_src/ratchet.baml
- typescript2/app-stdlib-matrix/src/components/matrix-group.ts
- baml_language/crates/baml_surface/src/export.rs
- typescript2/app-stdlib-matrix/src/types.ts
- tools/stdlib-matrix/run
- tools/stdlib-matrix/baml_src/revision.baml
- tools/stdlib-matrix/baml_src/ingest.baml
- typescript2/app-stdlib-matrix/src/signature.ts
- typescript2/app-stdlib-matrix/index.html
- tools/stdlib-matrix/baml_src/align.baml
- tools/stdlib-matrix/baml_src/references.baml
- .baml/.gitignore
- typescript2/app-stdlib-matrix/src/components/matrix-symbol.ts
- tools/stdlib-matrix/baml_src/symbols.baml
- tools/stdlib-matrix/baml_src/surface.baml
- tools/stdlib-matrix/baml_src/pools.baml
- tools/stdlib-matrix/.gitignore
- tools/stdlib-matrix/baml_src/tests.baml
- tools/stdlib-matrix/baml_src/entry.baml
- tools/stdlib-matrix/baml_src/ts_symbols.baml
Binary size checks passed✅ 7 passed
Generated by |
This was referenced Aug 7, 2026
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.
Compares the
bamlstandard library with typescript and creates a comparison table.Also serves as an initial human-readable web reference for the
bamlpackage.Summary by CodeRabbit