Consolidate TypeScript and JavaScript parser resolution

[Iron-Ham]

Summary

• consolidate the open TypeScript/JavaScript parser, extraction, accessor, namespace, abstract entity, and import-resolution fixes
• keep the combined graph/scope behavior from the parent PRs in one reviewable branch

Supersedes #303, #305, #291, #289, #288, #285.
Fixes #264.
Fixes #262.
Fixes #265.
Closes #267.
Fixes #266.
Fixes #263.

Test plan

• cargo test --manifest-path crates/Cargo.toml -p sem-core typescript
• cargo test --manifest-path crates/Cargo.toml -p sem-core js_ts
• cargo test --manifest-path crates/Cargo.toml -p sem-core import_resolution
• cargo test --manifest-path crates/Cargo.toml -p sem-core scope_resolve_typescript
• cargo test --manifest-path crates/Cargo.toml -p sem-cli --test accessor_cli
• cargo check --manifest-path crates/Cargo.toml -p sem-core -p sem-cli
• cargo test --manifest-path crates/Cargo.toml -p sem-core graph

[inspect-review]

inspect review

Triage: 174 entities analyzed | 0 critical, 0 high, 57 medium, 117 low
Verdict: standard_review

Findings (5)

1. [low] In resolve_with_scopes_full, the destructuring of pre_built is missing owner_members but the else branch creates it. This will cause a compilation error because the tuple sizes don't match.
2. [low] In resolve_ref, the function signature adds owner_members parameter but the BEFORE code shows it's being called without this parameter in multiple places. All call sites need to be updated or this will cause compilation errors.
3. [low] In extract_ts_import, the function signature adds ts_default_exports parameter but existing call sites throughout the codebase won't have this parameter, causing compilation errors.
4. [low] In resolve_ref, the function now calls is_local_binding_in_scopes_at_byte instead of is_local_binding_in_scopes, but this new function doesn't exist in the diff. This will cause a compilation error.
5. [low] In resolve_ref, the function changes from is_local_binding_in_scopes to is_local_binding_in_scopes_at_byte which adds a byte parameter. If is_local_binding_in_scopes_at_byte doesn't exist or has a different signature, this will cause a compilation error.

---

Reviewed by inspect | Entity-level triage found 0 high-risk changes

[inspect-review]

inspect review

Triage: 174 entities analyzed | 0 critical, 0 high, 57 medium, 117 low
Verdict: standard_review

Findings (7)

1. [low] In resolve_with_scopes_full, the destructuring of pre_built is missing owner_members but the else branch creates it. This will cause a compilation error because the tuple sizes don't match.
2. [low] In resolve_ref, the function signature adds owner_members parameter but the function is called from multiple places. If callers weren't updated to pass this new parameter, this will cause compilation errors or runtime issues.
3. [low] In extract_ts_import, the function signature adds ts_default_exports parameter but the function is called from other places. If callers weren't updated to pass this new parameter, this will cause compilation errors.
4. [low] In resolve_ref, the function now calls is_local_binding_in_scopes_at_byte instead of is_local_binding_in_scopes, passing ast_ref.byte as a new parameter. If is_local_binding_in_scopes_at_byte doesn't exist or has a different signature, this will cause a compilation error.
5. [low] In resolve_with_scopes_full, the destructuring pattern expects 5 values but only 4 are provided when pre_built is None. The AFTER code adds owner_members to the destructuring (5 variables) but the else branch that builds from scratch only initializes 4 variables (symbol_table, class_members, entity_ranges, go_pkg_index), missing owner_members.
6. [low] In resolve_ref, the function signature adds a new parameter owner_members but all call sites throughout the codebase would need to be updated to pass this parameter. The diff shows the signature change but doesn't show updates to call sites, which would cause compilation errors.
7. [low] In extract_ts_import, the function signature adds a new parameter ts_default_exports but all call sites would need to be updated. The diff shows the signature change but doesn't show corresponding updates to where this function is called.

---

Reviewed by inspect | Entity-level triage found 0 high-risk changes

[rs545837]

Performance note on the strip_children_content rewrite.

[rs545837]

