Skip to content

added extra logs #27

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
hemusimple wants to merge 15 commits into
base: main
Choose a base branch
from
Open

added extra logs #27

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
a3a88c7
added extra logs
hemusimple Apr 16, 2026
56f603a
Add Kotlin PR review workflow configuration
hemusimple Apr 16, 2026
04cab66
Add Kotlin PR review workflow configuration
hemusimple Apr 16, 2026
3a6ea4f
Delete kotlin-review.yml
hemusimple Apr 16, 2026
7b9d97b
Update Python command to use python3 in CI workflow
hemusimple Apr 16, 2026
aca19ec
Change pip install command to pip3 with specific packages
hemusimple Apr 16, 2026
6b998e7
Add getData function to LyricsActivity
hemusimple Apr 16, 2026
49e27b6
Update getData method to return string representation
hemusimple Apr 16, 2026
360576e
Add song property to LyricsActivity
hemusimple Apr 16, 2026
571eb7e
Update Python installation and script path in workflow
hemusimple Apr 18, 2026
cb731ee
Activate virtual environment in CI workflow
hemusimple Apr 18, 2026
4e1e7d0
Change shell from pwsh to powershell in workflow
hemusimple Apr 18, 2026
8fd40d0
Fix output redirection in Kotlin review workflow
hemusimple Apr 18, 2026
57d9a78
Change activation script and shell for CI workflow
hemusimple Apr 18, 2026
c5c17e8
Modify review script execution in workflow
hemusimple Apr 18, 2026
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
48 changes: 48 additions & 0 deletions .github/workflows/kotlin-review.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
name: Kotlin PR Review

on:
pull_request:
types: [opened, reopened, synchronize]

jobs:
kotlin_review:
runs-on: code-review # ← matches the label you gave your runner

timeout-minutes: 15

steps:
- name: Checkout
uses: actions/checkout@v4
with:
fetch-depth: 0 # full history needed for git diff

- name: Run Kotlin Review Agent
env:
PLATFORM: github
REVIEWER_TOKEN: ${{ secrets.REVIEWER_TOKEN }}
OLLAMA_HOST: ${{ secrets.OLLAMA_HOST }}
OLLAMA_MODEL: ${{ secrets.OLLAMA_MODEL }}
Comment on lines +20 to +24

@coderabbitai coderabbitai Bot Apr 18, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Inspect the review workflow and any committed review scripts for risky secret logging patterns.
rg -n -C3 'REVIEWER_TOKEN|OLLAMA_HOST|OLLAMA_MODEL|os\.environ|print\(|logger\.|loguru' .github || true

Repository: tejpratap46/AndroidVideoMotion

Length of output: 1461

🏁 Script executed:

# Get the full kotlin-review.yml to see artifact upload section (lines 40-46)
cat .github/workflows/kotlin-review.yml

Repository: tejpratap46/AndroidVideoMotion

Length of output: 1724

🏁 Script executed:

# Search for review_ci.py in the repository
fd review_ci.py

Repository: tejpratap46/AndroidVideoMotion

Length of output: 56

Redact secret-bearing output before uploading the log artifact.

The workflow passes secrets (REVIEWER_TOKEN, OLLAMA_HOST, OLLAMA_MODEL) into the review process environment, then pipes raw stdout/stderr to review_output.log and uploads it with if: always() (lines 40-46). GitHub's console log masking does not protect artifact files. Since the external review_ci.py script (not in this repository) could print environment variables, configuration, or HTTP headers, the artifact can persist values that would otherwise be masked in console logs.

🔒 Suggested hardening 
         run: |
           pip install -q httpx python-dotenv pydantic pydantic-settings ollama loguru tenacity
-          python D:\AIML\code-reviewer\review_ci.py 2>&1 | Tee-Object -FilePath review_output.log
+          $output = & python D:\AIML\code-reviewer\review_ci.py 2>&1
+          $exitCode = $LASTEXITCODE
+          $secrets = @($env:REVIEWER_TOKEN, $env:OLLAMA_HOST, $env:OLLAMA_MODEL) |
+            Where-Object { -not [string]::IsNullOrWhiteSpace($_) }
+          $output |
+            ForEach-Object {
+              $line = [string]$_
+              foreach ($secret in $secrets) {
+                $line = $line.Replace($secret, '***')
+              }
+              $line
+            } |
+            Tee-Object -FilePath review_output.log
+          exit $exitCode
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In @.github/workflows/kotlin-review.yml around lines 20 - 24, The workflow
currently exposes secrets (REVIEWER_TOKEN, OLLAMA_HOST, OLLAMA_MODEL) into the
environment and uploads raw review_output.log; add a sanitization step that
redacts those env values before the artifact upload: either run review_ci.py
with sensitive envs unset (or scoped only to the process) and then run a
scrubber that replaces the literal values of
REVIEWER_TOKEN/OLLAMA_HOST/OLLAMA_MODEL in review_output.log (e.g., using a
short bash/python step that reads the three env vars and sed/regex-replaces them
with "[REDACTED]") and only upload the sanitized review_output.log in the
upload-artifact step (the one with if: always()); reference the env block,
review_ci.py, and review_output.log to locate where to add the scrubber/unset
logic.

# Map GitHub context vars to the names review_ci.py expects
CI_PROJECT_DIR: ${{ github.workspace }}
CI_PROJECT_ID: ${{ github.repository }} # e.g. hemusimple/AndroidVideoMotion
CI_MERGE_REQUEST_IID: ${{ github.event.pull_request.number }}
CI_MERGE_REQUEST_TARGET_BRANCH_NAME: ${{ github.event.pull_request.base.ref }}
CI_MERGE_REQUEST_SOURCE_BRANCH_NAME: ${{ github.event.pull_request.head.ref }}
CI_MERGE_REQUEST_TITLE: ${{ github.event.pull_request.title }}
CI_COMMIT_AUTHOR: ${{ github.event.pull_request.user.login }}

