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.

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

Fixes the overall score for related keyphrase and collection assessors #21941

Merged
merged 18 commits into from
Feb 3, 2025
Merged
Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
3b25bec
Related keyphrase status neutral/orange instead of happy/green when a…
mykola Jan 2, 2025
1aed651
Related keyphrase status neutral/orange instead of happy/green when a…
mykola Jan 2, 2025
1241e1e
Merge remote-tracking branch 'origin/trunk' into 4475-fix-related-key…
mykola Jan 9, 2025
3d3449d
Related keyphrase status neutral/orange instead of happy/green when a…
mykola Jan 9, 2025
a808333
Related keyphrase status neutral/orange instead of happy/green when a…
mykola Jan 9, 2025
b6a6d7d
Merge remote-tracking branch 'origin/trunk' into 4475-fix-related-key…
mykola Jan 20, 2025
e26339c
Revert "Related keyphrase status neutral/orange instead of happy/gree…
mykola Jan 20, 2025
bf7f76d
Revert "Related keyphrase status neutral/orange instead of happy/gree…
mykola Jan 20, 2025
aa3f33e
Related keyphrase status neutral/orange instead of happy/green when a…
mykola Jan 24, 2025
a2b684d
Related keyphrase status neutral/orange instead of happy/green when a…
mykola Jan 24, 2025
9205a1b
Merge remote-tracking branch 'origin/trunk' into 4475-fix-related-key…
mykola Jan 24, 2025
75f717c
Merge remote-tracking branch 'origin/trunk' into 4475-fix-related-key…
mykola Jan 28, 2025
9856a0d
Related keyphrase status neutral/orange instead of happy/green when a…
mykola Jan 28, 2025
9800b9d
Related keyphrase status neutral/orange instead of happy/green when a…
mykola Jan 28, 2025
2e06b9a
Makes the unit tests more specific
mhkuu Jan 31, 2025
e6e1d6e
Do not set a default aggregator, set it at the right places, and clea…
mhkuu Jan 31, 2025
d95b5ef
Improves JSDoc for the score aggregators
mhkuu Jan 31, 2025
a056131
Additional cleanup for the AnalysisWebWorker
mhkuu Jan 31, 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
Original file line number Diff line number Diff line change
@@ -0,0 +1,127 @@
import { RelatedKeywordScoreAggregator } from "../../../src/scoring/scoreAggregators";
import AssessmentResult from "../../../src/values/AssessmentResult";

describe( "RelatedKeywordScoreAggregator", () => {
let aggregator;

beforeEach( () => {
aggregator = new RelatedKeywordScoreAggregator();
} );

describe( "aggregate", () => {
it( "returns score 0 with default assessment results", () => {
const results = [
new AssessmentResult(),
new AssessmentResult(),
];
const score = aggregator.aggregate( results );
expect( score ).toBe( 0 );
} );

it( "returns score 0 without assessment results", () => {
const results = [];
const score = aggregator.aggregate( results );
expect( score ).toBe( 0 );
} );

it( "returns score 100 with only score 9 assessment results", () => {
const results = [
new AssessmentResult( { score: 9 } ),
new AssessmentResult( { score: 9 } ),
new AssessmentResult( { score: 9 } ),
new AssessmentResult( { score: 9 } ),
];
const score = aggregator.aggregate( results );
expect( score ).toBe( 100 );
} );

it( "returns score 89 with only score 8 assessment results", () => {
const results = [
new AssessmentResult( { score: 8 } ),
new AssessmentResult( { score: 8 } ),
new AssessmentResult( { score: 8 } ),
];
const score = aggregator.aggregate( results );
expect( score ).toBe( 89 );
} );

it( "returns score 67 with only score 6 assessment results", () => {
const results = [
new AssessmentResult( { score: 6 } ),
new AssessmentResult( { score: 6 } ),
new AssessmentResult( { score: 6 } ),
new AssessmentResult( { score: 6 } ),
new AssessmentResult( { score: 6 } ),
new AssessmentResult( { score: 6 } ),
];
const score = aggregator.aggregate( results );
expect( score ).toBe( 67 );
} );

it( "returns score 33 with only score 3 assessment results", () => {
const results = [
new AssessmentResult( { score: 3 } ),
new AssessmentResult( { score: 3 } ),
new AssessmentResult( { score: 3 } ),
new AssessmentResult( { score: 3 } ),
new AssessmentResult( { score: 3 } ),
new AssessmentResult( { score: 3 } ),
new AssessmentResult( { score: 3 } ),
new AssessmentResult( { score: 3 } ),
new AssessmentResult( { score: 3 } ),
new AssessmentResult( { score: 3 } ),
new AssessmentResult( { score: 3 } ),
new AssessmentResult( { score: 3 } ),
];
const score = aggregator.aggregate( results );
expect( score ).toBe( 33 );
} );

it( "returns score 11 with only score 1 assessment results", () => {
const results = [
new AssessmentResult( { score: 1 } ),
new AssessmentResult( { score: 1 } ),
];
const score = aggregator.aggregate( results );
expect( score ).toBe( 11 );
} );

it( "returns score as expected with combined assessment results", () => {
const results = [
new AssessmentResult( { score: 1 } ),
new AssessmentResult( { score: 2 } ),
new AssessmentResult( { score: 3 } ),
new AssessmentResult( { score: 4 } ),
new AssessmentResult( { score: 5 } ),
new AssessmentResult( { score: 6 } ),
new AssessmentResult( { score: 7 } ),
new AssessmentResult( { score: 8 } ),
new AssessmentResult( { score: 9 } ),
];
const score = aggregator.aggregate( results );
expect( score ).toBe( 56 );
} );

it( "returns score as expected with combined assessment results - part 2", () => {
const results = [
new AssessmentResult( { score: 5 } ),
new AssessmentResult( { score: 4 } ),
new AssessmentResult( { score: 8 } ),
];
const score = aggregator.aggregate( results );
expect( score ).toBe( 63 );
} );

it( "exclude assessments without score from aggregator", () => {
const results = [
new AssessmentResult( { score: 5 } ),
new AssessmentResult(),
new AssessmentResult( { score: 4 } ),
new AssessmentResult( { score: 8 } ),
new AssessmentResult(),
];
const score = aggregator.aggregate( results );
expect( score ).toBe( 63 );
} );
} );
} );
16 changes: 16 additions & 0 deletions packages/yoastseo/src/scoring/assessors/assessor.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import LanguageProcessor from "../../parse/language/LanguageProcessor.js";
import MissingArgument from "../../errors/missingArgument.js";
import removeDuplicateMarks from "../../markers/removeDuplicateMarks.js";
import { showTrace } from "../../helpers/errors.js";
import SEOScoreAggregator from "../scoreAggregators/SEOScoreAggregator";

// The maximum score of individual assessment is 9. This is why we set the "score rating" here to 9.
const ScoreRating = 9;
Expand All @@ -17,6 +18,12 @@ const ScoreRating = 9;
* The Assessor is a base class for all assessors.
*/
class Assessor {
/**
* The ScoreAggregator for this assessor.
* @private
*/
_scoreAggregator = new SEOScoreAggregator();

/**
* Creates a new Assessor instance.
* @param {Researcher} researcher The researcher to use.
Expand Down Expand Up @@ -299,6 +306,15 @@ class Assessor {
}.bind( this )
);
}

/**
* Returns the ScoreAggregator for this assessor.
*
* @returns {ScoreAggregator} The specific marker for this assessor.
*/
getScoreAggregator() {
return this._scoreAggregator;
}
}

