refactor: simplify deduplicateIssues to fix complexity lint; fix useless spread in context-filter
Swadhin Biswas committed 6 months ago
ca3e50e
1 parent d39705a
commit ca3e50e
2 files changed
+52 -53
Showing 2 changed files+52-53
▼opengrammar/backend/src/analyzer.ts
+50-45
| @@ -88,61 +88,26 @@ export class RuleBasedAnalyzer { | ||
88 | 88 | * When two issues share the same offset+length, keep the higher priority one. |
89 | 89 | * When one issue fully contains another, keep the more specific (shorter) one. |
90 | 90 | */ |
91 | - private static deduplicateIssues(issues: Issue[]): Issue[] { | |
91 | + private static getPriority(type: string): number { | |
92 | 92 | const PRIORITY: Record<string, number> = { grammar: 4, spelling: 3, clarity: 2, style: 1 }; |
93 | + return PRIORITY[type] || 0; | |
94 | + } | |
93 | 95 | |
94 | - // Sort by offset, then by priority (highest first) | |
95 | - const sorted = issues.sort((a, b) => { | |
96 | + private static deduplicateIssues(issues: Issue[]): Issue[] { | |
97 | + const sorted = [...issues].sort((a, b) => { | |
96 | 98 | if (a.offset !== b.offset) return a.offset - b.offset; |
97 | - return (PRIORITY[b.type] || 0) - (PRIORITY[a.type] || 0); | |
99 | + return this.getPriority(b.type) - this.getPriority(a.type); | |
98 | 100 | }); |
99 | 101 | |
100 | 102 | const result: Issue[] = []; |
101 | - const seenSpans = new Map<string, Issue>(); // key: "offset:length" or normalized original | |
103 | + const seenSpans = new Map<string, Issue>(); | |
102 | 104 | |
103 | 105 | for (const issue of sorted) { |
104 | 106 | const spanKey = `${issue.offset}:${issue.length}`; |
105 | - const origKey = issue.original.toLowerCase().trim(); | |
106 | - | |
107 | - // Case 1: Exact same span — keep higher priority | |
108 | - if (seenSpans.has(spanKey)) { | |
109 | - const existing = seenSpans.get(spanKey)!; | |
110 | - if ((PRIORITY[issue.type] || 0) > (PRIORITY[existing.type] || 0)) { | |
111 | - // Replace with higher-priority version | |
112 | - const idx = result.indexOf(existing); | |
113 | - if (idx >= 0) result[idx] = issue; | |
114 | - seenSpans.set(spanKey, issue); | |
115 | - } | |
116 | - continue; | |
117 | - } | |
118 | 107 | |
119 | - // Case 2: Same original text, same suggestion — skip duplicate | |
120 | - const dedupKey = `${origKey}→${(issue.suggestion || '').toLowerCase().trim()}`; | |
121 | - let isDuplicate = false; | |
122 | - for (const seen of result) { | |
123 | - const seenDedupKey = `${seen.original.toLowerCase().trim()}→${(seen.suggestion || '').toLowerCase().trim()}`; | |
124 | - if (dedupKey === seenDedupKey && Math.abs(issue.offset - seen.offset) < 3) { | |
125 | - isDuplicate = true; | |
126 | - break; | |
127 | - } | |
128 | - } | |
129 | - if (isDuplicate) continue; | |
130 | - | |
131 | - // Case 3: Different rules flagging the same original text at the same position | |
132 | - let overlapping = false; | |
133 | - for (const seen of result) { | |
134 | - if (seen.original.toLowerCase().trim() === origKey && | |
135 | - Math.abs(issue.offset - seen.offset) <= issue.original.length) { | |
136 | - overlapping = true; | |
137 | - // Keep the one with higher priority | |
138 | - if ((PRIORITY[issue.type] || 0) > (PRIORITY[seen.type] || 0)) { | |
139 | - const idx = result.indexOf(seen); | |
140 | - if (idx >= 0) result[idx] = issue; | |
141 | - } | |
142 | - break; | |
143 | - } | |
144 | - } | |
145 | - if (overlapping) continue; | |
108 | + if (this.handleExactSpanMatch(issue, spanKey, seenSpans, result)) continue; | |
109 | + if (this.isIdenticalSuggestionDuplicate(issue, result)) continue; | |
110 | + if (this.handleOverlappingSameText(issue, result)) continue; | |
146 | 111 | |
147 | 112 | result.push(issue); |
148 | 113 | seenSpans.set(spanKey, issue); |
| @@ -151,6 +116,46 @@ export class RuleBasedAnalyzer { | ||
151 | 116 | return result; |
152 | 117 | } |
153 | 118 | |
119 | + private static handleExactSpanMatch(issue: Issue, spanKey: string, seenSpans: Map<string, Issue>, result: Issue[]): boolean { | |
120 | + if (!seenSpans.has(spanKey)) return false; | |
121 | + | |
122 | + const existing = seenSpans.get(spanKey)!; | |
123 | + if (this.getPriority(issue.type) > this.getPriority(existing.type)) { | |
124 | + const idx = result.indexOf(existing); | |
125 | + if (idx >= 0) result[idx] = issue; | |
126 | + seenSpans.set(spanKey, issue); | |
127 | + } | |
128 | + return true; | |
129 | + } | |
130 | + | |
131 | + private static isIdenticalSuggestionDuplicate(issue: Issue, result: Issue[]): boolean { | |
132 | + const origKey = issue.original.toLowerCase().trim(); | |
133 | + const dedupKey = `${origKey}→${(issue.suggestion || '').toLowerCase().trim()}`; | |
134 | + | |
135 | + for (const seen of result) { | |
136 | + const seenDedupKey = `${seen.original.toLowerCase().trim()}→${(seen.suggestion || '').toLowerCase().trim()}`; | |
137 | + if (dedupKey === seenDedupKey && Math.abs(issue.offset - seen.offset) < 3) { | |
138 | + return true; | |
139 | + } | |
140 | + } | |
141 | + return false; | |
142 | + } | |
143 | + | |
144 | + private static handleOverlappingSameText(issue: Issue, result: Issue[]): boolean { | |
145 | + const origKey = issue.original.toLowerCase().trim(); | |
146 | + | |
147 | + for (const seen of result) { | |
148 | + if (seen.original.toLowerCase().trim() === origKey && Math.abs(issue.offset - seen.offset) <= issue.original.length) { | |
149 | + if (this.getPriority(issue.type) > this.getPriority(seen.type)) { | |
150 | + const idx = result.indexOf(seen); | |
151 | + if (idx >= 0) result[idx] = issue; | |
152 | + } | |
153 | + return true; | |
154 | + } | |
155 | + } | |
156 | + return false; | |
157 | + } | |
158 | + | |
154 | 159 | |
155 | 160 | |
156 | 161 | private static checkCustomRules(text: string): Issue[] { |
157 | 162 | |
▼opengrammar/backend/src/rules/context-filter.ts
+2-8
| @@ -102,10 +102,7 @@ const CONTEXT_MODULES: Record<WritingContext, Set<string>> = { | ||
102 | 102 | 'clarity', 'spelling-advanced', 'grammar-advanced', |
103 | 103 | ]), |
104 | 104 | |
105 | - document: new Set([ | |
106 | - // Full check — all modules | |
107 | - ...Object.keys(MODULE_PREFIXES), | |
108 | - ]), | |
105 | + document: new Set(Object.keys(MODULE_PREFIXES)), | |
109 | 106 | |
110 | 107 | academic: new Set([ |
111 | 108 | 'grammar', 'spelling', 'confused-words', 'punctuation', |
| @@ -116,10 +113,7 @@ const CONTEXT_MODULES: Record<WritingContext, Set<string>> = { | ||
116 | 113 | 'formatting-idioms', 'spelling-advanced', 'grammar-advanced', |
117 | 114 | ]), |
118 | 115 | |
119 | - general: new Set([ | |
120 | - // Everything enabled | |
121 | - ...Object.keys(MODULE_PREFIXES), | |
122 | - ]), | |
116 | + general: new Set(Object.keys(MODULE_PREFIXES)), | |
123 | 117 | }; |
124 | 118 | |
125 | 119 | /** |
126 | 120 | |