content[search_start..search_end].find(&child.content) is a naive substring search — O(n×m) where n is the search window and m is the child content length. For large parent entities with many children (e.g. a 500-line class with 30 methods), this adds up.

Since we already computed line_starts and know the child's start line relative to the parent (start_idx), we could jump directly to the expected byte offset:

let expected_offset = line_starts.get(start_idx).copied().unwrap_or(0);
if content[expected_offset..].starts_with(&child.content) {
    excluded_ranges.push((expected_offset, expected_offset + child.content.len()));
    continue;
}
// fall back to find() only if the direct check fails

This makes the common case O(m) instead of O(n×m).

[Iron-Ham]

Addressed in 1819d73. strip_children_content now first checks the expected line-start offset with starts_with, and only falls back to the bounded .find() search if that direct match fails.

[inspect-review]

inspect review

Triage: 174 entities analyzed | 0 critical, 0 high, 57 medium, 117 low
Verdict: standard_review

Findings (7)

1. [low] In resolve_with_scopes_full, the destructuring of pre_built expects 5 fields but only 4 are provided in the tuple. The BEFORE code has 4 fields (symbol_table, class_members, entity_ranges, go_pkg_index) but AFTER expects 5 fields (symbol_table, class_members, owner_members, entity_ranges, go_pkg_index). This will cause a compilation error or runtime panic.
2. [low] In resolve_ref, the function signature adds a new parameter owner_members but the function is called from multiple places. If any call site wasn't updated to pass this new parameter, it will cause a compilation error. The diff shows the signature change but doesn't show all call sites being updated.
3. [low] In extract_ts_import, the function signature adds a new parameter ts_default_exports but the function is called from other places. If any call site wasn't updated to pass this new parameter, it will cause a compilation error.
4. [low] In resolve_ref, the function call changed from is_local_binding_in_scopes(scope_idx, scopes, name) to is_local_binding_in_scopes_at_byte(scope_idx, scopes, name, ast_ref.byte). This is a breaking change - if is_local_binding_in_scopes_at_byte doesn't exist or has different semantics than is_local_binding_in_scopes, this will cause a compilation error or incorrect behavior.
5. [low] In resolve_with_scopes_full, the destructuring pattern expects 5 values but only 4 are provided when pre_built is None. The code tries to destructure (symbol_table, class_members, owner_members, entity_ranges, go_pkg_index) but the else branch only builds 4 values without owner_members.
6. [low] In extract_ts_import, a new parameter ts_default_exports is added to the function signature, but all call sites in the codebase would need to be updated to pass this parameter. This is a breaking change that will cause compilation errors at all call sites.
7. [low] In resolve_ref, a new parameter owner_members is added between existing parameters, but this is a breaking change. All call sites would need to be updated to pass this new parameter in the correct position.

---

Reviewed by inspect | Entity-level triage found 0 high-risk changes

[inspect-review]

inspect review

Triage: 181 entities analyzed | 0 critical, 0 high, 60 medium, 121 low
Verdict: standard_review

Findings (6)

1. [low] In resolve_with_scopes_full, the destructuring of pre_built is missing owner_members in the tuple pattern, causing a compile error. The BEFORE code destructures 4 fields (pb.symbol_table, pb.class_members, pb.entity_ranges, pb.go_pkg_index) but the AFTER code destructures 5 fields (pb.symbol_table, pb.class_members, pb.owner_members, pb.entity_ranges, pb.go_pkg_index). However, the else branch still builds only 4 fields, creating a type mismatch.
2. [low] In resolve_ref, the function signature adds a new parameter owner_members but the function is called from multiple places. The diff shows the signature changed from 11 parameters to 12 parameters, but we don't see corresponding updates to all call sites in the diff, which would cause compilation errors at those call sites.
3. [low] In extract_ts_import, the function signature adds two new parameters ts_default_exports and top_level_entities, but the diff doesn't show updates to all call sites. This function is likely called from extract_imports_from_ast or similar functions, and those call sites would fail to compile without the new arguments.
4. [low] In resolve_ref, the function now calls is_local_binding_in_scopes_at_byte(scope_idx, scopes, name, ast_ref.byte) instead of is_local_binding_in_scopes(scope_idx, scopes, name). However, the diff doesn't show the definition of is_local_binding_in_scopes_at_byte, and if this function doesn't exist or has a different signature, this will cause a compilation error.
5. [low] In resolve_ref, the function signature adds owner_members parameter but the BEFORE code shows it's being called without this parameter in multiple places, which will cause compilation errors at all call sites.
6. [low] The function is_local_binding_in_scopes is renamed to is_local_binding_in_scopes_at_byte with an additional byte parameter, but this will break all existing call sites that use the old function name.

