Skip to content

Develop#16

Merged
quangptt0910 merged 4 commits intomainfrom
develop
Jan 31, 2026
Merged

Develop#16
quangptt0910 merged 4 commits intomainfrom
develop

Conversation

@quangptt0910
Copy link
Copy Markdown
Owner

@quangptt0910 quangptt0910 commented Jan 31, 2026

PR Type

Enhancement, Bug fix


Description

  • Rename VelocityConfig to MetricConfig for better semantic clarity

  • Refactor calibration to track separate lambda values for X and Y axes

  • Add lambdaGrid constant for consistent ridge regression parameter tuning

  • Improve accuracy calculation with configurable fixation metrics

  • Add diagnostic logging improvements and code cleanup


Diagram Walkthrough

flowchart LR
  A["VelocityConfig"] -->|Rename| B["MetricConfig"]
  B -->|Add FIXATION section| C["DURATION + SACCADE_GAIN_WINDOW"]
  D["Single lambda value"] -->|Split into| E["lambdaX + lambdaY"]
  F["Hardcoded lambda array"] -->|Extract to| G["lambdaGrid constant"]
  H["calibrationModel"] -->|Store| E
  I["Accuracy calculation"] -->|Use MetricConfig| J["Configurable thresholds"]
Loading

File Walkthrough

Relevant files
Enhancement
7 files
metricConfig.js
Rename VelocityConfig to MetricConfig and add FIXATION section
+9/-4     
mathUtils.js
Extract lambda grid as constant for ridge regression         
+4/-1     
display.js
Track separate lambdaX and lambdaY for calibration model 
+36/-13 
detectSaccade.js
Update imports and references from VelocityConfig to MetricConfig
+8/-8     
saccadeData.js
Update imports and references from VelocityConfig to MetricConfig
+3/-3     
accuracyAdjust.js
Use MetricConfig for thresholds and add documentation       
+22/-12 
index.js
Use MetricConfig constants and improve threshold calculation
+8/-10   
Bug fix
1 files
iris-facemesh.js
Remove debug logging and improve diagnostic output             
+12/-21 
Tests
2 files
detectSaccade.test.js
Update mock imports from VelocityConfig to MetricConfig   
+4/-4     
saccadeData.test.js
Update mock imports from VelocityConfig to MetricConfig   
+3/-3     

@qodo-code-review
Copy link
Copy Markdown

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
Sensitive data logging

Description: Diagnostic console.log statements added here (and additionally in
src/views/GameTest/index.js around the console.log("Pro-Saccade Analysis Data:",
currentPhaseTrials);) may expose sensitive biometric/behavioral data (raw/calibrated
gaze/iris positions, timestamps, trial context) to browser logs or remote log collectors
in production.
iris-facemesh.js [261-270]

Referred Code
// 🔍 DIAGNOSTIC - Log first 3 frames
if (this.trackingData.length <= 6) {
    console.log(`🎮Frame ${this.trackingData.length + 1}:`, {
        timestamp,
        leftIris,
        rightIris,
        totalLandmarks: landmarks.length,
        calibratedAvg: calibratedAvg,
    });
}
Ticket Compliance
🎫 No ticket provided
  • Create ticket/issue
Codebase Duplication Compliance
Codebase context is not defined

Follow the guide to enable codebase context checks.

Custom Compliance
🟢
Generic: Secure Error Handling

Objective: To prevent the leakage of sensitive system information through error messages while
providing sufficient detail for internal debugging.

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

🔴
Generic: Meaningful Naming and Self-Documenting Code

Objective: Ensure all identifiers clearly express their purpose and intent, making code
self-documenting

Status:
Confusing config naming: The new import MetricConfig as metricConfig, MetricConfig introduces two names for the
same configuration and harms readability/self-documentation.

Referred Code
import {MetricConfig as metricConfig, MetricConfig} from "./metricConfig";

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Robust Error Handling and Edge Case Management

Objective: Ensure comprehensive error handling that provides meaningful context and graceful
degradation

Status:
Misleading warning message: The new warning claims a "default threshold: 25 deg/s" is used, but the code
initializes trialThreshold from MetricConfig.SACCADE.STATIC_THRESHOLD_DEG_PER_SEC
(commented as 30°/s), reducing the actionable accuracy of logs during edge cases.