export default Assessor;
3 changes: 3 additions & 0 deletions packages/yoastseo/src/scoring/assessors/contentAssessor.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import PassiveVoice from "../assessments/readability/PassiveVoiceAssessment.js";
import SentenceBeginnings from "../assessments/readability/SentenceBeginningsAssessment.js";
import TextPresence from "../assessments/readability/TextPresenceAssessment.js";
import scoreToRating from "../interpreters/scoreToRating.js";
import { ReadabilityScoreAggregator } from "../scoreAggregators";

/**
* The ContentAssessor class is used for the readability analysis.
Expand All @@ -31,6 +32,8 @@ export default class ContentAssessor extends Assessor {
new TextPresence(),
new SentenceBeginnings(),
];

this._scoreAggregator = new ReadabilityScoreAggregator();
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import MetaDescriptionKeyword from "../assessments/seo/MetaDescriptionKeywordAss
import TextCompetingLinks from "../assessments/seo/TextCompetingLinksAssessment.js";
import FunctionWordsInKeyphrase from "../assessments/seo/FunctionWordsInKeyphraseAssessment";
import ImageKeyphrase from "../assessments/seo/KeyphraseInImageTextAssessment";
import RelatedKeywordScoreAggregator from "../scoreAggregators/RelatedKeywordScoreAggregator";

/**
* The relatedKeywordAssessor class is used for the related keyword analysis.
Expand All @@ -29,5 +30,7 @@ export default class RelatedKeywordAssessor extends Assessor {
new FunctionWordsInKeyphrase(),
new ImageKeyphrase(),
];

this._scoreAggregator = new RelatedKeywordScoreAggregator();
}
}
3 changes: 3 additions & 0 deletions packages/yoastseo/src/scoring/assessors/taxonomyAssessor.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import PageTitleWidthAssessment from "../assessments/seo/PageTitleWidthAssessmen
import FunctionWordsInKeyphrase from "../assessments/seo/FunctionWordsInKeyphraseAssessment.js";
import SingleH1Assessment from "../assessments/seo/SingleH1Assessment.js";
import { createAnchorOpeningTag } from "../../helpers";
import SEOScoreAggregator from "../scoreAggregators/SEOScoreAggregator";

/**
* Returns the text length assessment to use.
Expand Down Expand Up @@ -61,5 +62,7 @@ export default class TaxonomyAssessor extends Assessor {
new FunctionWordsInKeyphrase(),
new SingleH1Assessment(),
];

this._scoreAggregator = new SEOScoreAggregator();
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
import SEOScoreAggregator from "./SEOScoreAggregator";

/**
* Aggregates SEO assessment results into a single score.
*
* @memberOf module:parsedPaper/assess
*/
class RelatedKeywordScoreAggregator extends SEOScoreAggregator {
/**
* Returns the list of valid results.
* Valid results are all results that have a score.
*
* @param {AssessmentResult[]} results The results to filter the valid results from.
*
* @returns {AssessmentResult[]} The list of valid results.
*/
getValidResults( results ) {
return results.filter( result => result.hasScore() );
}

/**
* Aggregates the given assessment results into a single score.
*
* @param {AssessmentResult[]} results The assessment results.
*
* @returns {number} The aggregated score.
*/
aggregate( results ) {
const validResults = this.getValidResults( results );

return super.aggregate( validResults );
}
}

export default RelatedKeywordScoreAggregator;
1 change: 1 addition & 0 deletions packages/yoastseo/src/scoring/scoreAggregators/index.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
export { default as ReadabilityScoreAggregator } from "./ReadabilityScoreAggregator";
export { default as RelatedKeywordScoreAggregator } from "./RelatedKeywordScoreAggregator";
export { default as ScoreAggregator } from "./ScoreAggregator";
export { default as SEOScoreAggregator } from "./SEOScoreAggregator";
22 changes: 6 additions & 16 deletions packages/yoastseo/src/worker/AnalysisWebWorker.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,6 @@ import RelatedKeywordTaxonomyAssessor from "../scoring/assessors/relatedKeywordT
import SEOAssessor from "../scoring/assessors/seoAssessor.js";
import TaxonomyAssessor from "../scoring/assessors/taxonomyAssessor.js";