---

Reviewed by inspect | Entity-level triage found 0 high-risk changes

[Iron-Ham]

Pushed a runtime performance follow-up in c10963f.

What changed:

• default-export candidate files are sorted once and reused during TS/JS import resolution
• namespace imports lazily build the top-level entity index only when a namespace import needs it
• namespace imports now resolve one module candidate through the shared import-order helper before registering exports, which addresses the reviewer concern about merging exports from multiple module variants
• added coverage for ordered module variants and single-candidate namespace behavior

Validation:

• cargo test --manifest-path crates/Cargo.toml -p sem-core test_js_ts_namespace_import
• cargo test --manifest-path crates/Cargo.toml -p sem-core test_js_ts_default_import
• cargo test --manifest-path crates/Cargo.toml -p sem-core import_resolution
• cargo check --manifest-path crates/Cargo.toml -p sem-core
• git diff --no-ext-diff --check

[rs545837]

This has merge conflicts after #311 and #312 landed on main. Could you rebase when you get a chance?

[inspect-review]

inspect review

Triage: 179 entities analyzed | 0 critical, 0 high, 60 medium, 119 low
Verdict: standard_review

Findings (7)

1. [low] In resolve_ref function, the new parameter owner_members is added but the function signature in extract_ts_import doesn't match - extract_ts_import passes ts_default_exports and top_level_entities but resolve_ref expects owner_members in that position, causing a parameter mismatch.
2. [low] Function is_local_binding_in_scopes_at_byte is called in resolve_ref but this function name doesn't exist in the codebase - the original function was is_local_binding_in_scopes. This will cause a compilation error.
3. [low] In build_ts_default_export_table, the function calls default_export_names_from_content which uses regex patterns that may incorrectly match export statements inside comments or strings, leading to false positive default export detection.
4. [low] In default_export_names_from_content, the regex DEFAULT_SPECIFIER_RE checks if the next content starts with 'from ' to skip re-exports, but this check happens after capturing the match. If there's whitespace or comments between the closing brace and 'from', the check will fail and incorrectly treat a re-export as a local export.
5. [low] In build_import_table, the regex JS_DEFAULT_RE will incorrectly match and capture the default import even when it's followed by named imports in braces (e.g., import Foo, { bar } from './module'), but the regex doesn't account for the comma and named imports, potentially causing incorrect parsing.
6. [low] In resolve_default_export_target, if multiple files have the same default export name, the function returns the first match from sorted_files without considering that find_import_file might return a different file than what's in the default exports table, causing a potential mismatch.
7. [low] In build_export_alias_edges, the function filters for entities with entity_type == "export" and looks them up in import_table using (file_path, name) as key. However, the import_table is built for imports, not exports, so this lookup will likely fail for most export entities, making the function ineffective.

---

Reviewed by inspect | Entity-level triage found 0 high-risk changes

[inspect-review]

inspect review

Triage: 184 entities analyzed | 0 critical, 0 high, 60 medium, 124 low
Verdict: standard_review

Findings (7)

1. [low] In resolve_with_scopes_full, the function signature adds owner_members parameter but the destructuring in the else branch doesn't initialize it, causing a compilation error. The else branch builds symbol_table, class_members, entity_ranges, and go_pkg_index but never creates owner_members, yet the return statement expects 5 values including owner_members.
2. [low] In resolve_ref, the function signature adds owner_members parameter but the function body is truncated in the diff. However, all call sites to resolve_ref would need to be updated to pass this new parameter, and if they weren't updated, this would cause compilation errors.
3. [low] In extract_ts_import, the function signature adds two new parameters ts_default_exports and top_level_entities but the function body is truncated. All call sites to extract_ts_import must be updated to pass these parameters, otherwise compilation will fail.
4. [low] In is_local_binding_in_scopes_at_byte call within resolve_ref, the function name changed from is_local_binding_in_scopes to is_local_binding_in_scopes_at_byte and now takes an additional ast_ref.byte parameter. If this function doesn't exist or has a different signature, this will cause a compilation error.
5. [low] In find_import_file, the function returns Option<&'a str> but the last line is truncated as find(|path| file_stem(path) == source_mod. This appears to be missing the closing parenthesis and the rest of the expression, which would cause a syntax error.
6. [low] In default_export_names_from_content, the regex DEFAULT_SPECIFIER_RE matches export specifiers but the code checks if local_name == "default" to identify default exports. However, this logic is backwards - in export { foo as default }, local_name would be "default" and original_name would be "foo", so the code should push original_name not check if local_name == "default".
7. [low] In build_import_table, the code handles re-exports with if original_name == "default" to resolve default exports, but for the non-default case it calls find_import_target which returns Option<&String> and then calls .cloned() on it. However, the result is assigned to target_id which should be String, not Option<String>, causing a type mismatch in the subsequent if let Some(target_id).

---

Reviewed by inspect | Entity-level triage found 0 high-risk changes

[rs545837]

Hey @Iron-Ham — #306 and #307 have been merged. This one now has merge conflicts. Could you rebase onto main so we can get it merged? Thanks!

[inspect-review]

inspect review

Triage: 209 entities analyzed | 0 critical, 0 high, 83 medium, 126 low
Verdict: standard_review

Findings (7)

1. [low] In resolve_ref function, the parameter is_local_binding_in_scopes_at_byte is called but the function signature shows it was previously is_local_binding_in_scopes. The new function name includes _at_byte and takes an additional ast_ref.byte parameter, but there's no evidence in the diff that this function exists or was added. This will cause a compilation error.
2. [low] In find_import_file function, the return type is Option<&'a str> but the function returns candidate_path.as_str() which has lifetime tied to the local candidates vector, not to the input candidate_file_paths slice. This creates a dangling reference since candidates is dropped at the end of the function scope.
3. [low] In default_export_names_from_content, the function truncates at the first newline when checking only_js_ts_statement_trivia, but the regex DEFAULT_IDENTIFIER_RE can match identifiers that span multiple lines or have complex expressions after them. The line_tail calculation content[name.end()..].split_once('\n').map_or(&content[name.end()..], |(line, _)| line) only checks the first line, which could incorrectly classify multi-line statements.
4. [low] In resolve_with_scopes_full, the destructuring of PreBuiltLookups adds owner_members but the else branch that builds from scratch only declares let mut symbol_table and let mut class_members without declaring owner_members. This will cause a compilation error when trying to use owner_members later in the function.
5. [low] In extract_ts_import, the function signature adds two new parameters ts_default_exports: &TsDefaultExportTable and top_level_entities: &OnceLock<TopLevelEntityIndex>, but there's no evidence in the diff that these parameters are actually used in the function body or that callers have been updated to pass these arguments. This will cause compilation errors at call sites.
6. [low] In resolve_ref function, the new parameter owner_members is added but the function signature in extract_ts_import doesn't match - extract_ts_import now has 2 new parameters (ts_default_exports and top_level_entities) but resolve_ref is called from within scope resolution logic that may not have these parameters available, causing a potential compilation error or missing argument bug.
7. [low] In find_import_file, the fallback path at the end returns file_stem(path) comparison result but tries to return the path itself. The function signature expects Option<&'a str> but candidates.into_iter().find(...) returns an element from the local candidates vector which doesn't have the correct lifetime 'a.

---

Reviewed by inspect | Entity-level triage found 0 high-risk changes

[inspect-review]

inspect review

Triage: 219 entities analyzed | 0 critical, 0 high, 86 medium, 133 low
Verdict: standard_review

Findings (7)