Referred Code
let trialThreshold = MetricConfig.SACCADE.STATIC_THRESHOLD_DEG_PER_SEC; // 30 degree

if (irisTracker.current) {
    const allData = irisTracker.current.getTrackingData();

    console.log("Fixation Window:", fixationStartTime, "-", fixationEndTime);
    console.log("Data sample:", allData.slice(-1)[0]?.timestamp);

    const fixationVelocities = allData
        .filter(frame =>
            frame.timestamp >= fixationStartTime &&
            frame.timestamp <= fixationEndTime &&
            frame.velocity !== undefined &&
            frame.velocity !== null &&
            !isNaN(frame.velocity) &&
            frame.velocity < trialThreshold
        )
        .map(frame => frame.velocity);

    if (fixationVelocities.length >= 15) { // ~ 500 ms
        trialThreshold = calculatePerTrialThreshold(fixationVelocities);


 ... (clipped 3 lines)

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Logging Practices

Objective: To ensure logs are useful for debugging and auditing without exposing sensitive
information like PII, PHI, or cardholder data.

Status:
Sensitive data in logs: New diagnostic console.log outputs raw/calibrated eye tracking coordinates (biometric-like
gaze data) in unstructured logs, which risks exposing sensitive information.

Referred Code
// 🔍 DIAGNOSTIC - Log first 3 frames
if (this.trackingData.length <= 6) {
    console.log(`🎮Frame ${this.trackingData.length + 1}:`, {
        timestamp,
        leftIris,
        rightIris,
        totalLandmarks: landmarks.length,
        calibratedAvg: calibratedAvg,
    });
}

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Security-First Input Validation and Data Handling

Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent
vulnerabilities

Status:
Insecure biometric handling: The PR newly logs detailed gaze/iris data (leftIris, rightIris, calibratedAvg) rather than
handling it as sensitive data, increasing risk of unintended disclosure.

Referred Code
// 🔍 DIAGNOSTIC - Log first 3 frames
if (this.trackingData.length <= 6) {
    console.log(`🎮Frame ${this.trackingData.length + 1}:`, {
        timestamp,
        leftIris,
        rightIris,
        totalLandmarks: landmarks.length,
        calibratedAvg: calibratedAvg,
    });
}

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Comprehensive Audit Trails

Objective: To create a detailed and reliable record of critical system actions for security analysis
and compliance.

Status:
Non-audit console logs: New logging uses ad-hoc console.log without user ID, timestamp, action description, or
outcome, so it cannot serve as a compliant audit trail if these events are considered
critical actions.

Referred Code
if (testPhase === 'pro') {
    // If we just finished Pro-Saccade, trigger break time
    console.log("Pro-Saccade Analysis Data:", currentPhaseTrials);
    setShowBreak(true);
} else {
    // If we just finished Anti-Saccade, Finish the game
    console.log("Anti-Saccade Analysis Data:", currentPhaseTrials);

Learn more about managing compliance generic rules or creating your own custom rules

Compliance status legend 🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

@quangptt0910
Copy link
Copy Markdown
Owner Author

  • Change the config file name
  • Separate some metrics

@quangptt0910 quangptt0910 merged commit 441116c into main Jan 31, 2026
3 checks passed
@qodo-code-review
Copy link
Copy Markdown

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
General
Add TIME.MAX_DELTA_SEC config

Add a TIME block with a MAX_DELTA_SEC property to the MetricConfig object to
prevent a runtime error in detectSaccade.js.

src/views/GameTest/utils/metricConfig.js [7-82]

 export const MetricConfig = {
-    SCREEN: {
-        WIDTH: window.innerWidth || 1920,
-        HEIGHT: window.innerHeight || 1080,
-        HORIZONTAL_FOV_DEGREES: 40,
-        VERTICAL_FOV_DEGREES: 30,
+    // Screen and visual field parameters...
+    TIME: {
+        MAX_DELTA_SEC: 0.05 // max allowed frame interval in seconds
     },
     SACCADE: {
-        STATIC_THRESHOLD_DEG_PER_SEC: 30,
-        ONSET_THRESHOLD_DEG_PER_SEC: 35,
-        OFFSET_THRESHOLD_DEG_PER_SEC: 25,
-        MIN_PEAK_VELOCITY_DEG_PER_SEC: 40,
-        ADAPTIVE: {
-            ENABLED: true,
-            FIXATION_SD_MULTIPLIER: 2.5,
-            MIN_FIXATION_SAMPLES: 20,
-            MAX_FIXATION_VELOCITY: 100,
-        },
-        MAX_BINOCULAR_DISPARITY_DEG_PER_SEC: 100,
+        /* ... existing saccade settings ... */
     },
     FIXATION: {
-        DURATION: 200,
-        SACCADE_GAIN_WINDOW: 100,
+        /* ... */
     },
     LATENCY_VALIDATION: {
-        PRO_SACCADE: { /* ... */ },
-        ANTI_SACCADE: { /* ... */ }
+        /* ... */
     },
+    // other blocks...
 };

[To ensure code accuracy, apply this suggestion manually]

Suggestion importance[1-10]: 7

__

Why: The suggestion correctly identifies that MetricConfig.TIME is missing, which would cause a runtime error, and proposes adding the necessary configuration block.

Medium
Fix off-by-one in diagnostic logging

Adjust the condition in the diagnostic log from this.trackingData.length <= 6 to
this.trackingData.length < 3 to make the number of logged frames match the
comment's description.

src/views/GameTest/utils/iris-facemesh.js [261-270]

 // 🔍 DIAGNOSTIC - Log first 3 frames
-if (this.trackingData.length <= 6) {
+if (this.trackingData.length < 3) {
     console.log(`🎮Frame ${this.trackingData.length + 1}:`, {
         timestamp,
         leftIris,
         rightIris,
         totalLandmarks: landmarks.length,
         calibratedAvg: calibratedAvg,
     });
 }
  • Apply / Chat
Suggestion importance[1-10]: 2

__

Why: The suggestion correctly points out an inconsistency between the code's behavior (logging 7 frames) and its comment (stating 3 frames), which can be confusing during debugging.

Low
Possible issue
Fix typo in success property

Correct the typo in the return object key from sucess to success in the
displayPredictionModel function.

src/views/Calibration/utils/modules/display.js [164-173]

 return {
-    sucess: {left: leftRes.success, right: rightRes.success },
+    success: {left: leftRes.success, right: rightRes.success },
     rmse: {left: leftRes.rmse, right: rightRes.rmse },
     accuracy: { left: leftRes.accuracy, right: rightRes.accuracy },
     lambda: {
         left: {x: leftRes.lambdaX, y: leftRes.lambdaY},
         right: {x: rightRes.lambdaX, y: rightRes.lambdaY}
     },
     method: useRidge ? 'ridge' : 'ols'
 };
  • Apply / Chat
Suggestion importance[1-10]: 5

__

Why: The suggestion correctly identifies a typo in a return object's key (sucess instead of success), which could cause issues for consumers of this function.

Low
Correct inconsistent threshold in warning

Update the hardcoded threshold value in the console.warn message to use the
MetricConfig.SACCADE.STATIC_THRESHOLD_DEG_PER_SEC constant, ensuring the logged
value matches the actual fallback threshold.

src/views/GameTest/index.js [291-295]

 if (fixationVelocities.length >= 15) { // ~ 500 ms
     trialThreshold = calculatePerTrialThreshold(fixationVelocities);
 } else {
-    console.warn(`Trial ${i+1}: Insufficient fixation data (${fixationVelocities.length} samples). Using default threshold: 25 deg/s`);
+    console.warn(`Trial ${i+1}: Insufficient fixation data (${fixationVelocities.length} samples). Using default threshold: ${MetricConfig.SACCADE.STATIC_THRESHOLD_DEG_PER_SEC} deg/s`);
 }
  • Apply / Chat
Suggestion importance[1-10]: 4

__

Why: The suggestion correctly identifies and fixes a misleading log message where the hardcoded threshold value did not match the actual fallback value being used from MetricConfig.

Low
  • More

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant