Skip to content

MILAB-7092: Repair same-named labels told apart only by absence - #1885

Open
mchernys wants to merge 2 commits into
mainfrom
MILAB-7092_label-shared-distinctions
Open

mchernys wants to merge 2 commits into
mainfrom
MILAB-7092_label-shared-distinctions

Conversation

@mchernys

@mchernys mchernys commented Oct 2, 2026 •

Copy link
Copy Markdown

RetriggerConfidence Score: 5/5

The PR appears safe to merge, though the representative-dependent choice of label part is worth correcting.

Fix All in Claude CodeFindings

  1. P2 Representative depth changes labels ▶
Fix with agent prompt
### Issue 1
sdk/model/src/labels/derive_distinct_labels.ts:635-640
When columns show the same parts but have different hidden trace steps, this code uses only the first column’s trace depth to choose what a bare column should add. Reordering the columns can therefore change which kind of part appears in its label, making labels inconsistent.

```suggestion
          supersetSets[s].flatMap((t) =>
            setMembers[t].flatMap((m) =>
              shown[m]
                .filter(([u, l]) => !isQualification(u) && !setSets[s].has(partKey(u, l)))
                .map(([u]) => depths[group[m]].get(u)),
            ),
          ),
```

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Summary

The PR repairs same-named labels distinguished only by an absent part, then bundles identical shown-part sets to avoid repeated comparisons.

  • Touched terms: Trace is the sequence of label-producing entries; its visible parts are now grouped for subset comparisons. Entry is a column specification with optional trace and qualifications; its labels can be repaired without losing qualification tags. EnrichedRecord holds the expanded trace used to render and compare labels; the new bundling reads its shown parts and trace depths. TypeStats holds per-type importance and occurrence counts; importance guides the choice of replacement distinction.
  • Tests cover shared distinctions, bare members, qualifications, and label uniqueness; the changeset describes the user-visible behavior.

Reviews (2) · Last reviewed commit: "MILAB-7092: Compare shown part sets once..."

@changeset-bot

changeset-bot Bot commented Oct 2, 2026 •

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 8257573

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 6 packages
Name Type
@platforma-sdk/model Patch
@milaboratories/pl-middle-layer Patch
@milaboratories/uikit Patch
@platforma-sdk/test Patch
@platforma-sdk/ui-vue Patch
@platforma-sdk/pl-cli Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@notion-workspace

Copy link
Copy Markdown

Comment thread sdk/model/src/labels/derive_distinct_labels.ts Outdated
@codecov

codecov Bot commented Oct 2, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 57.44%. Comparing base (dc7eb8f) to head (8257573).
⚠️ Report is 1 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1885      +/-   ##
==========================================
+ Coverage   57.19%   57.44%   +0.25%     
==========================================
  Files         446      446              
  Lines       23273    23417     +144     
  Branches     5242     5270      +28     
==========================================
+ Hits        13310    13453     +143     
+ Misses       8422     8418       -4     
- Partials     1541     1546       +5     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@mchernys

mchernys commented Oct 2, 2026

Copy link
Copy Markdown
Author

@greptileai

Comment on lines +635 to +640
supersetSets[s].flatMap((t) => {
const rep = setMembers[t][0];
return shown[rep]
.filter(([u, l]) => !isQualification(u) && !setSets[s].has(partKey(u, l)))
.map(([u]) => depths[group[rep]].get(u));
}),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Representative depth changes labels When columns show the same parts but have different hidden trace steps, this code uses only the first column’s trace depth to choose what a bare column should add. Reordering the columns can therefore change which kind of part appears in its label, making labels inconsistent.

Suggested change
supersetSets[s].flatMap((t) => {
const rep = setMembers[t][0];
return shown[rep]
.filter(([u, l]) => !isQualification(u) && !setSets[s].has(partKey(u, l)))
.map(([u]) => depths[group[rep]].get(u));
}),
supersetSets[s].flatMap((t) =>
setMembers[t].flatMap((m) =>
shown[m]
.filter(([u, l]) => !isQualification(u) && !setSets[s].has(partKey(u, l)))
.map(([u]) => depths[group[m]].get(u)),
),
),
Prompt To Fix With AI
This is a comment left during a code review.
Path: sdk/model/src/labels/derive_distinct_labels.ts
Line: 635-640

Comment:
**Representative depth changes labels** When columns show the same parts but have different hidden trace steps, this code uses only the first column’s trace depth to choose what a bare column should add. Reordering the columns can therefore change which kind of part appears in its label, making labels inconsistent.

```suggestion
          supersetSets[s].flatMap((t) =>
            setMembers[t].flatMap((m) =>
              shown[m]
                .filter(([u, l]) => !isQualification(u) && !setSets[s].has(partKey(u, l)))
                .map(([u]) => depths[group[m]].get(u)),
            ),
          ),
```

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Fix in Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant