Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion lib/Controller/API/App/GetSegmentsController.php
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
use ReflectionException;
use Utils\TaskRunner\Exceptions\EndQueueException;
use Utils\TaskRunner\Exceptions\ReQueueException;
use Utils\LQA\ICUSourceSegmentDetector;
use Utils\Tools\CatUtils;

class GetSegmentsController extends KleinController
Expand Down Expand Up @@ -127,7 +128,7 @@ public function segments(): void
language: $job->source,
patternString: $seg['segment']
);
$string_contains_icu = $analyzer->containsComplexSyntax() && $analyzer->isValidSyntax();
$string_contains_icu = ICUSourceSegmentDetector::sourceContainsIcu($analyzer, $icu_enabled);
}

/** @var MateCatFilter $Filter */
Expand Down
8 changes: 7 additions & 1 deletion lib/Model/ProjectCreation/ProjectManager.php
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
use Plugins\Features\SecondPassReview;
use ReflectionException;
use Throwable;
use TypeError;
Comment thread
Ostico marked this conversation as resolved.
use Utils\ActiveMQ\AMQHandler;
use Utils\ActiveMQ\WorkerClient;
use Utils\AsyncTasks\Workers\ActivityLogWorker;
Expand Down Expand Up @@ -215,7 +216,11 @@ protected function getJobCreationService(): JobCreationService
*/
protected function getQAProcessor(): QAProcessor
{
return $this->qaProcessor ??= new QAProcessor($this->filter, $this->features);
return $this->qaProcessor ??= new QAProcessor(
$this->filter,
$this->features,
(bool) ($this->projectStructure->metadata[ProjectsMetadataMarshaller::ICU_ENABLED->value] ?? false),
);
}

