Week 5-9 : ANG-007, ANG-008, ANG-009, ANG-010 - #15
Conversation
ANG-007: Markdown preprocessing & embedding pipeline
ANG-009: Wire embedding pipeline into the graph command
ANG-010: Louvain Community detection and degree/centrality scoring
| for (const note of notes) { | ||
| for (const link of note.links ?? []) { | ||
| if (noteIdSet.has(link) && link !== note.id) { | ||
| const key = `${note.id}::${link}::link`; |
There was a problem hiding this comment.
The key for dedupe is include direction (a::b::link), so when note A link to B and also B link to A, this create two different edge, not one. But actually this is same relation, only one connection between A and B. Because of this, degree and centrality for A and B become double, and also in Louvain the weight of edge become 2, same like if have link + tag together, but here is only link. I think better to make key without direction, same way like createTagEdges do with a < b, so mutual link give only one edge
| * scale, not a batch-relative one. | ||
| */ | ||
| private filterBelowFloor(pairs: SimilarityPair[], floor: number): SimilarityPair[] { | ||
| return pairs.filter((p) => p.score >= floor || this.isDirectlyLinked(p)); |
There was a problem hiding this comment.
Pairs that are directly linked can stay in the list even if score is below SEMANTIC_FLOOR, this is by purpose. But problem is, these pairs also go into normalize() function, and there the min/max is calculated from all pairs, including this low-score one. So if one linked pair has a very low or negative score, the min becomes very small, and then all other pairs normalize value become bigger than should be. This means some pair that is not really similar can pass the threshold only because of this one outlier pair. I think min/max should calculate only from pairs that pass the floor by themself, not from the ones kept just because of link
| return bestWord ?? `note:${note.id}`; | ||
| } | ||
|
|
||
| /** Latin-script words only; other scripts fall through to the per-note key above. */ |
There was a problem hiding this comment.
Is this plan to improve later? Because for most other languages, just updating the regex should work fine. Maybe worth adding // TODO: here if this is something we want to do as a post-GSoC enhancement
| const tagEdgeMap = new Map<string, GraphEdge>(); | ||
|
|
||
| for (const [tagName, noteIds] of tagToNotes) { | ||
| if (noteIds.length > 20) continue; |
There was a problem hiding this comment.
Here, if a tag has more than 20 notes, we skip it completely, so zero edges for that tag. I understand the reason (avoid too many pairs, like 20 notes = 190 edges already), but I think is pretty normal for Joplin users to have tags with more than 20 notes. So this cutoff may trigger more often than we think. Is skipping the whole tag really what we want when this happens, or better to still show something (like a cap per note, similar to how Max edges per note slider works for semantic edges)?
| nodeStats[e.source].linkCount++; | ||
| nodeStats[e.target].linkCount++; |
There was a problem hiding this comment.
Is it correct that semantic edges also count into linkCount here?
| } | ||
|
|
||
| /** Counts each note's connections, including notes an edge references that aren't in `notes`. */ | ||
| private computeDegreeMap(notes: Note[], edges: GraphEdge[]): Map<string, number> { |
There was a problem hiding this comment.
computeDegreeMap counts every edge, so a note tied to one neighbor via link+tag+semantic gets degree 3, same as a note connected to 3 distinct neighbors. Is this intentional? LouvainDetector.runLouvain already deduplicates multiple edges between the same pair of nodes into a single weighted edge.
2 participants
No description provided.