Skip to content

Inflection 77: Adding C and C++ API for stating source grammemes and tests for the same. #174

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

Open
wants to merge 12 commits into
base: main
Choose a base branch
from
Open

Inflection 77: Adding C and C++ API for stating source grammemes and tests for the same. #174

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
c008f15
Adding C++ API for stating source grammemes
BHK4321 Aug 12, 2025
c8f22ec
updates
BHK4321 Aug 12, 2025
70fa3e1
Merge branch 'unicode-org:main' into main
BHK4321 Aug 24, 2025
9c8ba42
C API
BHK4321 Aug 24, 2025
c41ec8c
Done
BHK4321 Aug 25, 2025
e676a21
Inflection-77 Adding optional <initial> tag in InflectionTest#testInf…
BHK4321 Aug 27, 2025
873db46
Inflection-77 Test results
BHK4321 Aug 29, 2025
52dc3d1
Merge branch 'unicode-org:main' into main
BHK4321 Sep 12, 2025
e3851a6
Inflection-77 DictionaryLookupFunction::getFeatureValue
BHK4321 Sep 13, 2025
0c4bbeb
Inflection-77: Final issue resolved and added one CAPI test for iinf_…
BHK4321 Nov 3, 2025
fa7133e
Inflection-77: Use CMake’s LibXml2 package
BHK4321 Nov 6, 2025
2e90004
Inflection-77: failing test
BHK4321 Nov 20, 2025
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
5 changes: 0 additions & 5 deletions inflection/src/inflection/dialog/DictionaryLookupFunction.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -124,11 +124,6 @@ SpeakableString* DictionaryLookupFunction::getFeatureValue(const DisplayValue& d
if (result.empty()) {
return nullptr;
}
for(const auto& [feature, value]: displayValue.getConstraintMap()) {
if (feature.getName() == u"speak") {
return new SpeakableString(result, value);
}
}
return new SpeakableString(result);
}

Copy link
Contributor Author

@BHK4321 BHK4321 Sep 13, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

@grhoten
Was this the expected behavior in this function? If not, could you please point out where I am going wrong?

Copy link
Member

@grhoten grhoten Sep 14, 2025

Choose a reason for hiding this comment

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

No. Within the DictionaryLookupFunction, you're only returning the grammeme value. This does not need to change.

Copy link
Contributor Author

@BHK4321 BHK4321 Sep 14, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

DictionaryLookupFunction::getFeatureValue likely needs to be updated to query the relevant values from displayValue. It should probably query DisplayValue::getFeatureValue or DisplayValue::getConstraintMap

Okay sure I wanted to know exactly what I need to do, based on your previous comment I came up with this commit, could you please tell what is expected to make the tests pass so that i could implement the same and further add tests in InfectableStringConceptTest-c.cpp as well to complete this task ?

