-
-
Notifications
You must be signed in to change notification settings - Fork 23
.pr_agent_auto_best_practices
Pattern 1: Validate and sanitize function inputs with type checks, bounds, and defaults to prevent NaN/Infinity, out-of-range indices, or invalid states, and return a clear sentinel value when arguments are unusable.
Example code before:
function computeArea(w, h) {
return (w * h).toFixed(2); // breaks if w/h are null, strings, or negative
}
Example code after:
function computeArea(w, h) {
if (typeof w !== 'number' || typeof h !== 'number' || !isFinite(w) || !isFinite(h) || w < 0 || h < 0) {
return null; // or throw
}
return (w * h).toFixed(2);
}
Relevant past accepted suggestions:
Suggestion 1:
Handle empty/invalid inputs safely
Guard against empty coords after validation; current code will use Math.min/max on empty arrays, yielding Infinity/-Infinity and NaN density. Return false if no valid cells remain. Also coerce minDensity to a number and clamp between 0 and 1 to avoid unexpected truthy strings.
function clusteredCellsDensity(numbers, size, minDensity) {
- if (!numbers || !numbers.length) return false;
- if (typeof size !== 'number' || size <= 0) size = 4; // default grid size = 4
- if (typeof minDensity === 'undefined') minDensity = 0.6;
+ if (!Array.isArray(numbers) || numbers.length === 0) return false;
+ if (typeof size !== 'number' || size <= 0) size = 4;
+ if (typeof minDensity !== 'number' || isNaN(minDensity)) minDensity = 0.6;
+ if (minDensity < 0) minDensity = 0;
+ if (minDensity > 1) minDensity = 1;
- var maxIndex = size * size; // because it's 1-based (1..size*size)
+ var maxIndex = size * size;
- // 1. Convert cell number → (x,y) coordinates with 1-based check
var coords = [];
for (var i = 0; i < numbers.length; i++) {
- var n = numbers[i];
- // must be within 1..maxIndex
- if (typeof n !== 'number' || n < 1 || n > maxIndex) {
- return false; // invalid index -> immediately return false
- }
- var idx = n - 1; // shift to 0-based
+ var n = numbers[i];
+ if (typeof n !== 'number' || !isFinite(n) || n % 1 !== 0 || n < 1 || n > maxIndex) {
+ return false;
+ }
+ var idx = n - 1;
coords.push({ x: idx % size, y: Math.floor(idx / size) });
}
+
+ if (coords.length === 0) return false;
var xs = coords.map(function (c) { return c.x; });
var ys = coords.map(function (c) { return c.y; });
- // 2. Compute bounding box of selected cells
var minX = Math.min.apply(Math, xs), maxX = Math.max.apply(Math, xs);
var minY = Math.min.apply(Math, ys), maxY = Math.max.apply(Math, ys);
- // 3. Compute area and density
- var w = maxX - minX + 1;
- var h = maxY - minY + 1;
+ var w = (maxX - minX + 1);
+ var h = (maxY - minY + 1);
var boxArea = w * h;
+ if (!isFinite(boxArea) || boxArea <= 0) return false;
var density = coords.length / boxArea;
return density >= minDensity;
}Suggestion 2:
Validate tile position inputs
Add input validation to prevent NaN/Infinity when tiles is zero/negative or index is out of range. Clamp col/row to non-negative and return null for invalid arguments to avoid propagating bad coordinates.
-function estimateTileStartPosition(index, tiles, spreadSize, gap) {
+function estimateTileStartPosition(index, tiles, spreadSize, gap) {
+ if (typeof tiles !== 'number' || tiles <= 0) return null;
+ if (typeof spreadSize !== 'number' || !isFinite(spreadSize)) return null;
+ if (typeof gap !== 'number' || !isFinite(gap) || gap < 0) gap = 0;
+ if (typeof index !== 'number' || !isFinite(index) || index % 1 !== 0 || index < 1) return null;
+
+ var totalSlots = tiles * tiles;
+ if (index > totalSlots) return null;
+
var tileSize = (spreadSize - gap * (tiles - 1)) / tiles;
+ if (!isFinite(tileSize)) return null;
var i = index - 1;
var col = i % tiles;
var row = Math.floor(i / tiles);
-
- return {
- x: col * (tileSize + gap),
- y: row * (tileSize + gap)
+
+ return {
+ x: Math.max(0, col) * (tileSize + gap),
+ y: Math.max(0, row) * (tileSize + gap)
};
}Suggestion 3:
Fix grammar and add validation
The comment contains a grammatical error ("an unwanted features" should be "unwanted features"). Additionally, consider adding validation to ensure disableFeatures is an array before calling join() to prevent runtime errors.
-// disable an unwanted features
-cmd.push("--disable-features=" + this.disableFeatures.join(','));
+// disable unwanted features
+if (Array.isArray(this.disableFeatures) && this.disableFeatures.length > 0) {
+ cmd.push("--disable-features=" + this.disableFeatures.join(','));
+}Suggestion 4:
Add type check before parsing
The code assumes the response is always a string that needs parsing, but HTTP responses might already be parsed objects. This could cause errors if the response is already a JSON object. Add a type check before parsing.
lib/language-inference-engine.js [217]
-response = JSON.parse(response)
+response = typeof response === 'string' ? JSON.parse(response) : response;Suggestion 5:
Add null check
The function doesn't check if promptEditorRef.current exists before calling methods on it, which could lead to runtime errors if the ref isn't initialized.
WelsonJS.Toolkit/WelsonJS.Launcher/editor.html [195-202]
const invoke = () => {
try {
- const updated = promptEditorRef.current.get();
- promptMessagesRef.current = updated;
+ if (promptEditorRef.current) {
+ const updated = promptEditorRef.current.get();
+ promptMessagesRef.current = updated;
+ }
} catch (e) {
console.error("Invalid JSON structure", e);
}
};Pattern 2: Avoid fire-and-forget async operations; return Task/Promise and either await during initialization or attach continuations that log and observe exceptions.
Example code before:
// startup
loadConfig(); // async void/Promise not awaited
Example code after:
// startup
const start = async () => {
await loadConfig(); // or
// _ = loadConfig().catch(err => logger.error(err));
};
Relevant past accepted suggestions:
Suggestion 1:
Safely handle async init task
The async call to fetch configuration is fire-and-forget, which can swallow exceptions and race with code that depends on _blobConfig. Capture the task and log exceptions, or await during startup. If startup cannot be async, store the task and handle its exceptions on a background continuation.
WelsonJS.Toolkit/WelsonJS.Launcher/ResourceServer.cs [45-57]
public ResourceServer(string prefix, string resourceName, ICompatibleLogger logger = null)
{
_logger = logger ?? new TraceLogger();
_prefix = prefix;
_listener = new HttpListener();
_resourceName = resourceName;
- // Fetch a blob config from Internet
- FetchBlobConfig().ConfigureAwait(false);
+ // Fetch a blob config from Internet (safe fire-and-forget with logging)
+ _ = FetchBlobConfig().ContinueWith(t =>
+ {
+ if (t.IsFaulted)
+ {
+ _logger?.Error($"FetchBlobConfig failed: {t.Exception}");
+ }
+ }, TaskScheduler.Default);
// Add resource tools
_tools.Add(new ResourceTools.Completion(this, _httpClient));
_tools.Add(new ResourceTools.Settings(this, _httpClient));Suggestion 2:
Fix async method signature
The FetchBlobConfig method is asynchronous but doesn't return a Task, making it fire-and-forget. This could lead to unhandled exceptions and the application might continue without a properly loaded blob configuration. Consider making it return a Task and awaiting it during initialization.
WelsonJS.Toolkit/WelsonJS.Launcher/ResourceServer.cs [425-444]
-private static async void FetchBlobConfig()
+private static async Task FetchBlobConfig()
{
try
{
string url = Program.GetAppConfig("BlobConfigUrl");
var response = await _httpClient.GetStreamAsync(url);
var serializer = new XmlSerializer(typeof(BlobConfig));
using (var reader = new StreamReader(response))
{
_blobConfig = (BlobConfig)serializer.Deserialize(reader);
}
_blobConfig?.Compile();
}
catch (Exception ex)
{
Trace.TraceError($"Failed to fetch a blob config. Exception: {ex.Message}");
}
}Pattern 3: Check for null/undefined before dereferencing objects or using deserialization results and streams; gracefully handle null cases with logging or early returns.
Example code before:
const title = config.settings.title.toLowerCase();
Example code after:
if (!config || !config.settings || typeof config.settings.title !== 'string') {
logger.warn('Missing title in config');
} else {
const title = config.settings.title.toLowerCase();
}
Relevant past accepted suggestions:
Suggestion 1:
Guard deserialization with null checks
Add a null check on the deserialization result before using it to avoid possible NullReferenceException. Also validate that the stream is not null before constructing the reader.
WelsonJS.Toolkit/WelsonJS.Launcher/ResourceServer.cs [448-453]
using (var response = await _httpClient.GetStreamAsync(url))
-using (var reader = new StreamReader(response))
{
- var serializer = new XmlSerializer(typeof(BlobConfig));
- _blobConfig = (BlobConfig)serializer.Deserialize(reader);
+ if (response == null)
+ {
+ _logger?.Warn("Blob config response stream was null.");
+ return;
+ }
+ using (var reader = new StreamReader(response))
+ {
+ var serializer = new XmlSerializer(typeof(BlobConfig));
+ var deserialized = serializer.Deserialize(reader) as BlobConfig;
+ if (deserialized == null)
+ {
+ _logger?.Warn("Failed to deserialize BlobConfig (null result).");
+ return;
+ }
+ _blobConfig = deserialized;
+ }
}Suggestion 2:
Fix null reference exception risk
The null check for schema should occur before accessing schema.PrimaryKey to prevent potential null reference exceptions. Move the null validation to the beginning of the constructor.
WelsonJS.Toolkit/WelsonJS.Esent/EsentDatabase.cs [35-41]
public EsentDatabase(Schema schema, string workingDirectory, ICompatibleLogger logger = null)
{
+ if (schema == null)
+ throw new ArgumentNullException(nameof(schema));
+
_logger = logger ?? new TraceLogger();
-
_primaryKey = schema.PrimaryKey;
- if (schema == null)
-Suggestion 3:
Add null check
The function doesn't check if promptEditorRef.current exists before calling methods on it, which could lead to runtime errors if the ref isn't initialized.
WelsonJS.Toolkit/WelsonJS.Launcher/editor.html [195-202]
const invoke = () => {
try {
- const updated = promptEditorRef.current.get();
- promptMessagesRef.current = updated;
+ if (promptEditorRef.current) {
+ const updated = promptEditorRef.current.get();
+ promptMessagesRef.current = updated;
+ }
} catch (e) {
console.error("Invalid JSON structure", e);
}
};Pattern 4: Preserve error context by returning structured error objects or flags instead of ambiguous empty values to enable caller-side branching and diagnostics.
Example code before:
try {
const data = await fetchData();
return {};
} catch (e) {
return {};
}
Example code after:
try {
const data = await fetchData();
return { error: false, data };
} catch (e) {
return { error: true, message: e?.message || 'Fetch failed' };
}
Relevant past accepted suggestions:
Suggestion 1:
Return structured error on failure
Preserve error context in the response to allow callers to detect failures instead of returning an indistinguishable empty object. Include an error flag and message so downstream logic can branch appropriately.
} catch (e) {
console.warn(e.message);
- response = {};
+ response = { error: true, message: e && e.message ? e.message : "Request failed" };
}Suggestion 2:
Guard deserialization with null checks
Add a null check on the deserialization result before using it to avoid possible NullReferenceException. Also validate that the stream is not null before constructing the reader.
WelsonJS.Toolkit/WelsonJS.Launcher/ResourceServer.cs [448-453]
using (var response = await _httpClient.GetStreamAsync(url))
-using (var reader = new StreamReader(response))
{
- var serializer = new XmlSerializer(typeof(BlobConfig));
- _blobConfig = (BlobConfig)serializer.Deserialize(reader);
+ if (response == null)
+ {
+ _logger?.Warn("Blob config response stream was null.");
+ return;
+ }
+ using (var reader = new StreamReader(response))
+ {
+ var serializer = new XmlSerializer(typeof(BlobConfig));
+ var deserialized = serializer.Deserialize(reader) as BlobConfig;
+ if (deserialized == null)
+ {
+ _logger?.Warn("Failed to deserialize BlobConfig (null result).");
+ return;
+ }
+ _blobConfig = deserialized;
+ }
}[Auto-generated best practices - 2025-08-26]
This document may contain outdated content. For the latest information, please contact us directly or refer to the webpage below.