1. [low] In strip_children_content, the function attempts to find child content within a search window but doesn't handle the case where the child content spans multiple lines with different indentation. The search uses search_window.starts_with(&child.content) and search_window.find(&child.content) which require exact string matches, but the child content may have been extracted with different whitespace normalization than what appears in the parent content.
2. [low] In resolve_ref, the function signature adds a new parameter owner_members but the call sites in the diff are not shown. If existing call sites were not updated to pass this new parameter, it will cause compilation errors or runtime issues.
3. [low] In extract_ts_import, the function signature adds two new parameters ts_default_exports and top_level_entities but the diff doesn't show the function body being updated to use these parameters. If the function is called but doesn't use these parameters, it suggests incomplete implementation.
4. [low] In find_import_file, the function returns Option<&'a str> but the code snippet is truncated at find(|path| file_stem(path) == source_mod. This appears to be missing the closing parenthesis and the rest of the expression, which would cause a compilation error.
5. [low] In build_import_table, the code builds owned_content HashMap and then extends content_map with references to it. However, owned_content is a local variable that will be dropped at the end of the function scope, making all the string references in content_map dangling pointers.
6. [low] In resolve_with_scopes_full, the destructuring of PreBuiltLookups adds owner_members but the BEFORE code shows it destructures into 4 variables while AFTER shows 5. The struct definition change is not shown, so if PreBuiltLookups wasn't updated to include owner_members, this will cause a compilation error or field mismatch.
7. [low] In is_local_binding_in_scopes_at_byte, the function name changed from is_local_binding_in_scopes and now takes an additional byte parameter, but the implementation is not shown. If the function body wasn't updated to actually use the byte parameter for position-aware checking, it could lead to incorrect scope resolution.

---

Reviewed by inspect | Entity-level triage found 0 high-risk changes

[inspect-review]

inspect review

Triage: 221 entities analyzed | 0 critical, 0 high, 87 medium, 134 low
Verdict: standard_review

Findings (7)

1. [low] In find_import_file, the function returns Option<&'a str> but the last line is truncated and appears to return a comparison result instead of finding the matching path. The code shows candidates.into_iter().find(|path| file_stem(path) == source_mod which is incomplete and likely missing the closing parenthesis and the actual return logic.
2. [low] In resolve_with_scopes_full, the BEFORE code extracts class_members from PreBuiltLookups but the AFTER code extracts both class_members and owner_members. However, the BEFORE code shows let mut class_members: HashMap<String, V which is truncated. If owner_members was added to PreBuiltLookups but the unpacking in the else branch wasn't updated to initialize it, this would cause a compilation error or logic bug.
3. [low] In resolve_ref, the function signature adds a new parameter owner_members but the call sites in resolve_with_scopes_full may not be updated to pass this parameter. The diff shows the signature change but doesn't show all call sites being updated, which would cause a compilation error or incorrect behavior if some calls are missing the argument.
4. [low] In resolve_ref, the BEFORE code calls is_local_binding_in_scopes(scope_idx, scopes, name) but the AFTER code calls is_local_binding_in_scopes_at_byte(scope_idx, scopes, name, ast_ref.byte). This changes the function signature being called, but if is_local_binding_in_scopes_at_byte doesn't exist or has different semantics, this would break the logic for detecting local bindings.
5. [low] In build_import_table, the code builds owned_content HashMap but then extends content_map with references to it. However, owned_content is a local variable that will be dropped at the end of the function scope. The code shows content_map.extend(owned_content.iter().map(|(file_path, content)| (file_path.as_str(), content.as_str()))) which creates references with lifetime tied to owned_content, but then uses content_map after this point, potentially creating dangling references.
6. [low] In resolve_with_scopes, the function now calls resolve_with_scopes_full with 7 parameters (adding two None values), but based on the signature change of resolve_with_scopes_full, the 7th parameter is pre_built_import_table: Option<&HashMap<(String, String), String>>. However, the function is being called with None, None which means both pre_built and pre_built_import_table are None, but there's no indication that owner_members is being passed, suggesting a parameter count or ordering issue.
7. [low] In build_import_table, the code changes from using unwrap_or_default() to manually building content_map and owned_content. The new code reads files for non-Go files that aren't in content_map, but then it only adds to owned_content if the file read succeeds. However, the subsequent code that builds ts_default_exports expects all JS/TS files to be in content_map. If a JS/TS file fails to read, it won't be in content_map, causing build_ts_default_export_table to silently skip it, which could lead to incorrect import resolution.

