Show parent name in terminal output (Class::method_name) - #97
Merged
Conversation
Nested entities now display as Parent::name in the terminal formatter, making it immediately clear which class a changed method belongs to when a file contains multiple classes with similarly-named methods. Also fixes a UTF-8 panic in the truncation path (byte slicing replaced with char_indices). parent_name is serialised as parentName in JSON and omitted when null, so the change is fully additive.
Closed
3 tasks
There was a problem hiding this comment.
inspect review
Triage: 5 entities analyzed | 0 critical, 0 high, 3 medium, 2 low
Verdict: standard_review
Findings (2)
- [low] Terminal formatter truncation uses
display_name.len()(byte length) to decide whether to truncate, but truncates by character index (char_indices().nth(24)). For non-ASCII names,len() > 25can be true even when there are <= 25 characters, causing unnecessary truncation and incorrect output. Evidence:if display_name.len() > 25 { ... display_name.char_indices().nth(24) ... }incrates/sem-cli/src/formatters/terminal.rs. - [low] Terminal formatter truncation produces a string longer than the intended 25-character column: it takes the first 24 characters (
nth(24)gives the start index of the 25th char, so slicing[..i]yields 24 chars) and then appends an ellipsis, resulting in 25 visible chars, butformat!("{:<25}", truncated)will not truncate longer strings; if the slice yields 25 chars in some cases (depending on boundary), appending…can exceed 25 and break alignment. Evidence:format!("{}…", &display_name[..i])followed byformat!("{:<25}", truncated)interminal.rs.
Reviewed by inspect | Entity-level triage found 0 high-risk changes
Member
|
Thanks for opening the PRs, sorry got a bit packed with schedule, will review this today. |
Member
|
Thanks for the PR, it really does make sem more easier to view the diffs, and for the UTF-8 fix. |
3 tasks
nminev
added a commit
to nminev/sem
that referenced
this pull request
Apr 30, 2026
The JSON parser was depth-2: it surfaced top-level keys and one layer of children, then treated everything deeper as opaque text. A change to package.json's scripts.build surfaced as "scripts modified" with the entire scripts object as before/after content. Builds on the parent_name field merged in Ataraxy-Labs#97 and the precision guard merged in Ataraxy-Labs#98. Parser is now fully recursive — every key at every depth is an entity identified by JSON Pointer (/scripts/build). Arrays remain opaque (elements have no stable identity), but array-typed keys themselves are entities. JSON entity IDs are file::pointer (entity_type dropped) so a key whose value type changes (scalar↔object↔array) keeps the same ID and matches Phase 1 as Modified instead of Deleted+Added. parent_name traverses parent_id to build the full ancestor chain (e.g. jest::config for an entity at /jest/config/timeout). Empty ancestor names — package-lock.json's "" root-package key — are skipped so displayed paths stay clean. Suppression extends Ataraxy-Labs#98's precision check: - "object" joins CONTAINER_TYPES so JSON parents are eligible - The Modified-suppression branch additionally requires entity_type to match across before/after, so scalar↔object value transitions keep the parent change - An additional pass drops Moved entries when the entity's old_parent_id is itself in the change set, catching parent-rename failures where children matched by structural hash and the parent did not Behavior is documented in JSON_SEMANTIC_DIFF_SPEC.md, including the remaining limitation around parent-rename plus content change in the same commit. Adds 38 BDD-style tests in json.rs covering top-level, nested rename/ add/delete, deep nesting, type transitions, array opacity, parent- rename fallbacks, document edge cases, and the empty-string-key case from package-lock.json. All 190 sem-core tests pass.
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
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.
This is a follow-up to #94 which was closed. Splitting that PR into two smaller ones — this covers the output display change only.
What
Nested entities now display as
Parent::namein the terminal formatter:When a file has multiple classes with similarly-named methods, the current output gives you no way to tell which class a change belongs to without going to the file. This adds that context directly to the diff output.
Changes
SemanticChangegets a newparent_name: Option<String>field (serialised asparentNamein JSON, omitted when null — fully additive,#[non_exhaustive]already covers this)make_change()inidentity.rspopulates it fromentity.parent_idAlso fixes a latent UTF-8 panic in the truncation path — the old code sliced by byte index which would panic on non-ASCII names. Replaced with
char_indices.Tests
Existing tests all pass. The field is exercised end-to-end through the existing formatter output.