Skip to content
Open
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
39 changes: 39 additions & 0 deletions bin/cli.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ const toolkit = require("../index");
const liquidTestGenerator = require("../lib/liquidTestGenerator");
const liquidTestRunner = require("../lib/liquidTestRunner");
const { ExportFileInstanceGenerator } = require("../lib/exportFileInstanceGenerator");
const { LiquidSamplerRunner } = require("../lib/liquidSamplerRunner");
const stats = require("../lib/cli/stats");
const { Command, Option } = require("commander");
const pkg = require("../package.json");
Expand Down Expand Up @@ -514,6 +515,44 @@ program
);
});

// Run Liquid Sampler
program
.command("run-sampler")
.description("Run Liquid Sampler for partner templates (reconciliation texts, account detail templates, and/or shared parts)")
.requiredOption("-p, --partner <partner-id>", "Specify the partner to be used")
.option("-h, --handle <handles...>", "Specify reconciliation text handle(s) - can specify multiple")
.option("-at, --account-template <names...>", "Specify account detail template name(s) - can specify multiple")
.option("-s, --shared-part <names...>", "Specify shared part name(s) - can specify multiple")
.option("--firm-ids <firm-ids...>", "Specify firm ID(s) to run the sampler against - can specify multiple (optional)")
.option("--id <sampler-id>", "Specify an existing sampler ID to fetch results for (optional)")
.action(async (options) => {
// If an existing sampler ID is provided, fetch and display results
if (options.id) {
await new LiquidSamplerRunner(options.partner).checkStatus(options.id);
return;
}

// Validate: at least one template specified
const handles = options.handle || [];
const accountTemplates = options.accountTemplate || [];
const sharedParts = options.sharedPart || [];

if (handles.length === 0 && accountTemplates.length === 0 && sharedParts.length === 0) {
consola.error("You need to specify at least one template using -h, -at, or -s");
process.exit(1);
}

const templateHandles = {
reconciliationTexts: handles,
accountTemplates: accountTemplates,
sharedParts: sharedParts,
};

const firmIds = options.firmIds || [];

await new LiquidSamplerRunner(options.partner).run(templateHandles, firmIds);
});
Comment on lines +518 to +554
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Normalize firmIds to numbers before calling the runner.

options.firmIds from Commander are strings, but LiquidSamplerRunner expects Array<number> (and likely the API does too). Convert and validate before passing.

🛠️ Proposed fix
-    const firmIds = options.firmIds || [];
+    const firmIdsRaw = options.firmIds || [];
+    const firmIds = firmIdsRaw.map((id) => Number(id));
+
+    if (firmIds.some((id) => Number.isNaN(id))) {
+      consola.error("All --firm-ids values must be valid numbers");
+      process.exit(1);
+    }
 
     await new LiquidSamplerRunner(options.partner).run(templateHandles, firmIds);
🤖 Prompt for AI Agents
In `@bin/cli.js` around lines 491 - 527, options.firmIds are strings from
Commander but LiquidSamplerRunner.run expects Array<number>; before constructing
firmIds pass options.firmIds through a numeric normalization step (map to Number
or parseInt) and filter/validate out non-numeric entries, failing with a clear
consola.error and process.exit(1) if any provided ID is not a valid number;
update the firmIds assignment in the run-sampler handler (referencing
options.firmIds and the firmIds variable) so you call new
LiquidSamplerRunner(options.partner).run(templateHandles, firmIds) with an
Array<number>.


// Create Liquid Test
program
.command("create-test")
Expand Down
26 changes: 26 additions & 0 deletions lib/api/sfApi.js
Original file line number Diff line number Diff line change
Expand Up @@ -717,6 +717,30 @@ async function getExportFileInstance(firmId, companyId, periodId, exportFileInst
}
}

async function createSamplerRun(partnerId, attributes) {
const instance = AxiosFactory.createInstance("partner", partnerId);
try {
const response = await instance.post("liquid_sampler/run", attributes);
apiUtils.responseSuccessHandler(response);
return response;
} catch (error) {
const response = await apiUtils.responseErrorHandler(error);
return response;
}
}

async function readSamplerRun(partnerId, samplerId) {
const instance = AxiosFactory.createInstance("partner", partnerId);
try {
const response = await instance.get(`liquid_sampler/${samplerId}`);
apiUtils.responseSuccessHandler(response);
return response;
} catch (error) {
const response = await apiUtils.responseErrorHandler(error);
return response;
}
}