Expand Down
41 changes: 33 additions & 8 deletions inflection/src/inflection/dialog/InflectableStringConcept.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,11 +52,40 @@ SpeakableString* InflectableStringConcept::getFeatureValue(const SemanticFeature
if (constraint != nullptr) {
return new SpeakableString(*npc(constraint));
}
const auto displayValueResult = getDisplayValue(true);
if (displayValueResult) {
const auto defaultFeatureFunction = npc(getModel())->getDefaultFeatureFunction(feature);
::std::unique_ptr<SpeakableString> computedValue;
::std::unique_ptr<SpeakableString> baseValue;
if (defaultFeatureFunction != nullptr) {
computedValue.reset(npc(defaultFeatureFunction)->getFeatureValue(*displayValueResult, constraints));
baseValue.reset(npc(defaultFeatureFunction)->getFeatureValue(defaultDisplayValue, constraints));
}
const auto& displayConstraintMap = displayValueResult->getConstraintMap();
auto displayConstraint = displayConstraintMap.find(feature);
if (displayConstraint != displayConstraintMap.end()) {
auto numberFeature = npc(getModel())->getFeature(u"number");
if (feature.getName() == u"gender" && baseValue && numberFeature != nullptr && displayConstraintMap.find(*npc(numberFeature)) != displayConstraintMap.end()) {
return baseValue.release();
}
return new SpeakableString(displayConstraint->second);
}
if (computedValue) {
return computedValue.release();
}
if (baseValue) {
return baseValue.release();
}
}
const auto& initialConstraintMap = defaultDisplayValue.getConstraintMap();
auto initialConstraint = initialConstraintMap.find(feature);
if (initialConstraint != initialConstraintMap.end()) {
return new SpeakableString(initialConstraint->second);
}
Copy link
Member

@grhoten grhoten Nov 6, 2025

Choose a reason for hiding this comment

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

These changes are confusing. Why are these changes needed?

Copy link
Member

@grhoten grhoten Nov 6, 2025

Choose a reason for hiding this comment

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

More precisely, why is getFeatureValue being called here? Why is the feature name being checked for gender? What's special about gender or number? Why not grammatical case?

Copy link
Contributor Author

@BHK4321 BHK4321 Nov 6, 2025

Choose a reason for hiding this comment

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

auto displayConstraint = displayConstraintMap.find(feature);
if (displayConstraint != displayConstraintMap.end()) {
    auto numberFeature = npc(getModel())->getFeature(u"number");
    if (feature.getName() == u"gender" && baseValue && numberFeature != nullptr
        && displayConstraintMap.find(*npc(numberFeature)) != displayConstraintMap.end()) {
        return baseValue.release();
    }
    return new SpeakableString(displayConstraint->second);
}

This is the test case which was failing before the changes:

<test>
  <source definiteness="definite">cometa</source>
  <initial gender="masculine"/>
  <result gender="masculine" number="singular">el cometa</result>
</test>

  • InflectableStringConcept::getFeatureValue asks the default display function for a rendered DisplayValue, so feature lookups reflect whatever the display layer injected (articles, inflections, etc.). Spanish determiners add both gender and number to that map, and the gender can contradict the lexeme default (la cometa).

  • To avoid reporting the article’s guess as the noun’s gender, the code detects feature == "gender" and, when the number constraint is present too, returns the lexeme’s base value instead of the display-layer value. English articles never set gender, and no language mutates case in this path yet (I am not sure though), so only the gender/number pair needs a guard today.

  • InflectableStringConcept::getFeatureValue asks the default display function for a rendered DisplayValue, so feature lookups reflect whatever the display layer injected (articles, inflections, etc.).

  • Spanish determiners add both gender and number to that map, and the gender can contradict the lexeme default (la cometa).
    To avoid reporting the article’s guess as the noun’s gender, the code detects feature == "gender" and, when the number constraint is present too, returns the lexeme’s base value instead of the display-layer value, as these conditions when satisfied point to the case when we have an article being inserted.

  • Grammatical case isn’t treated specially because the Spanish article logic never synthesizes case information—so there’s no regression to guard against. If a language later injected case-altering determiners we’d need a similar safeguard, but today only the gender/number pair exhibits the mismatch.

Copy link
Member

@grhoten grhoten Nov 13, 2025

Choose a reason for hiding this comment

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

This behavior does not scale, and it does not make sense in my mind. An InflectableStringConcept is just a simplified SemanticConcept. A SemanticConcept should not have this issue. It does not have this kind of logic, and it does not check on specific grammatical category values.

Copy link
Contributor Author

@BHK4321 BHK4321 Nov 20, 2025

Choose a reason for hiding this comment

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

Yes, you're right it is just a general constructor and logic, I think the underlying locale specific synthesizer needs to be addressed for the failing test case.

auto defaultFeatureFunction = npc(getModel())->getDefaultFeatureFunction(feature);
if (defaultFeatureFunction != nullptr) {
const auto displayValueResult = getDisplayValue(true);
if (displayValueResult) {
return npc(defaultFeatureFunction)->getFeatureValue(*displayValueResult, constraints);
if (auto fallbackValue = ::std::unique_ptr<SpeakableString>(npc(defaultFeatureFunction)->getFeatureValue(defaultDisplayValue, constraints))) {
return fallbackValue.release();
}
}
return nullptr;
Expand All @@ -76,11 +105,7 @@ ::std::optional<DisplayValue> InflectableStringConcept::getDisplayValue(bool all
{
auto defaultDisplayFunction = npc(getModel())->getDefaultDisplayFunction();
if (defaultDisplayFunction != nullptr && !constraints.empty()) {
::std::map<SemanticFeature, ::std::u16string> constraintMap;
if (!value.speakEqualsPrint()) {
constraintMap.emplace(*npc(getSpeakFeature()), value.getSpeak());
}
SemanticFeatureModel_DisplayData displayData({DisplayValue(value.getPrint(), constraintMap)});
SemanticFeatureModel_DisplayData displayData({defaultDisplayValue});
::std::unique_ptr<DisplayValue> returnVal(npc(defaultDisplayFunction)->getDisplayValue(displayData, constraints, allowInflectionGuess));
if (returnVal != nullptr) {
return *returnVal;
Expand Down
67 changes: 67 additions & 0 deletions inflection/test/src/inflection/dialog/InflectableStringConceptTest-c.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,73 @@ TEST_CASE("InflectableStringConceptTest-c#testExistsAPISpanish")
iinf_destroy(inflectableConcept);
}

TEST_CASE("InflectableStringConceptTest-c#testCreateWithDefaults")
{
auto error = U_ZERO_ERROR;
auto ccfp = ilccfp_getDefaultCommonConceptFactoryProvider(&error);
REQUIRE(U_SUCCESS(error));

const auto locale = ::inflection::util::LocaleUtils::SPANISH().getName().c_str();
auto ccf = ilccfp_getCommonConceptFactory(ccfp, locale, &error);
REQUIRE(U_SUCCESS(error));

auto model = iccf_getSemanticFeatureModel(ccf, &error);
REQUIRE(U_SUCCESS(error));

const std::u16string source = u"cometa";
auto sourceString = iss_create(source.c_str(), static_cast<int32_t>(source.size()), &error);
REQUIRE(U_SUCCESS(error));

IDDisplayValue_Constraint defaultConstraints[] = {
{u"gender", u"masculine"},
};

auto inflectableConcept = iinf_createWithDefaults(model,
sourceString,
defaultConstraints,
static_cast<int32_t>(sizeof(defaultConstraints) / sizeof(defaultConstraints[0])),
&error);
iss_destroy(sourceString);
REQUIRE(U_SUCCESS(error));
REQUIRE(inflectableConcept != nullptr);

auto semanticConcept = iinf_toSemanticFeatureConcept(inflectableConcept, &error);
REQUIRE(U_SUCCESS(error));

isfc_putConstraintByName(semanticConcept, u"definiteness", u"definite", -1, &error);
REQUIRE(U_SUCCESS(error));

auto printedValue = isfc_toSpeakableStringCopy(semanticConcept, &error);
REQUIRE(U_SUCCESS(error));
util::StringContainer<IDSpeakableString, iss_getPrint> printedContainer(printedValue);
REQUIRE(printedContainer);
CHECK(printedContainer.value == u"el cometa");
iss_destroy(printedValue);

auto genderValue = isfc_createFeatureValueByNameCopy(semanticConcept, u"gender", &error);
REQUIRE(U_SUCCESS(error));
util::StringContainer<IDSpeakableString, iss_getPrint> genderContainer(genderValue);
REQUIRE(genderContainer);
CHECK(genderContainer.value == u"masculine");
iss_destroy(genderValue);

auto numberValue = isfc_createFeatureValueByNameCopy(semanticConcept, u"number", &error);
REQUIRE(U_SUCCESS(error));
util::StringContainer<IDSpeakableString, iss_getPrint> numberContainer(numberValue);
REQUIRE(numberContainer);
CHECK(numberContainer.value == u"singular");
iss_destroy(numberValue);

auto definitenessValue = isfc_createFeatureValueByNameCopy(semanticConcept, u"definiteness", &error);
REQUIRE(U_SUCCESS(error));
util::StringContainer<IDSpeakableString, iss_getPrint> definitenessContainer(definitenessValue);
REQUIRE(definitenessContainer);
CHECK(definitenessContainer.value == u"definite");
iss_destroy(definitenessValue);

iinf_destroy(inflectableConcept);
}

TEST_CASE("InflectableStringConceptTest-c#testSpanish")
{
auto error = U_ZERO_ERROR;
Expand Down
Loading