/**
Expand Down Expand Up @@ -1017,6 +1022,7 @@ protected function storeSegments(int $fid): void

/**
* @throws Exception
* @throws TypeError
*/
private function insertSegmentNotesForFile(): void
{
Expand Down
76 changes: 72 additions & 4 deletions lib/Model/ProjectCreation/QAProcessor.php
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,12 @@

namespace Model\ProjectCreation;

use Exception;
use Matecat\ICU\MessagePatternComparator;
use Matecat\ICU\MessagePatternValidator;
use Matecat\SubFiltering\MateCatFilter;
use Model\FeaturesBase\FeatureSet;
use Utils\LQA\ICUSourceSegmentDetector;
use Utils\LQA\QA;

/**
Expand All @@ -13,6 +17,10 @@
* selects the appropriate target string, converts back to Layer 0, and writes the results
* onto each {@see TranslationTuple} in place.
*
* When ICU is enabled for the project, the processor also detects ICU MessageFormat
* patterns in each segment and passes a {@see MessagePatternComparator} to QA so that
* ICU consistency is validated alongside tag checks.
*
Comment on lines +20 to +23
Copy link

Copilot AI Apr 20, 2026

Choose a reason for hiding this comment

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

The PR description lists additional changes/files (PR readiness workflow + scripts, PR template, phpstan @throws rules, phpstan-throws-backlog.txt) that don’t appear to be present in this PR’s code changes. Please either include those files/changes in the PR, or update the description to match what’s actually being merged to avoid confusion for reviewers and release notes.

Copilot uses AI. Check for mistakes.
* Produces 4 scalars per tuple:
* - translationLayer0: the QA-processed target (Layer 0)
* - suggestionLayer0: same as translationLayer0 (may diverge in future)
Expand All @@ -24,6 +32,7 @@ class QAProcessor
public function __construct(
private readonly MateCatFilter $filter,
private readonly FeatureSet $features,
private readonly bool $icuEnabled = false,
) {
}

Expand All @@ -35,6 +44,8 @@ public function __construct(
* @param ProjectStructure $projectStructure contains translations to process
* @param string $sourceLang source language code (e.g. 'en-US')
* @param string $targetLang target language code (e.g. 'it-IT')
*
* @throws Exception
*/
public function process(
ProjectStructure $projectStructure,
Expand All @@ -50,7 +61,14 @@ public function process(
$source = $this->filter->fromLayer0ToLayer1($tuple->source);
$target = $this->filter->fromLayer0ToLayer1($tuple->target);

$check = $this->createQA($source, $target);
[$comparator, $sourceContainsIcu] = $this->detectIcu(
$sourceLang,
$targetLang,
$tuple->source,
$tuple->target,
);

$check = $this->createQA($source, $target, $comparator, $sourceContainsIcu);
$check->setFeatureSet($this->features);
$check->setSourceSegLang($sourceLang);
$check->setTargetSegLang($targetLang);
Expand All @@ -70,12 +88,62 @@ public function process(
}
}

/**
* Detect ICU MessageFormat patterns in the source segment.
*
* Uses the raw Layer 0 content for detection because curly-bracket filter
* handlers are disabled by default at project-creation time, so ICU syntax
* survives the L0β†’L1 round-trip.
*
* @param string $sourceLang source language code
* @param string $targetLang target language code
* @param string $rawSource raw Layer 0 source content
* @param string $rawTarget raw Layer 0 target content
*
* @return array{0: ?MessagePatternComparator, 1: bool}
* [comparator (null when ICU is not detected), sourceContainsIcu flag]
*/
private function detectIcu(
string $sourceLang,
string $targetLang,
string $rawSource,
string $rawTarget,
): array {
if (!$this->icuEnabled) {
return [null, false];
}

$sourceValidator = new MessagePatternValidator($sourceLang, $rawSource);

$sourceContainsIcu = ICUSourceSegmentDetector::sourceContainsIcu($sourceValidator, $this->icuEnabled);

Comment thread
Ostico marked this conversation as resolved.
if (!$sourceContainsIcu) {
return [null, false];
}

$targetValidator = new MessagePatternValidator($targetLang, $rawTarget);

return [
MessagePatternComparator::fromValidators($sourceValidator, $targetValidator),
true,
];
Comment thread
Ostico marked this conversation as resolved.
}

/**
* Create a new QA instance.
* Protected so test subclasses can override to inject stubs.
*
* @param string $source Layer 1 source segment
* @param string $target Layer 1 target segment
* @param MessagePatternComparator|null $comparator ICU pattern comparator (null when ICU not detected)
* @param bool $sourceContainsIcu whether the source contains ICU patterns
*/
protected function createQA(string $source, string $target): QA
{
return new QA($source, $target);
protected function createQA(
string $source,
string $target,
?MessagePatternComparator $comparator = null,
bool $sourceContainsIcu = false,
): QA {
return new QA($source, $target, $comparator, $sourceContainsIcu);
}
}
9 changes: 5 additions & 4 deletions lib/Utils/LQA/ICUSourceSegmentChecker.php
Original file line number Diff line number Diff line change
Expand Up @@ -44,9 +44,10 @@ private function sourceContainsIcu(ProjectStruct $projectStruct, JobStruct $chun
$sourceSegment,
);

if ($this->icuEnabled($projectStruct)) {
$this->sourceContainsIcu = $this->icuSourcePatternValidator->containsComplexSyntax() && $this->icuSourcePatternValidator->isValidSyntax();
}
$this->sourceContainsIcu = ICUSourceSegmentDetector::sourceContainsIcu(
$this->icuSourcePatternValidator,
$this->icuEnabled($projectStruct)
);

return $this->sourceContainsIcu;

Expand All @@ -66,4 +67,4 @@ private function icuEnabled(ProjectStruct $projectStruct): bool
return $this->icuEnabled = $projectStruct->getMetadataValue(ProjectsMetadataMarshaller::ICU_ENABLED->value) ?? false;
}

}
}
26 changes: 26 additions & 0 deletions lib/Utils/LQA/ICUSourceSegmentDetector.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
<?php

namespace Utils\LQA;

use Matecat\ICU\MessagePatternValidator;