module.exports = {
authorizeFirm,
refreshFirmTokens,
Expand Down Expand Up @@ -771,4 +795,6 @@ module.exports = {
getFirmDetails,
createExportFileInstance,
getExportFileInstance,
createSamplerRun,
readSamplerRun,
};
259 changes: 259 additions & 0 deletions lib/liquidSamplerRunner.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,259 @@
const { UrlHandler } = require("./utils/urlHandler");
const errorUtils = require("./utils/errorUtils");
const { spinner } = require("./cli/spinner");
const SF = require("./api/sfApi");
const fsUtils = require("./utils/fsUtils");
const { consola } = require("consola");

const { ReconciliationText } = require("./templates/reconciliationText");
const { AccountTemplate } = require("./templates/accountTemplate");
const { SharedPart } = require("./templates/sharedPart");

/**
* Class to run liquid samplers for partner templates
*/
class LiquidSamplerRunner {
constructor(partnerId) {
this.partnerId = partnerId;
}

/**
* Run liquid sampler for partner templates
* @param {Object} templateHandles - Object containing arrays of template identifiers
* @param {Array<string>} templateHandles.reconciliationTexts - Array of reconciliation text handles
* @param {Array<string>} templateHandles.accountTemplates - Array of account template names
* @param {Array<string>} templateHandles.sharedParts - Array of shared part names
* @param {Array<number>} firmIds - Array of firm IDs to use in the sampler
* @returns {Promise<void>}
*/
async run(templateHandles = {}, firmIds = []) {
try {
// Validate at least one template specified
const { reconciliationTexts = [], accountTemplates = [], sharedParts = [] } = templateHandles;
if (reconciliationTexts.length === 0 && accountTemplates.length === 0 && sharedParts.length === 0) {
consola.error("You need to specify at least one template using -h, -at, or -s");
process.exit(1);
}

// Build payload
const samplerParams = await this.#buildSamplerParams(templateHandles, firmIds);

consola.info(`Starting sampler run with ${samplerParams.templates.length} template(s)...`);

// Start sampler run
const samplerResponse = await SF.createSamplerRun(this.partnerId, samplerParams);
const samplerId = samplerResponse.data.id || samplerResponse.data;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Suspicious fallback logic for samplerId extraction.

The expression samplerResponse.data.id || samplerResponse.data will assign the entire data object to samplerId if data.id is falsy (0, null, undefined, false, empty string). This could lead to incorrect behavior when a non-string/non-number value is used as the sampler ID in subsequent API calls.

Recommended fix

If the API can return either { data: { id: "..." } } or { data: "..." }, make this explicit:

-      const samplerId = samplerResponse.data.id || samplerResponse.data;
+      const samplerId = typeof samplerResponse.data === 'object' 
+        ? samplerResponse.data.id 
+        : samplerResponse.data;

Otherwise, if id should always be present, remove the fallback and validate:

-      const samplerId = samplerResponse.data.id || samplerResponse.data;
+      const samplerId = samplerResponse.data.id;
       
       if (!samplerId) {
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const samplerId = samplerResponse.data.id || samplerResponse.data;
const samplerId = typeof samplerResponse.data === 'object'
? samplerResponse.data.id
: samplerResponse.data;
🤖 Prompt for AI Agents
In @lib/liquidSamplerRunner.js at line 44, The fallback assigning samplerId
using "samplerResponse.data.id || samplerResponse.data" is unsafe; update the
logic that sets samplerId (the variable assigned from samplerResponse) to
explicitly handle the two supported shapes: if samplerResponse.data is an object
with a defined non-null id (check existence via !== undefined and !== null and
ensure type is string or number) use that id, else if samplerResponse.data is a
primitive string/number use it directly (check typeof), otherwise throw or
return a clear validation error so later API calls receive a valid samplerId.


if (!samplerId) {
consola.error("Failed to start sampler run - no ID returned");
process.exit(1);
}

consola.info(`Sampler run started with ID: ${samplerId}`);

// Poll for completion
const samplerRun = await this.#fetchAndWaitSamplerResult(samplerId);

// Process results
await this.#handleSamplerResponse(samplerRun);
} catch (error) {
errorUtils.errorHandler(error);
}
}

/**
* Fetch the status of an existing sampler run
* @param {string} samplerId - The sampler run ID
* @returns {Promise<void>}
*/
async checkStatus(samplerId) {
try {
consola.info(`Fetching status for sampler run ID: ${samplerId}`);

const response = await SF.readSamplerRun(this.partnerId, samplerId);

if (!response || !response.data) {
consola.error("Failed to fetch sampler run status. Is staging running?");
process.exit(1);
}

await this.#handleSamplerResponse(response.data);
} catch (error) {
errorUtils.errorHandler(error);
}
}

/**
* Build sampler parameters from local template files
* @param {Object} templateHandles - Object containing arrays of template identifiers
* @param {Array<string>} templateHandles.reconciliationTexts - Array of reconciliation text handles
* @param {Array<string>} templateHandles.accountTemplates - Array of account template names
* @param {Array<string>} templateHandles.sharedParts - Array of shared part names
* @param {Array<number>} firmIds - Array of firm IDs to use in the sampler
* @returns {Object} Sampler payload with templates array
*/
async #buildSamplerParams(templateHandles = {}, firmIds = []) {
const templates = [];
const { reconciliationTexts = [], accountTemplates = [], sharedParts = [] } = templateHandles;

// Process reconciliation texts
for (const handle of reconciliationTexts) {
const templateType = "reconciliationText";
const configPresent = fsUtils.configExists(templateType, handle);

if (!configPresent) {
consola.error(`Config file for reconciliation text "${handle}" not found`);
process.exit(1);
}

const config = fsUtils.readConfig(templateType, handle);

// Validate partner_id exists in config
if (!config.partner_id || !config.partner_id[this.partnerId]) {
consola.error(`Template '${handle}' has no partner_id entry for partner ${this.partnerId}. Import the template to this partner first.`);
process.exit(1);
}

const templateId = config.partner_id[this.partnerId];
const templateContent = ReconciliationText.read(handle);

templates.push({
type: "Global::Partner::ReconciliationText",
id: String(templateId),
text: templateContent.text,
text_parts: templateContent.text_parts,
});
}

// Process account templates
for (const name of accountTemplates) {
const templateType = "accountTemplate";
const configPresent = fsUtils.configExists(templateType, name);

if (!configPresent) {
consola.error(`Config file for account template "${name}" not found`);
process.exit(1);
}

const config = fsUtils.readConfig(templateType, name);

// Validate partner_id exists in config
if (!config.partner_id || !config.partner_id[this.partnerId]) {
consola.error(`Template '${name}' has no partner_id entry for partner ${this.partnerId}. Import the template to this partner first.`);
process.exit(1);
}

const templateId = config.partner_id[this.partnerId];
const templateContent = AccountTemplate.read(name);

templates.push({
type: "Global::Partner::AccountDetailTemplate",
id: String(templateId),
text: templateContent.text,
text_parts: templateContent.text_parts,
});
Comment on lines +117 to +154
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Await template reads (and handle falsy content) to avoid Promise deref.

ReconciliationText.read and AccountTemplate.read are called without await, while SharedPart.read is awaited. If these methods are async (as tests elsewhere suggest), templateContent.text will fail at runtime.

🛠️ Proposed fix
-      const templateContent = ReconciliationText.read(handle);
+      const templateContent = await ReconciliationText.read(handle);
+      if (!templateContent) {
+        consola.error(`Reconciliation text "${handle}" wasn't found`);
+        process.exit(1);
+      }
@@
-      const templateContent = AccountTemplate.read(name);
+      const templateContent = await AccountTemplate.read(name);
+      if (!templateContent) {
+        consola.error(`Account template "${name}" wasn't found`);
+        process.exit(1);
+      }
🤖 Prompt for AI Agents
In `@lib/liquidSamplerRunner.js` around lines 117 - 154, ReconciliationText.read
and AccountTemplate.read are being used synchronously and can return Promises or
falsy values; change the calls in the account and reconciliation template loops
to await ReconciliationText.read(handle) and await AccountTemplate.read(name),
and after awaiting validate the result (e.g., if (!templateContent ||
!templateContent.text) { consola.error(...); process.exit(1); }) before
accessing templateContent.text or templateContent.text_parts; use the same
defensive pattern as SharedPart.read to avoid Promise deref and runtime crashes.

}

// Process shared parts
for (const name of sharedParts) {
const templateType = "sharedPart";
const configPresent = fsUtils.configExists(templateType, name);

if (!configPresent) {
consola.error(`Config file for shared part "${name}" not found`);
process.exit(1);
}

const config = fsUtils.readConfig(templateType, name);

// Validate partner_id exists in config
if (!config.partner_id || !config.partner_id[this.partnerId]) {
consola.error(`Shared part '${name}' has no partner_id entry for partner ${this.partnerId}. Import the shared part to this partner first.`);
process.exit(1);
}

const templateId = config.partner_id[this.partnerId];
const templateContent = await SharedPart.read(name);

templates.push({
type: "Global::Partner::SharedPart",
id: String(templateId),
text: templateContent.text,
});
}

return { templates, firm_ids: firmIds };
}

/**
* Poll for sampler run completion
* @param {number} partnerId - The partner ID
* @param {string} samplerId - The sampler run ID
* @returns {Promise<Object>} The completed sampler run
*/
Comment on lines +188 to +193
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Fix stale JSDoc param in #fetchAndWaitSamplerResult.

The doc block lists a partnerId param that the method doesn’t accept.

✏️ Suggested doc fix
   /**
    * Poll for sampler run completion
-   * `@param` {number} partnerId - The partner ID
    * `@param` {string} samplerId - The sampler run ID
    * `@returns` {Promise<Object>} The completed sampler run
    */
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
/**
* Poll for sampler run completion
* @param {number} partnerId - The partner ID
* @param {string} samplerId - The sampler run ID
* @returns {Promise<Object>} The completed sampler run
*/
/**
* Poll for sampler run completion
* `@param` {string} samplerId - The sampler run ID
* `@returns` {Promise<Object>} The completed sampler run
*/
🤖 Prompt for AI Agents
In `@lib/liquidSamplerRunner.js` around lines 188 - 193, The JSDoc for
fetchAndWaitSamplerResult is stale: it lists a partnerId parameter that the
function does not accept; update the docblock above fetchAndWaitSamplerResult in
lib/liquidSamplerRunner.js to remove the incorrect `@param` {number} partnerId
entry and ensure the `@param` entries match the actual signature (e.g., `@param`
{string} samplerId and any other real params) and keep the `@returns` description
accurate so the comment reflects the real function signature.

async #fetchAndWaitSamplerResult(samplerId) {
let samplerRun = { status: "pending" };
const pollingDelay = 15000; // 15 seconds
const waitingLimit = 3600000; // 1 hour

spinner.spin("Running sampler...");
let waitingTime = 0;

while (samplerRun.status === "pending" || samplerRun.status === "running") {
await new Promise((resolve) => setTimeout(resolve, pollingDelay));

const response = await SF.readSamplerRun(this.partnerId, samplerId);
samplerRun = response.data;
Comment thread
coderabbitai[bot] marked this conversation as resolved.

waitingTime += pollingDelay;
// pollingDelay *= 1.05;

if (waitingTime >= waitingLimit) {
spinner.stop();
consola.error("Timeout. Try to fetch the status by using the --id flag, if not run your sampler again");
process.exit(1);
}
}

spinner.stop();
return samplerRun;
}

/**
* Process and display sampler run results
* @param {Object} response - The sampler run response
*/
async #handleSamplerResponse(response) {
if (!response || !response.status) {
consola.error("Invalid sampler response");
process.exit(1);
}

switch (response.status) {
case "failed":
consola.error(`Sampler run failed: ${response.error_message || "Unknown error"}`);
break;
Comment on lines +233 to +235
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Failed sampler runs should exit non-zero.

Line 234 logs the failure and then breaks, so both run() and checkStatus() can complete with exit code 0 even when the sampler run failed.

Suggested patch
       case "failed":
         consola.error(`Sampler run failed: ${response.error_message || "Unknown error"}`);
-        break;
+        process.exit(1);
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@lib/liquidSamplerRunner.js` around lines 233 - 235, The "failed" case
currently only logs the error and breaks, letting run() and checkStatus() exit
with code 0; update the case in the switch (the "failed" branch where
consola.error is called) to set a non-zero exit (e.g., call process.exit(1) or
assign process.exitCode = 1 and then process.exit()) after logging so the
process exits with failure; apply this change in the function containing the
switch (checkStatus / run flow) so a failed sampler run returns a non-zero exit
code.


case "completed":
consola.success("Sampler run completed successfully");

if (response && response.result_url) {
await new UrlHandler(response.result_url).openFile();
} else {
consola.warn("No URL returned");
}
break;

case "pending":
case "running":
consola.info(`Sampler run is still in progress. Current status: "${response.status}". Please check again later.`);
break;

default:
consola.error(`Unexpected sampler status: ${response.status}`);
process.exit(1);
}
}
}

module.exports = { LiquidSamplerRunner };
Loading