From 35ba44f22b760c89ffef16f56a62bb9e300c8a7f Mon Sep 17 00:00:00 2001 From: Jimmy Domagala-Tang Date: Fri, 31 Jul 2026 09:47:06 -0400 Subject: [PATCH] Adding project PNG optimization check to compile analytics (#35925) * feat: adding in check for if the project could have had a png optimized as part of the compile-result-backend event * feat: updating using new compile metrics to filter for only eligible project compiles * move projectHasUnconvertedPngs to stats * add e2e test for projectHasUnconvertedPngs * add unit tests for compile-with-optimizable-pngs analytics event --------- Co-authored-by: Brian Gough GitOrigin-RevId: 97e9d184420c34fc1d3e9f6cfe63f6b916f0202f --- services/clsi/app/js/CompileManager.js | 4 +- services/clsi/app/js/HistoryResourceWriter.js | 18 +++- .../unit/js/HistoryResourceWriter.test.js | 51 +++++++++++- .../Features/Compile/CompileController.mjs | 23 ++++++ .../src/Compile/CompileController.test.mjs | 82 ++++++++++++++++++- 5 files changed, 169 insertions(+), 9 deletions(-) diff --git a/services/clsi/app/js/CompileManager.js b/services/clsi/app/js/CompileManager.js index 87a1113460..57770bb290 100644 --- a/services/clsi/app/js/CompileManager.js +++ b/services/clsi/app/js/CompileManager.js @@ -283,7 +283,8 @@ async function doCompile(request, stats, timings) { // Record the PNGs this compile flagged as "slow" so the next sync can convert // them to PDFs (see HistoryResourceWriter). - if (request.isCompileFromHistory && request.png2pdf && Png2Pdf.isEnabled()) { + // todo: generated for every project for analytics, filter to only request.png2pdf once rollout completes + if (request.isCompileFromHistory && Png2Pdf.isEnabled()) { try { const slowPngs = stats.latexmk?.['latexmk-png-slow'] || [] await HistoryResourceWriter.saveSlowPngList( @@ -361,7 +362,6 @@ async function doCompile(request, stats, timings) { 'sampled performance log' ) } - return { outputFiles, buildId, baseHistoryVersion } } diff --git a/services/clsi/app/js/HistoryResourceWriter.js b/services/clsi/app/js/HistoryResourceWriter.js index 0beafbbb40..ea23fa5b20 100644 --- a/services/clsi/app/js/HistoryResourceWriter.js +++ b/services/clsi/app/js/HistoryResourceWriter.js @@ -530,8 +530,11 @@ export async function syncResourcesToDisk( // flagged it as "slow" and it is large enough to be worth converting; keeping // that decision here means the sync loop below just checks membership. const png2pdfActive = request.png2pdf && Png2Pdf.isEnabled() - const slowPngs = new Set(png2pdfActive ? await loadSlowPngList(cacheKey) : []) - const shouldConvert = new Set() + // todo: generated for every project for analytics, filter to only request.png2pdf && Png2Pdf.isEnabled() once rollout completes + const slowPngs = new Set(await loadSlowPngList(cacheKey)) + + // for analytics purposes, we want to generate candidate list for all projects, regardless if they are in fastPNG mode + let shouldConvert = new Set() for (const path of snapshot.getFilePathnames()) { if (!isPng(path) || !slowPngs.has(path)) continue // Avoid doing unnecessary work converting small PNGs. @@ -542,6 +545,17 @@ export async function syncResourcesToDisk( } shouldConvert.add(path) } + + // for analytics to determine if a project could have converted PNG's, even if they arent in the rollout + if (shouldConvert.size > 0) { + stats.projectHasUnconvertedPngs = 1 + } + + // only actually convert if png2pdf was enabled and user compile is eligible + if (!png2pdfActive) { + shouldConvert = new Set() + } + // On a png2pdf mode switch, also re-serve PNGs that a previous compile already // optimised. Once converted, a PNG is no longer flagged slow (it is included // as a PDF), so it drops off the slow-list; without this it would revert to diff --git a/services/clsi/test/unit/js/HistoryResourceWriter.test.js b/services/clsi/test/unit/js/HistoryResourceWriter.test.js index 76b25f103d..a19eada1ec 100644 --- a/services/clsi/test/unit/js/HistoryResourceWriter.test.js +++ b/services/clsi/test/unit/js/HistoryResourceWriter.test.js @@ -136,15 +136,18 @@ describe('HistoryResourceWriter', function () { compileGroup: 'standard', ...overrides, }) - ctx.sync = (overrides = {}) => - ctx.HistoryResourceWriter.syncResourcesToDisk( + ctx.sync = async (overrides = {}) => { + const stats = {} + const result = await ctx.HistoryResourceWriter.syncResourcesToDisk( ctx.projectId, ctx.userId, ctx.makeRequest(overrides), ctx.compileDir, {}, - {} + stats ) + return { ...result, stats } + } }) afterEach(function (ctx) { @@ -241,4 +244,46 @@ describe('HistoryResourceWriter', function () { expect(ctx.convertPng.called).to.equal(false) }) }) + + describe('projectHasUnconvertedPngs (analytics flag)', function () { + it('reports false and converts nothing when no PNG is on the slow-list', async function (ctx) { + // No slow-list saved: fig.png is large enough to convert but was never + // flagged slow, so it is not a conversion candidate. + const { stats } = await ctx.sync() + expect(stats.projectHasUnconvertedPngs).to.equal(undefined) + expect(ctx.convertPng.called).to.equal(false) + }) + + it('reports true without converting when the project has a slow PNG but png2pdf is off', async function (ctx) { + // The headline analytics case: a project in the default (non-png2pdf) + // group that WOULD have had a PNG converted is still flagged, so the two + // rollout groups can be compared like-for-like - but no conversion runs. + await ctx.HistoryResourceWriter.saveSlowPngList(ctx.cacheKey, ['fig.png']) + const { stats } = await ctx.sync({ png2pdf: false }) + expect(stats.projectHasUnconvertedPngs).to.equal(1) + expect(ctx.convertPng.called).to.equal(false) + }) + + it('reports true and converts when the project has a slow PNG and png2pdf is on', async function (ctx) { + // The rollout group: the same convertible PNG is both flagged and + // actually converted. The flag is independent of mode; conversion tracks + // mode. + await ctx.HistoryResourceWriter.saveSlowPngList(ctx.cacheKey, ['fig.png']) + const { stats } = await ctx.sync({ png2pdf: true }) + expect(stats.projectHasUnconvertedPngs).to.equal(1) + expect(ctx.convertPng.calledOnce).to.equal(true) + }) + + it('reports false for a slow PNG below the conversion size threshold', async function (ctx) { + // A slow-listed PNG that is too small to be worth converting is not a + // conversion candidate, so it must not be counted as convertible either - + // the analytics flag uses the same size-filtered set as conversion, so + // the optimised and default groups stay comparable. + ctx.rawSnapshot.files['fig.png'].byteLength = 512 * 1024 + await ctx.HistoryResourceWriter.saveSlowPngList(ctx.cacheKey, ['fig.png']) + const { stats } = await ctx.sync({ png2pdf: true }) + expect(stats.projectHasUnconvertedPngs).to.equal(undefined) + expect(ctx.convertPng.called).to.equal(false) + }) + }) }) diff --git a/services/web/app/src/Features/Compile/CompileController.mjs b/services/web/app/src/Features/Compile/CompileController.mjs index 34a7c39fd7..9b3debb2d3 100644 --- a/services/web/app/src/Features/Compile/CompileController.mjs +++ b/services/web/app/src/Features/Compile/CompileController.mjs @@ -305,6 +305,29 @@ const _CompileController = { ) } + // Report image-inclusion timings for compiles that either used optimised + // PNGs or contained PNGs the optimisation would have targeted. + // Projects the optimisation would never have touched are excluded, + // so they cannot dilute the comparison. + const optimisedPngCount = stats?.['include-image-optimised'] || 0 + const projectHasUnconvertedPngs = Boolean(stats?.projectHasUnconvertedPngs) + if (optimisedPngCount > 0 || projectHasUnconvertedPngs) { + AnalyticsManager.recordEventForUserInBackground( + userId, + 'compile-with-optimizable-pngs', + { + projectId, + optimizedPngCount: optimisedPngCount, + optimizedImageInclusionTime: + timings?.['include-image-optimised'] || 0, + totalImages: stats?.['include-image-all'] || 0, + totalImageInclusionTime: timings?.['include-image-all'] || 0, + isPng2pdf: !!options.png2pdf, + compiler: options.compiler, + } + ) + } + const outputFilesArchive = buildId ? getOutputFilesArchiveSpecification(projectId, userId, buildId) : null diff --git a/services/web/test/unit/src/Compile/CompileController.test.mjs b/services/web/test/unit/src/Compile/CompileController.test.mjs index 50117806e1..1541bbcaf5 100644 --- a/services/web/test/unit/src/Compile/CompileController.test.mjs +++ b/services/web/test/unit/src/Compile/CompileController.test.mjs @@ -166,9 +166,10 @@ describe('CompileController', function () { vi.doMock( '../../../../app/src/Features/Analytics/AnalyticsManager', () => ({ - default: { + default: (ctx.AnalyticsManager = { recordEventForSession: sinon.stub(), - }, + recordEventForUserInBackground: sinon.stub(), + }), }) ) @@ -435,6 +436,83 @@ describe('CompileController', function () { ) }) }) + + describe('compile-with-optimizable-pngs analytics event', function () { + it('records the event when the project has an unconverted png', async function (ctx) { + ctx.CompileManager.promises.compile.resolves({ + status: 'success', + outputFiles: [], + stats: { + 'include-image-all': 3, + projectHasUnconvertedPngs: 1, + }, + timings: { 'include-image-all': 123 }, + }) + await ctx.CompileController.compile(ctx.req, ctx.res, ctx.next) + expect( + ctx.AnalyticsManager.recordEventForUserInBackground + ).to.have.been.calledWith( + ctx.user_id, + 'compile-with-optimizable-pngs', + { + projectId: ctx.projectId, + optimizedPngCount: 0, + optimizedImageInclusionTime: 0, + totalImages: 3, + totalImageInclusionTime: 123, + isPng2pdf: false, + compiler: undefined, + } + ) + }) + + it('records the event when the compile included optimised pngs', async function (ctx) { + ctx.CompileManager.promises.compile.resolves({ + status: 'success', + outputFiles: [], + stats: { + 'include-image-optimised': 2, + 'include-image-all': 5, + projectHasUnconvertedPngs: 0, + }, + timings: { 'include-image-optimised': 12, 'include-image-all': 34 }, + }) + await ctx.CompileController.compile(ctx.req, ctx.res, ctx.next) + expect( + ctx.AnalyticsManager.recordEventForUserInBackground + ).to.have.been.calledWith( + ctx.user_id, + 'compile-with-optimizable-pngs', + { + projectId: ctx.projectId, + optimizedPngCount: 2, + optimizedImageInclusionTime: 12, + totalImages: 5, + totalImageInclusionTime: 34, + isPng2pdf: false, + compiler: undefined, + } + ) + }) + + it('does not record the event when there are no optimizable pngs', async function (ctx) { + ctx.CompileManager.promises.compile.resolves({ + status: 'success', + outputFiles: [], + stats: { 'include-image-all': 3, projectHasUnconvertedPngs: 0 }, + timings: {}, + }) + await ctx.CompileController.compile(ctx.req, ctx.res, ctx.next) + expect(ctx.AnalyticsManager.recordEventForUserInBackground).to.not.have + .been.called + }) + + it('does not record the event when stats are absent', async function (ctx) { + await ctx.CompileController.compile(ctx.req, ctx.res, ctx.next) + expect(ctx.AnalyticsManager.recordEventForUserInBackground).to.not.have + .been.called + }) + }) }) describe('compileSubmission', function () {