Skip to content
Merged
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
53 changes: 52 additions & 1 deletion .github/workflows/coderabbit-auto-fix.yml
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,10 @@ jobs:
AGG_TEXT=""
MATCH=0
TOTAL=0
PREV_MATCH=-1
STABLE_HITS=0
BEST_MATCH=0
BEST_AGG=""

# Approvals often have no inline threads — exit retry loop after first fetch (see break below).
for attempt in 1 2 3 4 5; do
Expand Down Expand Up @@ -121,14 +125,39 @@ jobs:
] | length' 2>/dev/null || echo 0)
fi

if [ "${MATCH}" != "0" ] || [ "${REVIEW_STATE}" = "approved" ]; then
# Keep the highest observed match during eventual-consistency windows.
if [ "${MATCH}" -gt "${BEST_MATCH}" ]; then
BEST_MATCH="${MATCH}"
BEST_AGG="${AGG_TEXT}"
fi

# Approvals often have no inline comments; no need to keep polling.
if [ "${REVIEW_STATE}" = "approved" ]; then
break
fi

# Wait for a stable non-zero count across consecutive polls.
if [ "${MATCH}" = "${PREV_MATCH}" ] && [ "${MATCH}" != "0" ]; then
STABLE_HITS=$((STABLE_HITS + 1))
else
STABLE_HITS=0
fi
PREV_MATCH="${MATCH}"

if [ "${STABLE_HITS}" -ge 1 ]; then
break
fi

if [ "${attempt}" -lt 5 ]; then
sleep 3
fi
done

if [ "${BEST_MATCH}" -gt "${MATCH}" ]; then
MATCH="${BEST_MATCH}"
AGG_TEXT="${BEST_AGG}"
fi

{
echo "CLICKUP_AGGREGATE_BODY<<__AGG_EOF__"
echo "$AGG_TEXT"
Expand Down Expand Up @@ -388,6 +417,28 @@ jobs:
fi

ISSUE_COUNT=$(printf '%s\n' "$CLEAN_COMMENT" | grep -oE '### Issue [0-9]+' | wc -l | tr -d '[:space:]')
MATCHED_COUNT="${CLICKUP_MATCH_COUNT:-0}"
if [ "${ISSUE_COUNT}" -lt "${MATCHED_COUNT}" ]; then
MISSING=$((MATCHED_COUNT - ISSUE_COUNT))
echo "::warning::Parsed issue blocks (${ISSUE_COUNT}) are less than matched actionable comments (${MATCHED_COUNT}); adding ${MISSING} fallback block(s)."
REVIEW_ID="${{ github.event.review.id }}"
for i in $(seq 1 "${MISSING}"); do
NEXT_NUM=$((ISSUE_COUNT + i))
CLEAN_COMMENT="${CLEAN_COMMENT}

---

### Issue ${NEXT_NUM}

**Review note (count guard)**

\`\`\`
An actionable CodeRabbit inline comment was matched but could not be structured into Suggested fix / Prompt blocks.
Please review PR inline comments for review id ${REVIEW_ID}.
\`\`\`"
done
ISSUE_COUNT="${MATCHED_COUNT}"
fi
echo "issue_count=${ISSUE_COUNT}" >> "$GITHUB_OUTPUT"
echo "Formatted ClickUp payload: ${ISSUE_COUNT} issue block(s) (### Issue headings)."
{
Expand Down
2 changes: 2 additions & 0 deletions backend/src/controllers/printJobController.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ async function triggerDepartmentPrint(req, res) {
const { order_id, department } = req.params;
const { type = 'standard' } = req.body;

console.log('triggerDepartmentPrint', order_id, department, type);

Comment on lines +13 to +14

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 | ⚡ Quick win

Drop direct console.log of print trigger parameters.

Lines 13-14 log order_id/department/type per request; this is noisy and leaks internal identifiers into raw logs. Use structured logger with level controls if you need diagnostics.

Proposed change
-    console.log('triggerDepartmentPrint', order_id, department, type);
-    
📝 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
console.log('triggerDepartmentPrint', order_id, department, type);
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@backend/src/controllers/printJobController.js` around lines 13 - 14, Remove
the direct console.log call that prints 'triggerDepartmentPrint' along with
order_id, department, and type; replace it with the project's structured logger
(e.g., logger.debug or processLogger.debug) or remove entirely, and if you must
log for diagnostics only emit non-sensitive, masked or aggregated info (no raw
order_id) using the same 'triggerDepartmentPrint' context to locate the log site
in printJobController.js.

if (!['dm', 'confectionery', 'design'].includes(department)) {
return res.status(400).json({
success: false,
Expand Down
3 changes: 2 additions & 1 deletion backend/src/controllers/webhookController.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,12 @@ const { getIO } = require('../config/socket');

function parseDeliveryDate(dateStr) {
if (!dateStr) return null;

console.log('parseDeliveryDate', dateStr);

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 | ⚡ Quick win

Remove ad-hoc payload logging from webhook processing path.

Line 11 logs payload-derived data on every webhook call; this adds noisy production logs and unnecessary data retention. Prefer removing this or gating it behind a proper debug logger flag.

Proposed change
-  console.log('parseDeliveryDate', dateStr);
📝 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
console.log('parseDeliveryDate', dateStr);
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@backend/src/controllers/webhookController.js` at line 11, Remove the ad-hoc
console.log call that prints payload-derived data in the webhook processing path
(the console.log('parseDeliveryDate', dateStr) inside parseDeliveryDate /
webhookController), and either delete it or replace it with a proper debug-level
logger call (e.g., logger.debug or conditional on a DEBUG/LOG_LEVEL env flag) so
production logs do not retain webhook payload values (ensure the symbol dateStr
and the parseDeliveryDate function remain unchanged except for logging).

const parts = dateStr.split('/');
if (parts.length === 3) {
return `${parts[2]}-${parts[1].padStart(2, '0')}-${parts[0].padStart(2, '0')}`;
}

return dateStr;
}

Expand Down
1 change: 1 addition & 0 deletions backend/src/routes/orderRoutes.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,5 +12,6 @@ router.get('/:order_id/timeline', orderController.getOrderTimeline);
router.post('/:order_id/departments/:department/print', printJobController.triggerDepartmentPrint);
router.post('/:order_id/departments/:department/retry', printJobController.retryDepartmentPrint);
router.get('/:order_id/departments/:department/download-pdf', printJobController.downloadDepartmentPdf);
router.get('/:order_id/departments/:department/download-pdf', printJobController.downloadDepartmentPdf);

module.exports = router;
1 change: 1 addition & 0 deletions backend/src/services/timelineService.js
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,7 @@ async function logPrinterValidationFailed(orderId, department, reason) {
}

async function logManualPrintTriggered(orderId, department) {
console.log('logManualPrintTriggered', orderId, department);

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 | ⚡ Quick win

Remove redundant service-level debug log before timeline write.

Line 118 writes a raw debug log for data already captured by the timeline event; this increases log volume without adding durable value.

Proposed change
-  console.log('logManualPrintTriggered', orderId, department);
📝 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
console.log('logManualPrintTriggered', orderId, department);
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@backend/src/services/timelineService.js` at line 118, Remove the redundant
console.log call that prints "logManualPrintTriggered" with orderId and
department; locate the console.log('logManualPrintTriggered', orderId,
department) in timelineService.js and delete it (or replace with a proper
debug-level logger if you must keep it) so that only the durable timeline event
write remains (e.g., the existing timeline/write/createTimelineEvent call) and
avoid duplicate/unnecessary service-level debug output.

await logEvent(
orderId,
'MANUAL_PRINT_TRIGGERED',
Expand Down