run: |
call D:\AIML\code-reviewer\myvenv\Scripts\activate.bat
pip install -q httpx python-dotenv pydantic pydantic-settings ollama loguru tenacity
python D:\AIML\code-reviewer\review_ci.py
exit 0
shell: cmd


- name: Upload review log
uses: actions/upload-artifact@v4
if: always()
with:
name: review-output
path: review_output.log
retention-days: 7
8 changes: 7 additions & 1 deletion ...aker/src/main/java/com/tejpratapsingh/lyricsmaker/presentation/activity/LyricsActivity.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import android.content.Context
import android.content.Intent
import android.os.Build
import android.os.Bundle
import android.util.Log
import com.google.android.material.dialog.MaterialAlertDialogBuilder
import com.tejpratapsingh.lyricsmaker.data.lrc.SyncedLyricFrame
import com.tejpratapsingh.lyricsmaker.presentation.motion.getLyricsVideoProducer
Expand Down Expand Up @@ -31,6 +32,7 @@ class LyricsActivity : PreviewActivity() {
lyrics: ArrayList<SyncedLyricFrame>,
socialMeta: SocialMeta? = null,
) {
Log.d(TAG,"start")

@gemini-code-assist gemini-code-assist Bot Apr 16, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

Add a space after the comma in the Log.d call to adhere to standard Kotlin style conventions and improve readability.

Suggested change
Log.d(TAG,"start")
Log.d(TAG, "start")

context.startActivity(
Intent(context, LyricsActivity::class.java).also {
it.putExtra(SONG, song)
Expand All @@ -41,6 +43,7 @@ class LyricsActivity : PreviewActivity() {
}
}


private val song: String
get() = intent.getStringExtra(SONG) ?: ""

Expand Down Expand Up @@ -71,7 +74,8 @@ class LyricsActivity : PreviewActivity() {

override fun onCreate(savedInstanceState: Bundle?) {
super.onCreate(savedInstanceState)

Log.d("LyricsActivity","onCreate")

@gemini-code-assist gemini-code-assist Bot Apr 16, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

high

  1. Use the TAG constant instead of a hardcoded string for consistency.
  2. Add a space after the comma in the Log.d call.
  3. Potential Crash: The lyrics list can be empty if the intent extra is missing. Calling minBy and maxBy on the subsequent lines (77-78) will throw a NoSuchElementException on an empty list. Added a safety check to finish the activity if no lyrics are provided.
Suggested change
Log.d("LyricsActivity","onCreate")
Log.d(TAG, "onCreate")
if (lyrics.isEmpty()) {
finish()
return
}

val data = getdata()
Comment on lines +77 to +78

@coderabbitai coderabbitai Bot Apr 16, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Verify whether `getdata()` actually exists anywhere and compare with `getData()`.
# Expected:
# - call-site(s) for getdata(...)
# - no definition for fun getdata(...) (if mismatch is real)
# - definition for fun getData(...)

rg -nP --type=kt '\bgetdata\s*\('
rg -nP --type=kt '\bfun\s+getdata\s*\('
rg -nP --type=kt '\bfun\s+getData\s*\('

Repository: tejpratap46/AndroidVideoMotion

Length of output: 154

🏁 Script executed:

#!/bin/bash
# Search for both method variants using glob patterns
echo "=== Searching for getdata() calls ==="
rg -nP '\.getdata\s*\(' --glob='*.kt'

echo -e "\n=== Searching for getData() calls ==="
rg -nP '\.getData\s*\(' --glob='*.kt'

echo -e "\n=== Searching for getdata() definitions ==="
rg -nP 'fun\s+getdata\s*\(' --glob='*.kt'

echo -e "\n=== Searching for getData() definitions ==="
rg -nP 'fun\s+getData\s*\(' --glob='*.kt'

echo -e "\n=== Reading LyricsActivity.kt to check line 77 and 105 ==="
if [ -f "modules/lyrics-maker/src/main/java/com/tejpratapsingh/lyricsmaker/presentation/activity/LyricsActivity.kt" ]; then
  echo "File found. Showing lines 70-110:"
  sed -n '70,110p' "modules/lyrics-maker/src/main/java/com/tejpratapsingh/lyricsmaker/presentation/activity/LyricsActivity.kt" | cat -n
else
  echo "File not found at expected path. Searching for LyricsActivity.kt..."
  fd -e kt -i 'lyricsactivity' 
fi

Repository: tejpratap46/AndroidVideoMotion

Length of output: 2058

Fix method name mismatch: getdata() does not exist. The method is defined as getData().

Line 77 calls getdata(), which will result in an unresolved reference. The class defines getData() at line 105. This will prevent compilation.

✅ Suggested fix 
-        Log.d("LyricsActivity","onCreate")
-        val data = getdata()
+        Log.d(TAG, "onCreate")
+        val data = getData()
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In
`@modules/lyrics-maker/src/main/java/com/tejpratapsingh/lyricsmaker/presentation/activity/LyricsActivity.kt`
around lines 76 - 77, In LyricsActivity onCreate you're calling a non-existent
getdata() which causes an unresolved reference; change the call to the
correctly-cased method getData() so replace getdata() with getData() in the
onCreate method to match the defined getData() function in the class.

val start = lyrics.minBy { it.frame }.frame
val end = lyrics.maxBy { it.frame }.frame

Expand All @@ -98,4 +102,6 @@ class LyricsActivity : PreviewActivity() {
}

override fun getMotionVideo(): MotionVideoProducer = video

fun getData() = "user data".toString()
}
Loading