Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

disable some French rules if model is available #10500

Merged
merged 3 commits into from
Apr 16, 2024
Merged

disable some French rules if model is available #10500

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
23093c3
disable some French rules if model is available
jaumeortola Apr 11, 2024
2aee6fe
fix: return true
jaumeortola Apr 11, 2024
92e903f
[fr] add one more rule to disabled rules
jaumeortola Apr 11, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
36 changes: 30 additions & 6 deletions languagetool-core/src/main/java/org/languagetool/JLanguageTool.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1008,11 +1008,12 @@ protected CheckResults checkInternal(AnnotatedText annotatedText, ParagraphHandl
paraMode, annotatedText, listener, mode, level, remoteRulesThreadPool == null, toneTags);
long textCheckEnd = System.nanoTime();

Map<String, RemoteRuleResult> remoteRulesResults = null;
try {
TelemetryProvider.INSTANCE.createSpan("fetch-remote-rules",
remoteRulesResults = TelemetryProvider.INSTANCE.createSpan("fetch-remote-rules",
Attributes.builder().put("check.remote_rules.count", remoteRules.size()).build(),
() -> fetchRemoteRuleResults(deadlineStartNanos, mode, level, analyzedSentences, remoteMatches, remoteRuleTasks, remoteRules, requestSize,
cachedResults, matchOffset, annotatedText, textSessionID, toneTags));
cachedResults, matchOffset, annotatedText, textSessionID, toneTags));
} catch (Exception e) {
throw new RuntimeException(e);
}
Expand All @@ -1034,7 +1035,7 @@ protected CheckResults checkInternal(AnnotatedText annotatedText, ParagraphHandl
return res;
}

ruleMatches = filterMatches(annotatedText, rules, ruleMatches, level, toneTags);
ruleMatches = filterMatches(annotatedText, rules, ruleMatches, level, toneTags, remoteRulesResults);

// decide if this should be done right after performCheck, before waiting for remote rule results
// better for latency, remote rules probably don't need resorting
Expand All @@ -1058,12 +1059,16 @@ protected CheckResults checkInternal(AnnotatedText annotatedText, ParagraphHandl
return new CheckResults(ruleMatches, res.getIgnoredRanges(), res.getExtendedSentenceRanges());
}

private List<RuleMatch> filterMatches(AnnotatedText annotatedText, RuleSet rules, List<RuleMatch> ruleMatches, Level level, Set<ToneTag> toneTags) {
private List<RuleMatch> filterMatches(AnnotatedText annotatedText, RuleSet rules, List<RuleMatch> ruleMatches, Level level,
Set<ToneTag> toneTags, Map<String, RemoteRuleResult> remoteRulesResults) {
// rules can create matches with rule IDs different from the original rule (see e.g. RemoteRules)
// so while we can't avoid execution of these rules, we still want disabling them to work
// so do another pass with ignoreRule here
ruleMatches = ruleMatches.stream().filter(match -> !ignoreRule(match.getRule())).collect(Collectors.toList());

ruleMatches = ruleMatches.stream().filter(match -> isRuleActiveForLanguageWithModel(
match.getRule(), language, remoteRulesResults)).collect(Collectors.toList());

ruleMatches = ruleMatches.stream().filter(match -> isRuleActiveForLevelAndToneTags(
match.getRule(), level, toneTags)).collect(Collectors.toList());

Expand All @@ -1078,6 +1083,19 @@ private List<RuleMatch> filterMatches(AnnotatedText annotatedText, RuleSet rules
return applyCustomFilters(ruleMatches, annotatedText);
}

private boolean isRuleActiveForLanguageWithModel(Rule rule, Language language, Map<String, RemoteRuleResult> remoteRulesResults) {
if (language.getShortCode().equals("fr")) {
List<String> disableFrenchRules = Arrays.asList("OU", "OU_FIGEES");
RemoteRuleResult remoteRulesResult = remoteRulesResults.get("AI_FR_GGEC");
if (remoteRulesResult != null) {
if (remoteRulesResult.isSuccess()) {
return !disableFrenchRules.contains(rule.getId());
}
}
}
return true;
}

private final Map<LevelToneTagCacheKey, RuleSet> ruleSetCache = new ConcurrentHashMap<>();

static boolean isRuleActiveForLevelAndToneTags(Rule rule, Level level, Set<ToneTag> toneTags) {
Expand Down Expand Up @@ -1122,13 +1140,14 @@ private RuleSet getActiveRulesForLevelAndToneTags(Level level, Set<ToneTag> tone
});
}

protected void fetchRemoteRuleResults(long deadlineStartNanos, Mode mode, Level level, List<AnalyzedSentence> analyzedSentences, List<RuleMatch> remoteMatches,
protected Map<String, RemoteRuleResult> fetchRemoteRuleResults(long deadlineStartNanos, Mode mode, Level level, List<AnalyzedSentence> analyzedSentences, List<RuleMatch> remoteMatches,
List<FutureTask<RemoteRuleResult>> remoteRuleTasks, List<RemoteRule> remoteRules,
List<Integer> requestSize,
Map<Integer, List<RuleMatch>> cachedResults,
Map<Integer, Integer> matchOffset,
AnnotatedText annotatedText, Long textSessionID,
Set<ToneTag> toneTags) {
Map<String, RemoteRuleResult> remoteRuleResults = new HashMap<>();
if (remoteRuleTasks != null && !remoteRuleTasks.isEmpty()) {
int timeout = IntStream.range(0, requestSize.size()).map(i ->
(int) remoteRules.get(i).getTimeout(requestSize.get(i))
Expand All @@ -1153,13 +1172,17 @@ protected void fetchRemoteRuleResults(long deadlineStartNanos, Mode mode, Level
RemoteRuleMetrics.request(ruleKey, deadlineStartNanos, chars, RemoteRuleMetrics.RequestResult.DOWN);
continue;
}
RemoteRuleResult remoteRuleResult = null;
try {
//logger.info("Fetching results for remote rule for {} chars", chars);
RemoteRuleMetrics.inCircuitBreaker(deadlineStartNanos, rule.circuitBreaker(), ruleKey, chars, () ->
remoteRuleResult = RemoteRuleMetrics.inCircuitBreaker(deadlineStartNanos, rule.circuitBreaker(), ruleKey, chars, () ->
fetchResults(deadlineStartNanos, mode, level, analyzedSentences, remoteMatches, matchOffset, annotatedText, textSessionID, chars, deadlineEndNanos, task, rule, ruleKey, toneTags));
} catch (InterruptedException e) {
break;
}
if (remoteRuleResult != null) {
remoteRuleResults.put(ruleKey, remoteRuleResult);
}
}

for (Integer cachedSentenceIndex : cachedResults.keySet()) {
Expand All @@ -1180,6 +1203,7 @@ protected void fetchRemoteRuleResults(long deadlineStartNanos, Mode mode, Level
// cancel any remaining tasks (e.g. after interrupt because request timed out)
remoteRuleTasks.stream().filter(Objects::nonNull).forEach(t -> t.cancel(true));
}
return remoteRuleResults;
}

private RemoteRuleResult fetchResults(long deadlineStartNanos, Mode mode, Level level, List<AnalyzedSentence> analyzedSentences, List<RuleMatch> remoteMatches, Map<Integer, Integer> matchOffset, AnnotatedText annotatedText, Long textSessionID, long chars, long deadlineEndNanos, FutureTask<RemoteRuleResult> task, RemoteRule rule, String ruleKey, Set<ToneTag> toneTags) throws InterruptedException, ExecutionException, TimeoutException {
Expand Down