/**
* Shared ICU source-segment detection logic.
*/
final class ICUSourceSegmentDetector
{
/**
* Determines whether a source segment contains valid ICU MessageFormat patterns.
*
* @param MessagePatternValidator $validator ICU validator for the source segment
* @param bool $icuEnabled Whether ICU support is enabled for the current project
*
* @return bool True when ICU is enabled and the source has valid complex ICU syntax
*/
public static function sourceContainsIcu(MessagePatternValidator $validator, bool $icuEnabled): bool
{
return $icuEnabled
&& $validator->containsComplexSyntax()
&& $validator->isValidSyntax();
}
}
96 changes: 95 additions & 1 deletion tests/unit/Model/ProjectCreation/QAProcessorTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ protected function setUp(): void
*
* @throws MockException
*/
private function buildProcessor(?QA $qa = null): TestableQAProcessor
private function buildProcessor(?QA $qa = null, bool $icuEnabled = false): TestableQAProcessor
{
$filter = $this->createStub(MateCatFilter::class);
$filter->method('fromLayer0ToLayer1')
Expand All @@ -61,6 +61,7 @@ private function buildProcessor(?QA $qa = null): TestableQAProcessor
$processor = new TestableQAProcessor(
$filter,
$this->createStub(FeatureSet::class),
$icuEnabled,
);

if ($qa !== null) {
Expand Down Expand Up @@ -247,4 +248,97 @@ public function appliesLayerConversions(): void
// fromLayer1ToLayer0('L1:target text') β†’ 'L0:L1:target text'
$this->assertSame('L0:L1:target text', $tuple->translationLayer0);
}

// ── Test 6: ICU enabled + ICU source β†’ comparator passed to createQA ─

/**
* @throws MockException
*/
#[Test]
public function passesComparatorWhenIcuEnabledAndSourceContainsIcu(): void
{
$qa = $this->buildQAStub(normalizedTarget: 'norm');

$icuSource = '{count, plural, one{# item} other{# items}}';
$icuTarget = '{count, plural, one{# elemento} other{# elementi}}';

$tuple = $this->makeTuple(source: $icuSource, target: $icuTarget);
$this->projectStructure->translations = [
'tu-1' => [0 => $tuple],
];

$processor = $this->buildProcessor($qa, icuEnabled: true);
$processor->process($this->projectStructure, 'en-US', 'it-IT');

$this->assertNotNull($processor->lastComparator);
$this->assertTrue($processor->lastSourceContainsIcu);
}

// ── Test 7: ICU disabled + ICU source β†’ no comparator ────────────

/**
* @throws MockException
*/
#[Test]
public function skipsIcuDetectionWhenIcuDisabled(): void
{
$qa = $this->buildQAStub(normalizedTarget: 'norm');

$icuSource = '{count, plural, one{# item} other{# items}}';
$icuTarget = '{count, plural, one{# elemento} other{# elementi}}';

$tuple = $this->makeTuple(source: $icuSource, target: $icuTarget);
$this->projectStructure->translations = [
'tu-1' => [0 => $tuple],
];

$processor = $this->buildProcessor($qa, icuEnabled: false);
$processor->process($this->projectStructure, 'en-US', 'it-IT');

$this->assertNull($processor->lastComparator);
$this->assertFalse($processor->lastSourceContainsIcu);
}

// ── Test 8: ICU enabled + non-ICU source β†’ no comparator ─────────

/**
* @throws MockException
*/
#[Test]
public function skipsIcuDetectionWhenSourceHasNoIcuPatterns(): void
{
$qa = $this->buildQAStub(normalizedTarget: 'norm');

$tuple = $this->makeTuple(source: 'plain source text', target: 'plain target text');
$this->projectStructure->translations = [
'tu-1' => [0 => $tuple],
];

$processor = $this->buildProcessor($qa, icuEnabled: true);
$processor->process($this->projectStructure, 'en-US', 'it-IT');

$this->assertNull($processor->lastComparator);
$this->assertFalse($processor->lastSourceContainsIcu);
}

// ── Test 9: ICU enabled + broken target β†’ QA reports ICU errors ──

#[Test]
public function reportsIcuValidationErrorsForBrokenTarget(): void
{
$icuSource = '{count, plural, one{# item} other{# items}}';
$brokenTarget = '{count, plural, one{# elemento}}';

$tuple = $this->makeTuple(source: $icuSource, target: $brokenTarget);
$this->projectStructure->translations = [
'tu-1' => [0 => $tuple],
];

// No QA stub β€” uses real QA so ICU validation actually runs
$processor = $this->buildProcessor(icuEnabled: true);
$processor->process($this->projectStructure, 'en-US', 'it-IT');

$this->assertSame(1, $tuple->warning);
$this->assertNotEmpty($tuple->serializedErrors);
}
}
26 changes: 17 additions & 9 deletions tests/unit/Model/ProjectCreation/TestableQAProcessor.php
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

namespace unit\Model\ProjectCreation;

use Matecat\ICU\MessagePatternComparator;
use Model\ProjectCreation\QAProcessor;
use Utils\LQA\QA;

Expand All @@ -12,19 +13,26 @@ class TestableQAProcessor extends QAProcessor
{
private ?QA $qaInstance = null;

/**
* Inject a QA stub/mock for tests.
*/
/** @var ?MessagePatternComparator Captures the comparator passed to the last createQA() call */
public ?MessagePatternComparator $lastComparator = null;

/** @var ?bool Captures the sourceContainsIcu flag passed to the last createQA() call */
public ?bool $lastSourceContainsIcu = null;

public function setQA(QA $qa): void
{
$this->qaInstance = $qa;
}

/**
* Override to return the injected QA instance instead of creating a real one.
*/
protected function createQA(string $source, string $target): QA
{
return $this->qaInstance ?? parent::createQA($source, $target);
protected function createQA(
string $source,
string $target,
?MessagePatternComparator $comparator = null,
bool $sourceContainsIcu = false,
): QA {
$this->lastComparator = $comparator;
$this->lastSourceContainsIcu = $sourceContainsIcu;

return $this->qaInstance ?? parent::createQA($source, $target, $comparator, $sourceContainsIcu);
}
}
Loading