// Tree assessor functionality.
import { ReadabilityScoreAggregator, SEOScoreAggregator } from "../scoring/scoreAggregators";

const logger = getLogger( "yoast-analysis-worker" );
logger.setDefaultLevel( "error" );

Expand Down Expand Up @@ -310,10 +307,6 @@ export default class AnalysisWebWorker {
// Registered assessments
this._registeredTreeAssessments = [];

// Score aggregators
this._seoScoreAggregator = new SEOScoreAggregator();
this._contentScoreAggregator = new ReadabilityScoreAggregator();

// Tree representation of text to analyze
this._tree = null;

Expand Down Expand Up @@ -1103,7 +1096,6 @@ export default class AnalysisWebWorker {
this._results.seo[ "" ] = await this.assess( this._paper, this._tree, {
oldAssessor: this._seoAssessor,
treeAssessor: this._seoTreeAssessor,
scoreAggregator: this._seoScoreAggregator,
} );
}

Expand Down Expand Up @@ -1133,10 +1125,10 @@ export default class AnalysisWebWorker {
const analysisCombination = {
oldAssessor: this._contentAssessor,
treeAssessor: this._contentTreeAssessor,
scoreAggregator: this._contentScoreAggregator,
};

// Set the locale (we are more lenient for languages that have full analysis support).
analysisCombination.scoreAggregator.setLocale( this._configuration.locale );
analysisCombination.oldAssessor.getScoreAggregator().setLocale( this._configuration.locale );
mhkuu marked this conversation as resolved.
Show resolved Hide resolved
this._results.readability = await this.assess( this._paper, this._tree, analysisCombination );
}

Expand All @@ -1157,16 +1149,15 @@ export default class AnalysisWebWorker {
* @param {Paper} paper The paper to analyze.
* @param {module:parsedPaper/structure.Node} tree The tree to analyze.
*
* @param {Object} analysisCombination Which assessors and score aggregator to use.
* @param {Object} analysisCombination Which assessors to use.
mhkuu marked this conversation as resolved.
Show resolved Hide resolved
* @param {Assessor} analysisCombination.oldAssessor The original assessor.
* @param {module:parsedPaper/assess.TreeAssessor} analysisCombination.treeAssessor The new assessor.
* @param {module:parsedPaper/assess.ScoreAggregator} analysisCombination.scoreAggregator The score aggregator to use.
*
* @returns {Promise<{score: number, results: AssessmentResult[]}>} The analysis results.
*/
async assess( paper, tree, analysisCombination ) {
// Disabled code: The variable `treeAssessor` is removed from here.
const { oldAssessor, scoreAggregator } = analysisCombination;
const { oldAssessor } = analysisCombination;
/*
* Assess the paper and the tree
* using the original assessor and the tree assessor.
Expand All @@ -1193,7 +1184,7 @@ export default class AnalysisWebWorker {
const results = [ ...treeAssessmentResults, ...oldAssessmentResults ];

// Aggregate the results.
const score = scoreAggregator.aggregate( results );
const score = oldAssessor.getScoreAggregator().aggregate( results );

return {
results: results,
Expand Down Expand Up @@ -1243,11 +1234,10 @@ export default class AnalysisWebWorker {
synonyms: this._relatedKeywords[ key ].synonyms,
} );

// Which combination of (tree) assessors and score aggregator to use.
// Which combination of (tree) assessors to use.
mhkuu marked this conversation as resolved.
Show resolved Hide resolved
const analysisCombination = {
oldAssessor: this._relatedKeywordAssessor,
treeAssessor: this._relatedKeywordTreeAssessor,
scoreAggregator: this._seoScoreAggregator,
};

// We need to remember the key, since the SEO results are stored in an object, not an array.
Expand Down
Loading