---

Reviewed by inspect | Entity-level triage found 0 high-risk changes

[rs545837]

@Iron-Ham — #308 just went in and this has conflicts now. Needs a rebase onto main before we can merge. Thanks!

I have started realizing why weave-crdts is even more of an interesting thing to have because multiple PRs affecting the same files in a codebase make it a little hard to sort out merge conflicts.

[Iron-Ham]

I have started realizing why weave-crdts is even more of an interesting thing to have because multiple PRs affecting the same files in a codebase make it a little hard to sort out merge conflicts.

👀
Talk to me more about this? AFAIK we can't instruct GitHub to calculate conflicts any way other than the default, but if you're getting at what I think you're getting at it sounds like you're touching on something highly valuable that solves a lot of pain.

FWIW, I think this set of work that's open now would have been better represented as a stack of open PRs but that's not something GitHub supports through fork PRs 🫠

[inspect-review]

inspect review

Triage: 115 entities analyzed | 0 critical, 0 high, 48 medium, 67 low
Verdict: standard_review

Findings (7)

1. [low] In find_import_file, the function returns Option<&'a str> but the last line is truncated and appears to be missing the complete return statement. The code shows candidates.into_iter().find(|path| file_stem(path) == source_mod which is incomplete - it's missing the closing parenthesis and the rest of the expression.
2. [low] In build_import_table, the code checks if file_path.ends_with(".go") || content_map.contains_key(file but the condition is incomplete/truncated. This will cause a compilation error.
3. [low] In resolve_with_scopes_full, the function signature adds a new parameter pre_built_import_table: Option<&HashMap<(String, String), String>> but the corresponding call in resolve_with_scopes passes None for both the last two parameters. However, the destructuring in the BEFORE code shows (symbol_table, class_members, entity_ranges, go_pkg_index) (4 items) while AFTER shows (symbol_table, class_members, owner_members, entity_ranges, go_pkg_index) (5 items). This mismatch will cause a compilation error when pre_built is Some because the tuple destructuring expects 5 items but PreBuiltLookups struct only has 4 fields in the old code.
4. [low] In resolve_ref, there's a logic error where the code checks for entity types "module" | "variable" | "object" before checking for "class" | "struct" | "interface". If an entity matches both conditions (which shouldn't happen but the code structure allows), the first branch will execute and potentially miss class member lookups. More critically, the first branch uses owner_members and lookup_owned_scope_member while the second uses class_members, but both check info.name == receiver, suggesting they might be handling the same case differently.
5. [low] In build_import_table, the code creates owned_content: HashMap<String, String> but the variable is declared as mutable and appears to be used for reading file content for files not in content_map. However, the diff is truncated and doesn't show how this HashMap is populated or used, suggesting incomplete code that may not compile or function correctly.
6. [low] In build_import_table, the code creates a mutable content_map and then tries to extend it, but then creates a separate owned_content HashMap. The logic appears incomplete as shown by the truncated line if file_path.ends_with(".go") || content_map.contains_key(file. This incomplete condition will cause a compilation error.
7. [low] In resolve_ref, there's a logic change where the code now checks for "module" | "variable" | "object" entity types before checking for "class" | "struct" | "interface". However, the second condition uses else if which means if an entity is both a module AND matches the class/struct/interface check, only the first branch executes. This could miss valid class member lookups if an entity somehow satisfies both conditions.

---

Reviewed by inspect | Entity-level triage found 0 high-risk changes

[rs545837]

Yeah it was basically an idea I have been chasing with sem and weave is to think of the file system and the search primitives from first principles now. Because most of our analysis is not line by line or file by file at the moment.

It's basically on top of weave's working and the CRDT layer on top adds proactive prevention, agents claim entities before editing so concurrent edits on the same function are caught before branches even exist, so that as a concept makes the most sense to me at the moment.