Add the v2 API: SentimentResult, analyze(), analyzeMany(), withLexicon() - #10
Merged
Conversation
… update documentation and migration guide
…ward compatibility and migration details
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds the public API planned for v2. All of it is new surface layered over the
existing
getSentiment()— the diff is 626 insertions and 0 deletions. Noexisting line of scoring logic was touched.
Why this is separate from the modernization PR
The previous PR raised the PHP floor and typed the codebase. This one adds API.
Keeping them apart means a refactor bug and an API-design bug can never appear in
the same diff.
What's added
SentimentResult—final, immutable,readonlypromoted properties.SentimentResult::POSITIVE_THRESHOLD/NEGATIVE_THRESHOLD— ±0.05, theVADER convention, public so callers can reclassify without hardcoding.
InvalidLexiconTermExceptionfor rejected terms.Scores are unchanged — and this proves it
tests/fixtures/baseline.jsonis byte-identical to1.3.0.analyze()delegates to
getSentiment(), andtestAnalyzeAgreesWithGetSentimentAcrossTheBaseline()replays all 355 pinnedcases through the new API and asserts the result object matches the golden
values exactly. That is why the suite goes from 369 to 750 tests. The new API
cannot drift from the scoring contract without failing the same golden master.
tests/ApiContractTest.phpis unmodified and still passes, so the frozenlegacy contract (
getSentiment(),updateLexicon(), the constructor) isprovably unaffected.
Design decisions worth reviewing
withLexicon()is immutable and clones. A freshnew Analyzer()wouldre-parse ~11,000 lines of lexicon files per call; PHP arrays are copy-on-write so
cloning is cheap.
__clone()resets$current_sentitext— without it a cloneinherits the previous call's caps-differential flag, which
testCloneDoesNotShareTransientState()pins.The new API is strict where the legacy one is lenient, deliberately:
updateLexicon()(frozen)withLexicon()(new)['good' => 'abc']0['cut the mustard' => 3]testLegacyUpdateLexiconStaysLenientWhileWithLexiconIsStrict()asserts bothhalves in one test, because the asymmetry looks like an inconsistency someone
would later "tidy up".
Multi-word terms are rejected, not routed into the idiom table. That matcher
has known defects (15 of 21 entries never fire — see
KNOWN-DIVERGENCES.md), sorouting would work only in some positions. A clear error beats a feature that
works sometimes.
toArray()usespositive/negative/neutralwhere the legacy array usespos/neg/neu. This is intentional: the legacy shape is frozen for backwardcompatibility and can't be renamed, while the new one should read clearly.
SentimentResultdoes not implementArrayAccess, so the two can never beswapped silently.
analyzeMany()usesforeach, notarray_map. The parameter isiterableand
array_maprejectsTraversable; there's a test passing anArrayIterator.Not included
explain()— it needs a trace accumulator threaded through the scoring path, soit lands in a later release. A stub that throws would be worse than its absence.
Lexicon file loading is also deferred;
withLexicon()'s signature can widenlater without breaking callers.
Verification
composer matrix— green on PHP 8.1/8.2/8.3/8.4,baseline: identicalcomposer matrix --fresh— same, with a realcomposer installper containercomposer stan— PHPStan level 5 cleangit diff 1.3.0 -- tests/fixtures/baseline.json— emptyverified programmatically, not transcribed by hand