Accumulated changes to the integration branch - #1596
Conversation
…G (It doesn't work)
Bumps [@babel/core](https://github.com/babel/babel/tree/HEAD/packages/babel-core) from 7.26.8 to 7.26.9. - [Release notes](https://github.com/babel/babel/releases) - [Changelog](https://github.com/babel/babel/blob/main/CHANGELOG.md) - [Commits](https://github.com/babel/babel/commits/v7.26.9/packages/babel-core) --- updated-dependencies: - dependency-name: "@babel/core" dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com>
Bumps [mongoose](https://github.com/Automattic/mongoose) from 8.10.0 to 8.10.1. - [Release notes](https://github.com/Automattic/mongoose/releases) - [Changelog](https://github.com/Automattic/mongoose/blob/master/CHANGELOG.md) - [Commits](Automattic/mongoose@8.10.0...8.10.1) --- updated-dependencies: - dependency-name: mongoose dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com>
Bumps nginxinc/nginx-unprivileged from 1.27.3-alpine to 1.27.4-alpine. --- updated-dependencies: - dependency-name: nginxinc/nginx-unprivileged dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com>
Bumps [@types/jasmine](https://github.com/DefinitelyTyped/DefinitelyTyped/tree/HEAD/types/jasmine) from 5.1.5 to 5.1.6. - [Release notes](https://github.com/DefinitelyTyped/DefinitelyTyped/releases) - [Commits](https://github.com/DefinitelyTyped/DefinitelyTyped/commits/HEAD/types/jasmine) --- updated-dependencies: - dependency-name: "@types/jasmine" dependency-type: direct:development update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com>
Bumps [neo4j-driver](https://github.com/neo4j/neo4j-javascript-driver) from 5.28.0 to 5.28.1. - [Release notes](https://github.com/neo4j/neo4j-javascript-driver/releases) - [Commits](neo4j/neo4j-javascript-driver@5.28.0...5.28.1) --- updated-dependencies: - dependency-name: neo4j-driver dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com>
Bumps nginxinc/nginx-unprivileged from 1.27.3-alpine to 1.27.4-alpine. --- updated-dependencies: - dependency-name: nginxinc/nginx-unprivileged dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com>
…MKG" courseId was undefined in the HTTP request.
Prepared the logging for the activity "User filtered top N concepts". It doesn't work yet.
…w/ understood/ not understood. -Fixed the problem of the activity logging of "User viewed recommended concepts" -edited the type of the recommended concepts. Previously they could be either main_concept/ related_concept or both. -Now the ones that are under recommended concepts Tab, they are considered as recommended_concept. -Added type to the concepts while marking as understood/ not understood/ new. -There is still a logging problem, by marking a concept from the recommended materials tab!
…mended concept" -Additionally, commented out some unnecessary console logs and added comments where needed.
Two activites are being logged by adding an annotation: "User added an annotation" and "User annotated a material". The annotation object can be "Note", "Question", "External Resource". The material object can be "pdf", "video", "Youtube". So the 6 possibilities are the following: "User added a note", "User added an external resource", "User asked a question", "User annotated a PDF", "User annotated a video", "User annotated a youtube video".
The activities include "User zoomed in a pdf", "User zoomed out a pdf", "User reset zoom in a pdf"
-Added comments for the previous implementation -Removed console logs.
The activity is "User viewed a Material's slide"
-Logged the Activity "User did not understand a slide", additionally to "User accessed Slide Kg"
Activities include: "User marked a notification as read", "User marked a notification as unread"
fixing a typo and adding some text
…/CourseMapper-webserver into william-recommendations
Activities include: "User viewed notifications " "User marked a notification/s as read", "User marked a notification/s as unread", "User starred a notification/s", "User unstarred a notification/s", "User deleted a notification/s"
Activities include: "User follow/ unfollow/ hid/ unhid/ filtered/ replied/ added/asked"
Implementation include the following activities: "User accessed Slide Knowledge Graph", "User accessed Material Knowledge Graph", "User accessed Course Knowledge Graph", "User did not understand a Slide".
postgres:16 image no longer requires Debian archive hacks
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
coursemapper-kg/wp-pg/Dockerfile (4)
15-15: Ensure the custom entrypoint is executable at build timeMake the permission explicit to avoid relying on host file mode.
Apply this diff:
-COPY bin/entrypoint.sh /usr/local/bin/custom-entrypoint.sh +COPY --chmod=0755 bin/entrypoint.sh /usr/local/bin/custom-entrypoint.sh
18-18: Reconsider rclone config placement and ownershipCopying to
/rootlikely won’t be readable if the process runs aspostgres. Prefer a well-known location and proper ownership, e.g./etc/rclone/rclone.conf(system-wide) or$PGUSER’s XDG config (/var/lib/postgresql/.config/rclone/rclone.conf).Minimal, system-wide approach (requires the directory):
+# create system rclone config dir +RUN install -d -m 0755 /etc/rclone -COPY config/rclone.conf.orig /root +COPY --chmod=0644 config/rclone.conf.orig /etc/rclone/rclone.confIf you keep a per-user config instead, create the tree and set ownership:
RUN install -d -o postgres -g postgres -m 0700 /var/lib/postgresql/.config/rclone COPY --chown=postgres:postgres --chmod=0600 config/rclone.conf.orig /var/lib/postgresql/.config/rclone/rclone.conf
20-20: Add a container HEALTHCHECKHelps orchestration detect readiness/liveness; matches Checkov hint CKV_DOCKER_2.
Apply this diff:
ENTRYPOINT ["/usr/local/bin/custom-entrypoint.sh"] +HEALTHCHECK --interval=30s --timeout=5s --start-period=30s --retries=5 \ + CMD pg_isready -U "${POSTGRES_USER:-postgres}" -d "${POSTGRES_DB:-postgres}" || exit 1
15-20: Run as non-root (or drop privileges) to satisfy least-privilege and Checkov CKV_DOCKER_3If your entrypoint doesn’t need root, explicitly run as
postgres. Otherwise, ensure the script drops privileges (e.g.,exec gosu postgres …) before starting Postgres.One-option diff (if root not required):
-COPY --chmod=0755 bin/entrypoint.sh /usr/local/bin/custom-entrypoint.sh -ENTRYPOINT ["/usr/local/bin/custom-entrypoint.sh"] +COPY --chmod=0755 bin/entrypoint.sh /usr/local/bin/custom-entrypoint.sh +USER postgres +ENTRYPOINT ["/usr/local/bin/custom-entrypoint.sh"]If root is required during init, keep USER as root but drop privileges within the script before launching Postgres.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
coursemapper-kg/wp-pg/Dockerfile(1 hunks)
🧰 Additional context used
🪛 Checkov (3.2.334)
coursemapper-kg/wp-pg/Dockerfile
[LOW] 1-20: Ensure that HEALTHCHECK instructions have been added to container images
(CKV_DOCKER_2)
[LOW] 1-20: Ensure that a user for the container has been created
(CKV_DOCKER_3)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Build container images / Build image (coursemapper-kg/preprocess)
- GitHub Check: Build container images / Build image (coursemapper-kg/concept-map)
- GitHub Check: Build container images / Build image (webapp)
🔇 Additional comments (2)
coursemapper-kg/wp-pg/Dockerfile (2)
1-1: Dockerfile syntax 1.15: good upgrade; ensure your builders default to BuildKitAdopting 1.15 unlocks heredocs and cache mounts used below. Confirm your CI/builders run with DOCKER_BUILDKIT=1 (most do by default now).
15-20: Overriding Postgres ENTRYPOINT: verify you still initialize the cluster and drop privilegesReplacing the official
docker-entrypoint.shcan bypass PG init scripts, env var handling (e.g., POSTGRES_DB/USER/PASSWORD), and the privilege drop topostgres. Ensure yourcustom-entrypoint.sheither:
- chains to the official entrypoint (recommended), or
- fully replicates its semantics and drops to the
postgresuser (via gosu) before starting the server.If you intend to chain, a common pattern inside your script is:
exec /usr/local/bin/docker-entrypoint.sh "$@"Optionally, if you prefer to run the container explicitly as a non-root user, add before ENTRYPOINT:
+USER postgresConfirm your script does not require root if you set USER.
|
|
||
| # Install dependencies | ||
| ENV RUNTIME_DEPS "ca-certificates rclone" | ||
| ENV RUNTIME_DEPS="ca-certificates rclone" |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Use ARG for build-time deps to avoid leaking ENV into the runtime image
RUNTIME_DEPS is only needed during build; prefer ARG so it doesn’t become a runtime env var.
Apply this diff:
-ENV RUNTIME_DEPS="ca-certificates rclone"
+ARG RUNTIME_DEPS="ca-certificates rclone"📝 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.
| ENV RUNTIME_DEPS="ca-certificates rclone" | |
| ARG RUNTIME_DEPS="ca-certificates rclone" |
🤖 Prompt for AI Agents
In coursemapper-kg/wp-pg/Dockerfile around line 5, RUNTIME_DEPS is declared with
ENV which leaks build-only packages into the runtime image; change ENV
RUNTIME_DEPS="ca-certificates rclone" to an ARG declaration (e.g., ARG
BUILD_DEPS="ca-certificates rclone") and update subsequent references in the
Dockerfile to use that ARG during build steps (apt-get install, cleanup, etc.)
so the variable is not present as a runtime environment variable.
| RUN --mount=type=cache,sharing=private,target=/var/cache/apt \ | ||
| --mount=type=cache,sharing=private,target=/var/lib/apt <<EOF | ||
| rm -f /etc/apt/apt.conf.d/docker-clean | ||
| echo 'Binary::apt::APT::Keep-Downloaded-Packages "true";' > /etc/apt/apt.conf.d/keep-cache | ||
| DEBIAN_FRONTEND=noninteractive apt-get update -q && | ||
| apt-get --reinstall install debian-archive-keyring && | ||
| apt-get install -qq --no-install-recommends -o=Dpkg::Use-Pty=0 $RUNTIME_DEPS | ||
| EOF |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Harden the APT block: fail fast, noninteractive install, and don’t persist keep-cache config
- Add
set -euxto fail early. - Use
-yfor install (update doesn’t take-y). - Remove
/etc/apt/apt.conf.d/keep-cacheafter install so the runtime image isn’t left with build-time APT behavior. - Optional: explicitly clear lists (mostly moot with cache mounts, but harmless).
Apply this diff:
RUN --mount=type=cache,sharing=private,target=/var/cache/apt \
--mount=type=cache,sharing=private,target=/var/lib/apt <<EOF
- rm -f /etc/apt/apt.conf.d/docker-clean
- echo 'Binary::apt::APT::Keep-Downloaded-Packages "true";' > /etc/apt/apt.conf.d/keep-cache
- DEBIAN_FRONTEND=noninteractive apt-get update -q &&
- apt-get install -qq --no-install-recommends -o=Dpkg::Use-Pty=0 $RUNTIME_DEPS
+ set -eux
+ rm -f /etc/apt/apt.conf.d/docker-clean
+ echo 'Binary::apt::APT::Keep-Downloaded-Packages "true";' > /etc/apt/apt.conf.d/keep-cache
+ DEBIAN_FRONTEND=noninteractive apt-get update -q
+ apt-get install -y -qq --no-install-recommends -o=Dpkg::Use-Pty=0 $RUNTIME_DEPS
+ rm -f /etc/apt/apt.conf.d/keep-cache
+ rm -rf /var/lib/apt/lists/*
EOF📝 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.
| RUN --mount=type=cache,sharing=private,target=/var/cache/apt \ | |
| --mount=type=cache,sharing=private,target=/var/lib/apt <<EOF | |
| rm -f /etc/apt/apt.conf.d/docker-clean | |
| echo 'Binary::apt::APT::Keep-Downloaded-Packages "true";' > /etc/apt/apt.conf.d/keep-cache | |
| DEBIAN_FRONTEND=noninteractive apt-get update -q && | |
| apt-get --reinstall install debian-archive-keyring && | |
| apt-get install -qq --no-install-recommends -o=Dpkg::Use-Pty=0 $RUNTIME_DEPS | |
| EOF | |
| RUN --mount=type=cache,sharing=private,target=/var/cache/apt \ | |
| --mount=type=cache,sharing=private,target=/var/lib/apt <<EOF | |
| set -eux | |
| rm -f /etc/apt/apt.conf.d/docker-clean | |
| echo 'Binary::apt::APT::Keep-Downloaded-Packages "true";' > /etc/apt/apt.conf.d/keep-cache | |
| DEBIAN_FRONTEND=noninteractive apt-get update -q | |
| apt-get install -y -qq --no-install-recommends -o=Dpkg::Use-Pty=0 $RUNTIME_DEPS | |
| rm -f /etc/apt/apt.conf.d/keep-cache | |
| rm -rf /var/lib/apt/lists/* | |
| EOF |
🤖 Prompt for AI Agents
In coursemapper-kg/wp-pg/Dockerfile around lines 6 to 12, the APT block should
be hardened: add a shell fail-fast and verbose header (set -eux) at the top of
the heredoc, use -y on apt-get install (DEBIAN_FRONTEND=noninteractive apt-get
install -y ... -qq --no-install-recommends -o=Dpkg::Use-Pty=0 $RUNTIME_DEPS),
and ensure we remove the temporary keep-cache file after installation (rm -f
/etc/apt/apt.conf.d/keep-cache) so runtime image isn’t left with build-time
behavior; optionally clear apt lists after install (rm -rf /var/lib/apt/lists/*)
to avoid persisting package lists (harmless with cache mounts).
postgres:16 image no longer requires Debian archive hacks
hide recommended concept tab
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
webapp/src/app/pages/components/knowledge-graph/concept-map/concept-map.component.ts (1)
1537-1566: Restore button state on error/complete of materials call
isRecommendationButtonDisplayedis reset only on success. Add resets on error/complete to avoid stuck UI..subscribe({ next: (result) => { this.isRecommendationButtonDisplayed = true; this.resourcesPagination = result; this.kgTabs.kgTabsEnable(); this.mainConceptsTab = false; this.recommendedConceptsTab = false; this.recommendedMaterialsTab = true; }, - complete: () => { - this.showRecommendationButtonClicked = false; - }, + complete: () => { + this.isRecommendationButtonDisplayed = true; + this.showRecommendationButtonClicked = false; + }, - }); + }, + error: (err) => { + this.isRecommendationButtonDisplayed = true; + this.showRecommendationButtonClicked = false; + console.error(err); + this.displayMessage(err.message); + this.isLoading = false; + this.loading.emit(false); + });
♻️ Duplicate comments (3)
webapp/src/app/pages/components/knowledge-graph/concept-map/concept-map.component.ts (3)
901-905: Don’t hard‑code graph width to 75%; compute responsivelyUse container/sidebar widths instead of a magic percentage. This aligns with prior feedback.
- setResponsiveWidthKnowledgeGraph(knowledgeGraph) { - if (knowledgeGraph && knowledgeGraph.style) { - knowledgeGraph.style.width = '75%'; - } - } + setResponsiveWidthKnowledgeGraph(knowledgeGraph: HTMLElement | null) { + if (!knowledgeGraph) return; + const container = document.getElementById('slideKgDialogDiv') as HTMLElement | null; + const sidebar = document.getElementById('flexboxNotUnderstood') as HTMLElement | null; + const margin = this.showConceptsListSidebar ? 16 : 0; // 1rem + const containerWidth = + container?.offsetWidth ?? knowledgeGraph.parentElement?.clientWidth ?? window.innerWidth; + const sidebarWidth = this.showConceptsListSidebar && sidebar ? sidebar.offsetWidth : 0; + knowledgeGraph.style.width = Math.max(0, containerWidth - sidebarWidth - margin) + 'px'; + }
2295-2298: MakecroUpdaterrobust (early return + defaults)Avoid silent no‑ops and undefined param flows. Mirrors earlier guidance to guard
croComponent.- croUpdater(didNotUnderstandConceptsObj: any[], previousConceptsObj: any[]) { - this.croComponent?.updateCROformAll(didNotUnderstandConceptsObj, previousConceptsObj); - } + croUpdater(didNotUnderstandConceptsObj: any[] = [], previousConceptsObj: any[] = []) { + if (!this.croComponent) return; + this.croComponent.updateCROformAll(didNotUnderstandConceptsObj, previousConceptsObj); + }
2308-2324: Remove console noise and avoidevent.screen; delegate to responsive helperUse
window.innerWidthand reusesetResponsiveWidthKnowledgeGraph. Also drop magic40em.- setWeightGraphComponent(event) { + setWeightGraphComponent(event: UIEvent) { setTimeout(() => { - let knowledgeGraph = document.getElementById('graphSection'); - if (this.showMaterialKg) { - console.warn("resize -> hideConceptsList -> HostListener event.screen ->", event.screen.width); - // let screenWidth = window.innerHeight; - let screenWidth = event.screen.width; // event.outerWidth - - if (screenWidth >= 768 && screenWidth < 992) { - knowledgeGraph.style.width = '40em'; - } - if (screenWidth > 992 && screenWidth <= 1200) { - knowledgeGraph.style.width = '40em'; - } - } + const knowledgeGraph = document.getElementById('graphSection') as HTMLElement | null; + if (!knowledgeGraph || !this.showMaterialKg) return; + const screenWidth = window.innerWidth; + if (screenWidth >= 768 && screenWidth < 1200) { + this.setResponsiveWidthKnowledgeGraph(knowledgeGraph); + } }, 3); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
webapp/src/app/pages/components/knowledge-graph/concept-map/concept-map.component.ts(25 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
webapp/src/app/pages/components/knowledge-graph/concept-map/concept-map.component.ts (1)
webapp/src/app/models/croForm.ts (2)
ActivatorPartCRO(4-8)ResourcesPagination(66-73)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Build container images / Build image (coursemapper-kg/preprocess)
- GitHub Check: Build container images / Build image (coursemapper-kg/concept-map)
- GitHub Check: Build container images / Build image (webapp)
- GitHub Check: Build container images / Build image (coursemapper-kg/recommendation)
🔇 Additional comments (1)
webapp/src/app/pages/components/knowledge-graph/concept-map/concept-map.component.ts (1)
1490-1490: Verify necessity of PII in recommender requestsBefore shipping, please confirm whether the
userId(anduser_id) fields you’re appending to the concept- and material-recommender payloads are actually consumed by the backend. If they’re not used server-side, remove them to avoid leaking unnecessary PII.• webapp/src/app/pages/components/knowledge-graph/concept-map/concept-map.component.ts (line 1489)
reqDataMaterial1["userId"] = this.userid;• webapp/src/app/pages/components/knowledge-graph/result-view/result-view.component.ts (lines 321 & 342)
this.filteringParamsSavedTab.user_id = this.userId;• webapp/src/app/pages/components/knowledge-graph/custom-recommendation-option/custom-recommendation-option.component.ts (line 141)
this.croForm["user_id"] = this.userId;
| resourcesPagination: ResourcesPagination = undefined; | ||
| isRecommendationButtonDisplayed = true; | ||
| conceptsUpdatedCRO: any; |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Prefer explicit nullable type for resourcesPagination
Assigning undefined to a non‑nullable type weakens type safety.
- resourcesPagination: ResourcesPagination = undefined;
+ resourcesPagination: ResourcesPagination | null = null;📝 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.
| resourcesPagination: ResourcesPagination = undefined; | |
| isRecommendationButtonDisplayed = true; | |
| conceptsUpdatedCRO: any; | |
| resourcesPagination: ResourcesPagination | null = null; | |
| isRecommendationButtonDisplayed = true; | |
| conceptsUpdatedCRO: any; |
🤖 Prompt for AI Agents
In
webapp/src/app/pages/components/knowledge-graph/concept-map/concept-map.component.ts
around lines 272 to 274, the field resourcesPagination is currently declared as
ResourcesPagination and assigned undefined which weakens type safety; change its
declaration to an explicit nullable type (ResourcesPagination | undefined) or
ResourcesPagination | null and keep the undefined assignment (or initialize to
null) so the compiler understands the field may be absent and enforces checks
where it is used.
| onActiveItemChange(event: MenuItem) { | ||
| // console.warn("tab onActiveItemChange") // graphSection | ||
| if (event.label === 'Main Concepts') { | ||
| // console.warn("Main Concepts") | ||
| } else if (event.label === 'Recommended Concepts') { | ||
| // console.warn("Recommended Concepts") | ||
| } else { | ||
| } | ||
|
|
||
| } |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Clean up no‑op handler and commented logs
Either implement tab‑change side effects or keep a minimal no‑op.
- onActiveItemChange(event: MenuItem) {
- // console.warn("tab onActiveItemChange") // graphSection
- if (event.label === 'Main Concepts') {
- // console.warn("Main Concepts")
- } else if (event.label === 'Recommended Concepts') {
- // console.warn("Recommended Concepts")
- } else {
- }
-
- }
+ onActiveItemChange(_event: MenuItem): void {
+ // No-op for now.
+ }📝 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.
| onActiveItemChange(event: MenuItem) { | |
| // console.warn("tab onActiveItemChange") // graphSection | |
| if (event.label === 'Main Concepts') { | |
| // console.warn("Main Concepts") | |
| } else if (event.label === 'Recommended Concepts') { | |
| // console.warn("Recommended Concepts") | |
| } else { | |
| } | |
| } | |
| onActiveItemChange(_event: MenuItem): void { | |
| // No-op for now. | |
| } |
🤖 Prompt for AI Agents
In
webapp/src/app/pages/components/knowledge-graph/concept-map/concept-map.component.ts
around lines 810 to 819, the onActiveItemChange handler contains only commented
console logs and empty conditional branches; remove the commented logs and the
empty else branch and either implement the needed tab-change side effects or
convert the function to a minimal no-op that clearly does nothing (e.g., keep a
single-line comment like "// no-op: tab changes handled elsewhere" and return),
ensuring the method is concise and has no unused branches or stray commented
code.
| this.tabIndex = 2; | ||
| this.showRecommendationButtonClicked = true; |
There was a problem hiding this comment.
Fix tab index: index 2 doesn’t exist with current tabs
Only two tabs remain (“Main Concepts”, “Recommended Materials”). Selecting index 2 is out of bounds and may misroute UI state.
- this.tabIndex = 2;
+ this.tabIndex = 1;📝 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.
| this.tabIndex = 2; | |
| this.showRecommendationButtonClicked = true; | |
| this.tabIndex = 1; | |
| this.showRecommendationButtonClicked = true; |
🤖 Prompt for AI Agents
In
webapp/src/app/pages/components/knowledge-graph/concept-map/concept-map.component.ts
around lines 1478 to 1479, the code sets this.tabIndex = 2 which is out of range
because only two tabs exist; update the assignment to use a valid index (set to
1 for the "Recommended Materials" tab) or compute it dynamically (e.g.
this.tabIndex = Math.min(desiredIndex, this.tabs.length - 1)) so tabIndex cannot
exceed available tabs; keep this.showRecommendationButtonClicked = true as-is.
| let reqDataFinal = this.croComponent.buildFinalRequestRecMaterial(reqData); | ||
|
|
There was a problem hiding this comment.
Null‑guard CRO before building request (fallback to reqData)
croComponent may be undefined; avoid runtime errors.
- let reqDataFinal = this.croComponent.buildFinalRequestRecMaterial(reqData);
+ const reqDataFinal = this.croComponent
+ ? this.croComponent.buildFinalRequestRecMaterial(reqData)
+ : reqData;📝 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.
| let reqDataFinal = this.croComponent.buildFinalRequestRecMaterial(reqData); | |
| const reqDataFinal = this.croComponent | |
| ? this.croComponent.buildFinalRequestRecMaterial(reqData) | |
| : reqData; |
🤖 Prompt for AI Agents
In
webapp/src/app/pages/components/knowledge-graph/concept-map/concept-map.component.ts
around lines 1535-1536, the code calls
this.croComponent.buildFinalRequestRecMaterial(reqData) without guarding against
croComponent being undefined; add a null-guard so you only call
buildFinalRequestRecMaterial when croComponent exists and otherwise fall back to
using reqData (e.g. compute reqDataFinal = this.croComponent ?
this.croComponent.buildFinalRequestRecMaterial(reqData) : reqData), ensuring
types are adjusted or cast if necessary.
| setHeightGraphComponent() { | ||
| let knowledgeGraph = document.getElementById('graphSection'); | ||
| if (knowledgeGraph) { | ||
| let ipo_interact = document.getElementById('ipo_interact'); | ||
| // console.warn("ipo_interact with -> ", ipo_interact.offsetWidth) | ||
| this.cyHeight = ipo_interact.offsetHeight - (ipo_interact.offsetHeight * 0.15); | ||
| } | ||
| } |
There was a problem hiding this comment.
Null‑guard DOM node before using offsetHeight
ipo_interact may be null; prevent NPE.
- setHeightGraphComponent() {
- let knowledgeGraph = document.getElementById('graphSection');
- if (knowledgeGraph) {
- let ipo_interact = document.getElementById('ipo_interact');
- // console.warn("ipo_interact with -> ", ipo_interact.offsetWidth)
- this.cyHeight = ipo_interact.offsetHeight - (ipo_interact.offsetHeight * 0.15);
- }
- }
+ setHeightGraphComponent() {
+ const knowledgeGraph = document.getElementById('graphSection');
+ if (!knowledgeGraph) return;
+ const container = document.getElementById('ipo_interact') as HTMLElement | null;
+ if (container) {
+ this.cyHeight = container.offsetHeight * 0.85;
+ }
+ }📝 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.
| setHeightGraphComponent() { | |
| let knowledgeGraph = document.getElementById('graphSection'); | |
| if (knowledgeGraph) { | |
| let ipo_interact = document.getElementById('ipo_interact'); | |
| // console.warn("ipo_interact with -> ", ipo_interact.offsetWidth) | |
| this.cyHeight = ipo_interact.offsetHeight - (ipo_interact.offsetHeight * 0.15); | |
| } | |
| } | |
| setHeightGraphComponent() { | |
| const knowledgeGraph = document.getElementById('graphSection'); | |
| if (!knowledgeGraph) return; | |
| const container = document.getElementById('ipo_interact') as HTMLElement | null; | |
| if (container) { | |
| this.cyHeight = container.offsetHeight * 0.85; | |
| } | |
| } |
🤖 Prompt for AI Agents
In
webapp/src/app/pages/components/knowledge-graph/concept-map/concept-map.component.ts
around lines 2299 to 2306, the code uses ipo_interact.offsetHeight without
guarding against ipo_interact being null; update the method to first check that
ipo_interact exists (e.g., if (ipo_interact) { ... }) before accessing
offsetHeight, and handle the null case by either returning early or using a safe
fallback height value so no NPE occurs.
* nber recommendations and search box activated * support for large screen --------- Co-authored-by: boby024 <williamkana46@gmail.com>
There was a problem hiding this comment.
Actionable comments posted: 27
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
coursemapper-kg/recommendation/app/services/course_materials/db/neo4_db.py (3)
373-383: User fetch/creation bugs: incorrect return handling and wrong function call signature.
get_useruses.single()thenlist(result): raises when result is None and returns meaningless list otherwise.get_or_create_usercallscreate_user(tx, user_id, username, user_email), butcreate_useraccepts only(tx, user_id).-def get_user(tx, uid): +def get_user(tx, uid): @@ - result = tx.run( - "MATCH (u:User) WHERE u.uid = $uid RETURN u", - uid=uid).single() - - return list(result) + record = tx.run( + "MATCH (u:User) WHERE u.uid = $uid RETURN u.uid AS uid", + uid=uid).single() + return record["uid"] if record else Nonedef get_or_create_user(self, user_id, username="", user_email=""): @@ - _uid = get_user(tx, user_id) - if _uid: - return _uid - else: - # print(tx) - create_user(tx, user_id, username, user_email) - tx.commit() + uid = get_user(tx, user_id) + if uid: + tx.commit() + return uid + create_user(tx, user_id) + tx.commit()Also applies to: 749-768
716-725: Wrong argument order when creating user–concept relationship.
create_user_concept_relationshipsexpects(tx, uid, cid, relation_type), but you pass(tx, concept_id, user_id, ...).- create_user_concept_relationships(tx, concept_id, user_id, relation_type) + create_user_concept_relationships(tx, user_id, concept_id, relation_type)
1780-1788: Invalidsession.commit()usage; run in explicit transaction or use context manager.
Sessionhas nocommit(). Use a transaction or implicit auto-commit viasession.run.- session = self.driver.session() - session.run( - """Match (c:Concept) WHERE c.cid=$cid and c.mid=$mid SET c.rank=$rank """, - cid=node["id"], - mid=node["mid"], - rank=node["rank"]) - session.commit() - session.close() + with self.driver.session() as session: + session.run( + """MATCH (c:Concept) WHERE c.cid=$cid AND c.mid=$mid SET c.rank=$rank""", + cid=node["id"], + mid=node["mid"], + rank=node["rank"], + )
♻️ Duplicate comments (13)
webapp/src/app/pages/components/knowledge-graph/videos/card-video-list/card-video-list.component.html (1)
3-14: AddtrackByand remove the wrapper div to cut DOM churnMove
*ngForonto the card component and providetrackByfor efficient re-rendering.- <div class="" *ngFor="let videoElement of videoElements"> - <app-card-video + <app-card-video + *ngFor="let videoElement of videoElements; trackBy: trackByRid" [notUnderstoodConcepts]="notUnderstoodConcepts" (onWatchVideo)="readVideo($event)" [videoElement]="videoElement" [userId]="userId" [resultTabType]="resultTabType" (resourceRemovedEvent)="onResourceRemovedEvent($event)" [currentMaterial]="currentMaterial" > </app-card-video> - </div>Note: Implement
trackByRid(_: number, el: VideoElementModel) { return el.rid; }in the component TS.webapp/src/app/pages/components/knowledge-graph/result-view/result-view.component.html (3)
57-60: Duplicate id "cro_sorting" on siblings.Keep ids unique or remove them if not needed. This was flagged earlier as well.
- <div id="cro_sorting" class="w-3/12"></div> + <div class="w-3/12"></div> - <div id="cro_sorting" class="w-3/12" (click)="op.toggle($event, targetEl)"> + <div id="cro_sorting" class="w-3/12" (click)="op.toggle($event, targetEl)">
116-135: *Fix Angular ngIf microsyntax and render logic for Videos.
- Use semicolon in microsyntax (previously reported).
- Ensure the list renders only when items exist; otherwise show empty/loading states.
- <div class="mt-1 w-full" id="cro_viedo_list"> - <div - *ngIf="resourcesPagination?.nodes?.videos?.total_items > 0, else elseBlockRsVideos" - > - </div> - <p-scrollPanel styleClass="scroll_panel_list"> - <app-card-video-list - [notUnderstoodConcepts]="concepts" - [videoElements]="resourcesPagination?.nodes?.videos?.content" - [userId]="userId" - (backButtonClicked)="logUserViewedRecommendedVideos()" - [currentMaterial]="currentMaterial" - ></app-card-video-list> - </p-scrollPanel> - </div> + <div class="mt-1 w-full"> + <ng-container *ngIf="resourcesPagination?.nodes?.videos?.total_items > 0; else elseBlockRsVideos"> + <p-scrollPanel styleClass="scroll_panel_list"> + <app-card-video-list + [notUnderstoodConcepts]="concepts" + [videoElements]="resourcesPagination?.nodes?.videos?.content" + [userId]="userId" + (backButtonClicked)="logUserViewedRecommendedVideos()" + [currentMaterial]="currentMaterial" + ></app-card-video-list> + </p-scrollPanel> + </ng-container> + </div>Also applies to: 136-156
159-175: *Fix Angular ngIf microsyntax and render logic for Articles.Mirror the Videos fix; also remove duplicate/typo id.
- <div class="mt-1 w-full" id="cro_viedo_list"> - <div - *ngIf="resourcesPagination?.nodes?.articles?.total_items > 0, else elseBlockRsArticles" - > - </div> - <p-scrollPanel styleClass="scroll_panel_list"> - <app-card-article-list - [notUnderstoodConcepts]="concepts" - [articleElements]="resourcesPagination?.nodes?.articles?.content" - [userId]="userId" - [currentMaterial]="currentMaterial" - ></app-card-article-list> - </p-scrollPanel> - </div> + <div class="mt-1 w-full"> + <ng-container *ngIf="resourcesPagination?.nodes?.articles?.total_items > 0; else elseBlockRsArticles"> + <p-scrollPanel styleClass="scroll_panel_list"> + <app-card-article-list + [notUnderstoodConcepts]="concepts" + [articleElements]="resourcesPagination?.nodes?.articles?.content" + [userId]="userId" + [currentMaterial]="currentMaterial" + ></app-card-article-list> + </p-scrollPanel> + </ng-container> + </div>Also applies to: 176-196
webapp/src/app/pages/components/knowledge-graph/concept-map/concept-map.component.ts (8)
911-913: Stop manually setting width/margin; use the helperInline writes fight the helper and cause inconsistent layouts.
- knowledgeGraph.style.marginLeft = 0 + 'rem'; - knowledgeGraph.style.width = slideKgDialogDiv.offsetWidth + 'px'; + this.setResponsiveWidthKnowledgeGraph();
270-275: FixresourcesPaginationnullability (assigned undefined/null to non-nullable type)Make the type explicitly nullable and initialize consistently.
- resourcesPagination: ResourcesPagination = undefined; + resourcesPagination: ResourcesPagination | null = null;(No further code changes needed where you already set it to
nullin Line 1450.)Also applies to: 1450-1451
810-819: Trim no-op tab-change handlerConvert to a minimal no-op; remove commented logs and empty branches.
- onActiveItemChange(event: MenuItem) { - // console.warn("tab onActiveItemChange") // graphSection - if (event.label === 'Main Concepts') { - // console.warn("Main Concepts") - } else if (event.label === 'Recommended Concepts') { - // console.warn("Recommended Concepts") - } else { - } - - } + onActiveItemChange(_event: MenuItem): void { + // No-op (tab side effects handled elsewhere) + }
843-853: Avoid hard-coded widths; compute width from container/sidebar; remove debug log75%/85% breaks responsiveness and diverges from sidebar sizing. Compute based on container and sidebar widths.
- setResponsiveWidthKnowledgeGraph() { - console.warn("window.innerWidth ", window.innerWidth) - let knowledgeGraph = document.getElementById('graphSection'); - if (knowledgeGraph && knowledgeGraph.style) { - if (window.innerWidth < 2700) { - knowledgeGraph.style.width = '75%'; - } else if (window.innerWidth > 2700) { - knowledgeGraph.style.width = '85%'; - } - } - } + setResponsiveWidthKnowledgeGraph(): void { + const graph = document.getElementById('graphSection') as HTMLElement | null; + const container = document.getElementById('slideKgDialogDiv') as HTMLElement | null; + const sidebar = document.getElementById('flexboxNotUnderstood') as HTMLElement | null; + if (!graph || !container) return; + const sidebarWidth = this.showConceptsListSidebar && sidebar ? sidebar.offsetWidth : 0; + const padding = 16; // 1rem margin when sidebar is visible + const width = Math.max(320, container.offsetWidth - sidebarWidth - (this.showConceptsListSidebar ? padding : 0)); + graph.style.marginLeft = this.showConceptsListSidebar ? '1rem' : '0'; + graph.style.width = `${width}px`; + }
893-901: Unify spacing with the helper; drop inlinemarginLeftwriteDelegate width/margin to
setResponsiveWidthKnowledgeGraphto prevent drift.- if (knowledgeGraph) { - knowledgeGraph.style.marginLeft = 1 + 'rem'; - } + // spacing handled in setResponsiveWidthKnowledgeGraph()
1487-1488: Fix tab index (2 is out of bounds for two tabs)Set to 1 for “Recommended Materials”.
- this.tabIndex = 2; + this.tabIndex = 1;
1542-1545: Null‑guardcroComponentbefore building request (fallback toreqData)Avoid runtime error when the child component isn’t initialized.
- let reqDataFinal = this.croComponent.buildFinalRequestRecMaterial(reqData); + const reqDataFinal = this.croComponent + ? this.croComponent.buildFinalRequestRecMaterial(reqData) + : reqData;
2308-2315: Null‑guardipo_interactbefore usingoffsetHeightPrevents NPE when the container is missing on first render.
- setHeightGraphComponent() { - let knowledgeGraph = document.getElementById('graphSection'); - if (knowledgeGraph) { - let ipo_interact = document.getElementById('ipo_interact'); - // console.warn("ipo_interact with -> ", ipo_interact.offsetWidth) - this.cyHeight = ipo_interact.offsetHeight - (ipo_interact.offsetHeight * 0.15); - } - } + setHeightGraphComponent(): void { + const container = document.getElementById('ipo_interact') as HTMLElement | null; + if (!container) return; + this.cyHeight = container.offsetHeight * 0.85; + }coursemapper-kg/recommendation/app/services/course_materials/db/neo4_db.py (1)
6-6: Remove unused import (duplicate of prior review).
import jsonisn’t used. Please delete it.- import json
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
coursemapper-kg/recommendation/app/services/course_materials/db/neo4_db.py(12 hunks)webapp/src/app/pages/components/knowledge-graph/concept-map/concept-map.component.ts(26 hunks)webapp/src/app/pages/components/knowledge-graph/result-view/result-view.component.html(1 hunks)webapp/src/app/pages/components/knowledge-graph/videos/card-video-list/card-video-list.component.html(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
coursemapper-kg/recommendation/app/services/course_materials/db/neo4_db.py (2)
webserver/src/graph/recommendation.neo4j.js (15)
result(27-27)result(79-79)result(186-192)result(204-204)result(307-307)result(323-323)result(344-344)result(363-363)resources(237-263)query(206-227)query(283-287)query(308-312)query(325-331)query(345-350)query(364-370)webserver/src/controllers/recommendation.controller.js (12)
result(9-9)result(20-20)result(34-34)result(45-45)result(90-90)result(104-104)result(116-116)result(129-129)data(7-7)data(18-18)data(32-32)cids(86-86)
webapp/src/app/pages/components/knowledge-graph/concept-map/concept-map.component.ts (1)
webapp/src/app/models/croForm.ts (2)
ActivatorPartCRO(4-8)ResourcesPagination(66-73)
🪛 Ruff (0.12.2)
coursemapper-kg/recommendation/app/services/course_materials/db/neo4_db.py
6-6: json imported but unused
Remove unused import: json
(F401)
129-129: Missing return type annotation for public function create_external_source_resource
Add return type annotation: None
(ANN201)
129-129: Missing type annotation for function argument tx
(ANN001)
129-129: Missing type annotation for function argument node
(ANN001)
134-134: Use format specifiers instead of percent format
Replace with format specifiers
(UP031)
134-134: Logging statement uses %
(G002)
493-493: Missing return type annotation for public function create_user_v2
(ANN201)
493-493: Missing type annotation for function argument tx
(ANN001)
493-493: Missing type annotation for function argument user
(ANN001)
503-503: Unnecessary assignment to user_id before return statement
Remove unnecessary assignment
(RET504)
1795-1795: Missing return type annotation for public function get_or_create_user_v2
(ANN201)
1795-1795: Missing type annotation for function argument user
(ANN001)
1816-1816: Missing return type annotation for public function create_or_update_video_resource
Add return type annotation: None
(ANN201)
1816-1816: Missing type annotation for function argument tx
(ANN001)
1817-1817: Missing type annotation for function argument recommendation_type
(ANN001)
1817-1817: Unused method argument: recommendation_type
(ARG002)
1818-1818: Boolean default positional argument in function definition
(FBT002)
1818-1818: Missing type annotation for function argument update_embedding_values
(ANN001)
1819-1819: Boolean default positional argument in function definition
(FBT002)
1819-1819: Missing type annotation for function argument update_detail_found
(ANN001)
1819-1819: Unused method argument: update_detail_found
(ARG002)
1855-1855: Avoid equality comparisons to True; use update_embedding_values: for truth checks
Replace with update_embedding_values
(E712)
1865-1865: Use node.get("keyphrases", []) instead of an if block
Replace with node.get("keyphrases", [])
(SIM401)
1866-1866: Use node.get("keyphrase_embedding", []) instead of an if block
Replace with node.get("keyphrase_embedding", [])
(SIM401)
1867-1867: Use node.get("document_embedding", []) instead of an if block
Replace with node.get("document_embedding", [])
(SIM401)
1868-1868: Use node.get("keyphrases_infos", "") instead of an if block
Replace with node.get("keyphrases_infos", "")
(SIM401)
1889-1889: Use format specifiers instead of percent format
Replace with format specifiers
(UP031)
1893-1893: Use format specifiers instead of percent format
Replace with format specifiers
(UP031)
1899-1899: Use node.get("keyphrases", []) instead of an if block
Replace with node.get("keyphrases", [])
(SIM401)
1900-1900: Use node.get("keyphrase_embedding", []) instead of an if block
Replace with node.get("keyphrase_embedding", [])
(SIM401)
1901-1901: Use node.get("document_embedding", []) instead of an if block
Replace with node.get("document_embedding", [])
(SIM401)
1902-1902: Use node.get("helpful_count", 0) instead of an if block
Replace with node.get("helpful_count", 0)
(SIM401)
1903-1903: Use node.get("not_helpful_count", 0) instead of an if block
Replace with node.get("not_helpful_count", 0)
(SIM401)
1904-1904: Use node.get("saves_count", 0) instead of an if block
Replace with node.get("saves_count", 0)
(SIM401)
1907-1907: datetime.datetime.now() called without a tz argument
(DTZ005)
1908-1908: Use node.get("keyphrases_infos", "") instead of an if block
Replace with node.get("keyphrases_infos", "")
(SIM401)
1910-1910: Do not catch blind exception: Exception
(BLE001)
1912-1912: Unnecessary pass statement
Remove unnecessary pass
(PIE790)
1914-1914: Missing return type annotation for public function create_or_update_wikipedia_resource
Add return type annotation: None
(ANN201)
1914-1914: Missing type annotation for function argument tx
(ANN001)
1914-1914: Missing type annotation for function argument node
(ANN001)
1914-1914: Missing type annotation for function argument recommendation_type
(ANN001)
1914-1914: Unused method argument: recommendation_type
(ARG002)
1915-1915: Boolean default positional argument in function definition
(FBT002)
1915-1915: Missing type annotation for function argument update_embedding_values
(ANN001)
1916-1916: Boolean default positional argument in function definition
(FBT002)
1916-1916: Missing type annotation for function argument update_detail_found
(ANN001)
1916-1916: Unused method argument: update_detail_found
(ARG002)
1944-1944: Avoid equality comparisons to True; use update_embedding_values: for truth checks
Replace with update_embedding_values
(E712)
1953-1953: Use node.get("keyphrases", []) instead of an if block
Replace with node.get("keyphrases", [])
(SIM401)
1954-1954: Use node.get("keyphrase_embedding", []) instead of an if block
Replace with node.get("keyphrase_embedding", [])
(SIM401)
1955-1955: Use node.get("document_embedding", []) instead of an if block
Replace with node.get("document_embedding", [])
(SIM401)
1956-1956: Use node.get("keyphrases_infos", "") instead of an if block
Replace with node.get("keyphrases_infos", "")
(SIM401)
1976-1976: Use node.get("keyphrases", []) instead of an if block
Replace with node.get("keyphrases", [])
(SIM401)
1979-1979: Use node.get("keyphrase_embedding", []) instead of an if block
Replace with node.get("keyphrase_embedding", [])
(SIM401)
1980-1980: Use node.get("document_embedding", []) instead of an if block
Replace with node.get("document_embedding", [])
(SIM401)
1981-1981: Use node.get("helpful_count", 0) instead of an if block
Replace with node.get("helpful_count", 0)
(SIM401)
1982-1982: Use node.get("not_helpful_count", 0) instead of an if block
Replace with node.get("not_helpful_count", 0)
(SIM401)
1983-1983: Use node.get("saves_count", 0) instead of an if block
Replace with node.get("saves_count", 0)
(SIM401)
1984-1984: datetime.datetime.now() called without a tz argument
(DTZ005)
1985-1985: Use node.get("keyphrases_infos", "") instead of an if block
Replace with node.get("keyphrases_infos", "")
(SIM401)
1987-1987: Do not catch blind exception: Exception
(BLE001)
1989-1989: Unnecessary pass statement
Remove unnecessary pass
(PIE790)
1991-1991: Missing return type annotation for public function get_top_n_concept_by_slide_id
(ANN201)
1991-1991: PEP 484 prohibits implicit Optional
Convert to Optional[T]
(RUF013)
1991-1991: Missing type annotation for function argument top_n
(ANN001)
2026-2026: Missing return type annotation for public function create_concept_modified
(ANN201)
2055-2094: Missing explicit return at the end of function able to return non-None value
Add explicit return statement
(RET503)
2055-2055: Missing return type annotation for public function update_rs_btw_user_and_cm
(ANN201)
2055-2055: Boolean default positional argument in function definition
(FBT002)
2055-2055: Missing type annotation for function argument only_status
(ANN001)
2062-2062: Avoid equality comparisons to True; use only_status: for truth checks
Replace with only_status
(E712)
2094-2094: Unnecessary assignment to r_detail before return statement
Remove unnecessary assignment
(RET504)
2096-2096: Missing return type annotation for public function update_rs_btw_user_and_cms
Add return type annotation: None
(ANN201)
2096-2096: Missing type annotation for function argument special_status
(ANN001)
2096-2096: Unused method argument: special_status
(ARG002)
2104-2104: Local variable result is assigned to but never used
Remove assignment to unused variable result
(F841)
2114-2114: Missing return type annotation for public function get_user_embedding_with_concept_modified
(ANN201)
2167-2167: Missing return type annotation for public function user_rates_resources
(ANN201)
2183-2183: Comparison to None should be cond is not None
Replace with cond is not None
(E711)
2183-2183: Avoid equality comparisons to True; use rating["reset"]: for truth checks
Replace with rating["reset"]
(E712)
2267-2267: Unnecessary assignment to result before return statement
Remove unnecessary assignment
(RET504)
2269-2269: Missing return type annotation for public function update_rs_btw_resource_and_cm
Add return type annotation: None
(ANN201)
2269-2269: Boolean default positional argument in function definition
(FBT002)
2269-2269: Missing type annotation for function argument action
(ANN001)
2300-2300: Do not catch blind exception: Exception
(BLE001)
2303-2303: Unnecessary pass statement
Remove unnecessary pass
(PIE790)
2305-2305: Missing return type annotation for public function update_rs_btw_resources_and_cm
Add return type annotation: None
(ANN201)
2305-2305: Boolean default positional argument in function definition
(FBT002)
2305-2305: Missing type annotation for function argument action
(ANN001)
2328-2328: Do not catch blind exception: Exception
(BLE001)
2331-2331: Unnecessary pass statement
Remove unnecessary pass
(PIE790)
2333-2333: Missing return type annotation for public function user_saves_or_removes_resource
(ANN201)
2349-2349: Avoid equality comparisons to True; use data["status"]: for truth checks
Replace with data["status"]
(E712)
2384-2384: store_resources is too complex (14 > 10)
(C901)
2384-2384: Missing return type annotation for public function store_resources
Add return type annotation: None
(ANN201)
2384-2384: PEP 484 prohibits implicit Optional
Convert to Optional[T]
(RUF013)
2384-2384: Missing type annotation for function argument recommendation_type
(ANN001)
2385-2385: PEP 484 prohibits implicit Optional
Convert to Optional[T]
(RUF013)
2385-2385: Missing type annotation for function argument resources_form
(ANN001)
2386-2386: Unused method argument: resources_updated
(ARG002)
2386-2386: PEP 484 prohibits implicit Optional
Convert to Optional[T]
(RUF013)
2386-2386: Missing type annotation for function argument resources_updated_type
(ANN001)
2386-2386: Unused method argument: resources_updated_type
(ARG002)
2404-2404: Missing return type annotation for private function get_resource_primary_key
(ANN202)
2415-2415: Logging statement uses f-string
(G004)
2422-2423: Use a single if statement instead of nested if statements
(SIM102)
2424-2424: Logging statement uses f-string
(G004)
2449-2449: Missing return type annotation for public function retrieve_resources
(ANN201)
2449-2449: Boolean default positional argument in function definition
(FBT002)
2449-2449: Missing type annotation for function argument embedding_values
(ANN001)
2455-2455: Missing return type annotation for private function resource_replace_none_value
(ANN202)
2455-2455: Missing type annotation for function argument value
(ANN001)
2456-2456: Comparison to None should be cond is None
Replace with cond is None
(E711)
2462-2462: Avoid equality comparisons to True; use embedding_values: for truth checks
Replace with embedding_values
(E712)
2513-2513: Unnecessary assignment to result before return statement
Remove unnecessary assignment
(RET504)
2515-2515: Missing return type annotation for public function retrieve_resources_by_updated_at_exist_or_counts
(ANN201)
2515-2515: Missing type annotation for function argument cids
(ANN001)
2515-2515: Boolean default positional argument in function definition
(FBT002)
2515-2515: Missing type annotation for function argument only_exist
(ANN001)
2515-2515: Missing type annotation for function argument days
(ANN001)
2526-2526: Avoid equality comparisons to True; use only_exist: for truth checks
Replace with only_exist
(E712)
2548-2548: Unnecessary assignment to count before return statement
Remove unnecessary assignment
(RET504)
2550-2550: Missing return type annotation for public function resources_wrapper_from_query
(ANN201)
2570-2570: Use resource.get("is_bookmarked_fill", False) instead of an if block
Replace with resource.get("is_bookmarked_fill", False)
(SIM401)
2571-2571: Use resource.get("saves_count", 0) instead of an if block
Replace with resource.get("saves_count", 0)
(SIM401)
2572-2572: Use resource.get("concept_cid", 0) instead of an if block
Replace with resource.get("concept_cid", 0)
(SIM401)
2592-2592: Missing return type annotation for public function update_resource_action
Add return type annotation: None
(ANN201)
2592-2592: Boolean default positional argument in function definition
(FBT002)
2592-2592: Missing type annotation for function argument action
(ANN001)
2610-2610: Missing return type annotation for public function filter_user_resources_saved_by
(ANN201)
2676-2676: Missing return type annotation for public function get_rids_from_user_saves
(ANN201)
2691-2691: Unnecessary assignment to nodes before return statement
Remove unnecessary assignment
(RET504)
2693-2693: Missing return type annotation for public function get_main_concepts_by_mid
(ANN201)
2710-2710: Unnecessary assignment to nodes before return statement
Remove unnecessary assignment
(RET504)
2712-2712: Missing return type annotation for public function get_main_concepts_by_slide_id
(ANN201)
2729-2729: Unnecessary assignment to nodes before return statement
Remove unnecessary assignment
(RET504)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: Build container images / Build image (coursemapper-kg/concept-map)
- GitHub Check: Build container images / Build image (webapp)
- GitHub Check: Build container images / Build image (webserver)
- GitHub Check: Build container images / Build image (coursemapper-kg/recommendation)
- GitHub Check: Build container images / Build image (coursemapper-kg/preprocess)
🔇 Additional comments (4)
webapp/src/app/pages/components/knowledge-graph/concept-map/concept-map.component.ts (3)
7-9: LGTM: required imports added correctly
ViewChild,Renderer2, CRO types, and the CRO component are appropriately imported.Also applies to: 34-36
2304-2306: LGTM: safe optional chaining incroUpdaterThis avoids errors when the child ViewChild isn’t yet available.
2317-2333: Delegate width logic to the responsive helper
Verify that the resize handler is actually bound (via@HostListener('window:resize', ['$event'])) and that asetResponsiveWidthKnowledgeGraph()helper exists before renamingsetWeightGraphComponenttosetWidthGraphComponentand updating its callsites.coursemapper-kg/recommendation/app/services/course_materials/db/neo4_db.py (1)
873-896: Potential signature mismatch in concept resource retrieval.
retrieve_concept_resourcesis defined as(tx, mid, cid)butget_concept_resourcescalls it with(tx, concept_id)and earlier with(session, material_id, cid). Ensure all call sites pass bothmidandcid.Would you like me to scan the repo and generate a patch to normalize this API and its usages?
Also applies to: 1023-1046
| # os.environ['PYTHONHASHSEED'] = '0' | ||
| # os.execv(sys.executable, [sys.executable] + sys.argv) | ||
|
|
||
| from datetime import datetime |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Make timestamps timezone-aware (UTC).
Use timezone-aware timestamps to avoid Cypher datetime parsing inconsistencies and DST bugs.
-from datetime import datetime
+from datetime import datetime, timezone- updated_at=datetime.now().isoformat(),
+ updated_at=datetime.now(timezone.utc).isoformat(),- updated_at=datetime.now().isoformat(),
+ updated_at=datetime.now(timezone.utc).isoformat(),- updated_at=datetime.now().isoformat(),
+ updated_at=datetime.now(timezone.utc).isoformat(),Also applies to: 1848-1849, 1907-1907, 1984-1984
🤖 Prompt for AI Agents
In coursemapper-kg/recommendation/app/services/course_materials/db/neo4_db.py
around line 16 (and also for the occurrences at lines 1848-1849, 1907, 1984),
the code currently uses naive datetime objects; change imports and usages to
produce timezone-aware UTC timestamps (e.g., import timezone from datetime and
replace datetime.now() / datetime.utcnow() usages with
datetime.now(timezone.utc) or datetime.utcnow().replace(tzinfo=timezone.utc)) so
all created timestamps are explicitly UTC-aware before being used in Cypher
queries or stored.
| def create_external_source_resource(tx, node): | ||
| """ | ||
| Create ExternalSource Node | ||
| """ | ||
| logger.info( | ||
| "Creating ExternalSource resource '%s'" % node["id"]) | ||
| tx.run( | ||
| """MERGE (c:Resource:ExternalSource {rid: $rid, uri: $uri, | ||
| publish_time: $created_at, cid: $cid, description: $description, helpful_count: $helpful_count, | ||
| not_helpful_count: $not_helpful_count, saves_count: $saves_count})""", | ||
| rid=node["uri"], | ||
| uri=node["uri"], | ||
| publish_time=node["created_at"], | ||
| description=node["description"], | ||
| cid=node["cid"], | ||
| helpful_count=0, | ||
| not_helpful_count=0, | ||
| saves_count=0 | ||
| ) |
There was a problem hiding this comment.
Broken MERGE parameter names and RID; fix logger and type hints.
- Cypher uses
$created_atbut you passpublish_time; this makespublish_timealways null. ridis set tonode["uri"]instead of the resource id; likely unintended.- Prefer structured logging args over
%formatting. - Add annotations.
-def create_external_source_resource(tx, node):
+def create_external_source_resource(tx, node) -> None:
@@
- logger.info(
- "Creating ExternalSource resource '%s'" % node["id"])
+ logger.info("Creating ExternalSource resource '%s'", node["id"])
tx.run(
- """MERGE (c:Resource:ExternalSource {rid: $rid, uri: $uri,
- publish_time: $created_at, cid: $cid, description: $description, helpful_count: $helpful_count,
+ """MERGE (c:Resource:ExternalSource {rid: $rid, uri: $uri,
+ publish_time: $publish_time, cid: $cid, description: $description, helpful_count: $helpful_count,
not_helpful_count: $not_helpful_count, saves_count: $saves_count})""",
- rid=node["uri"],
+ rid=node["id"],
uri=node["uri"],
- publish_time=node["created_at"],
+ publish_time=node["created_at"],
description=node["description"],
cid=node["cid"],
helpful_count=0,
not_helpful_count=0,
saves_count=0
)📝 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.
| def create_external_source_resource(tx, node): | |
| """ | |
| Create ExternalSource Node | |
| """ | |
| logger.info( | |
| "Creating ExternalSource resource '%s'" % node["id"]) | |
| tx.run( | |
| """MERGE (c:Resource:ExternalSource {rid: $rid, uri: $uri, | |
| publish_time: $created_at, cid: $cid, description: $description, helpful_count: $helpful_count, | |
| not_helpful_count: $not_helpful_count, saves_count: $saves_count})""", | |
| rid=node["uri"], | |
| uri=node["uri"], | |
| publish_time=node["created_at"], | |
| description=node["description"], | |
| cid=node["cid"], | |
| helpful_count=0, | |
| not_helpful_count=0, | |
| saves_count=0 | |
| ) | |
| def create_external_source_resource(tx, node) -> None: | |
| """ | |
| Create ExternalSource Node | |
| """ | |
| logger.info("Creating ExternalSource resource '%s'", node["id"]) | |
| tx.run( | |
| """MERGE (c:Resource:ExternalSource {rid: $rid, uri: $uri, | |
| publish_time: $publish_time, cid: $cid, description: $description, helpful_count: $helpful_count, | |
| not_helpful_count: $not_helpful_count, saves_count: $saves_count})""", | |
| rid=node["id"], | |
| uri=node["uri"], | |
| publish_time=node["created_at"], | |
| description=node["description"], | |
| cid=node["cid"], | |
| helpful_count=0, | |
| not_helpful_count=0, | |
| saves_count=0 | |
| ) |
🧰 Tools
🪛 Ruff (0.12.2)
129-129: Missing return type annotation for public function create_external_source_resource
Add return type annotation: None
(ANN201)
129-129: Missing type annotation for function argument tx
(ANN001)
129-129: Missing type annotation for function argument node
(ANN001)
134-134: Use format specifiers instead of percent format
Replace with format specifiers
(UP031)
134-134: Logging statement uses %
(G002)
| def get_or_create_user_v2(self, user): | ||
| """ | ||
| """ | ||
| tx = self.driver.session() | ||
| user_node = tx.run( | ||
| "MATCH (u:User) WHERE u.uid = $uid RETURN u.uid as uid", | ||
| uid=user["user_id"] | ||
| ).single() | ||
|
|
||
| if user_node is None: | ||
| user_node = tx.run( | ||
| """MERGE (u:User {name: $name, uid: $uid, type: $type, email: $userEmail, embedding:$embedding}) RETURN u.uid""", | ||
| name=user["name"], | ||
| uid=user["user_id"], | ||
| type="user", | ||
| userEmail=user["user_email"], | ||
| embedding="" | ||
| ).single() | ||
|
|
||
| return user_node | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Resource leak and naming: open session is never closed.
Use a context manager; return normalized shape (uid string).
-def get_or_create_user_v2(self, user):
+def get_or_create_user_v2(self, user) -> str:
@@
- tx = self.driver.session()
- user_node = tx.run(
+ with self.driver.session() as session:
+ record = session.run(
"MATCH (u:User) WHERE u.uid = $uid RETURN u.uid as uid",
uid=user["user_id"]
- ).single()
-
- if user_node is None:
- user_node = tx.run(
+ ).single()
+
+ if record is None:
+ record = session.run(
"""MERGE (u:User {name: $name, uid: $uid, type: $type, email: $userEmail, embedding:$embedding}) RETURN u.uid as uid""",
name=user["name"],
uid=user["user_id"],
type="user",
userEmail=user["user_email"],
embedding=""
- ).single()
-
- return user_node
+ ).single()
+ return record["uid"]📝 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.
| def get_or_create_user_v2(self, user): | |
| """ | |
| """ | |
| tx = self.driver.session() | |
| user_node = tx.run( | |
| "MATCH (u:User) WHERE u.uid = $uid RETURN u.uid as uid", | |
| uid=user["user_id"] | |
| ).single() | |
| if user_node is None: | |
| user_node = tx.run( | |
| """MERGE (u:User {name: $name, uid: $uid, type: $type, email: $userEmail, embedding:$embedding}) RETURN u.uid""", | |
| name=user["name"], | |
| uid=user["user_id"], | |
| type="user", | |
| userEmail=user["user_email"], | |
| embedding="" | |
| ).single() | |
| return user_node | |
| def get_or_create_user_v2(self, user) -> str: | |
| """ | |
| """ | |
| with self.driver.session() as session: | |
| record = session.run( | |
| "MATCH (u:User) WHERE u.uid = $uid RETURN u.uid as uid", | |
| uid=user["user_id"] | |
| ).single() | |
| if record is None: | |
| record = session.run( | |
| """MERGE (u:User {name: $name, uid: $uid, type: $type, email: $userEmail, embedding:$embedding}) RETURN u.uid as uid""", | |
| name=user["name"], | |
| uid=user["user_id"], | |
| type="user", | |
| userEmail=user["user_email"], | |
| embedding="" | |
| ).single() | |
| return record["uid"] |
🧰 Tools
🪛 Ruff (0.12.2)
1795-1795: Missing return type annotation for public function get_or_create_user_v2
(ANN201)
1795-1795: Missing type annotation for function argument user
(ANN001)
🤖 Prompt for AI Agents
In coursemapper-kg/recommendation/app/services/course_materials/db/neo4_db.py
around lines 1795-1815 the Neo4j session is opened but never closed and the
function returns a raw record instead of a normalized uid string; fix by using a
context manager (with self.driver.session() as session:) and call
session.run(...) for the MATCH and MERGE queries, retrieve the uid from the
resulting record (e.g. record["uid"] or record.get("uid")) and return that uid
string (not the Record object), and ensure the MERGE/parameters match the same
uid field so the function always returns a consistent uid string.
| def create_or_update_video_resource(self, tx, node: dict, | ||
| recommendation_type='', | ||
| update_embedding_values=False, | ||
| update_detail_found=False | ||
| ): | ||
| ''' | ||
| Creating Resource YouTube | ||
| r.similarity_score = $similarity_score, | ||
| ''' | ||
| # logger.info(" Creating Resource YouTube") | ||
| try: | ||
| """ | ||
| if update_detail_found == True: | ||
| tx.run( | ||
| ''' | ||
| MATCH (r:Resource: Video) | ||
| WHERE r.rid = $rid | ||
| SET r.title = $title, r.description = $description, r.description_full = $description_full, | ||
| r.text = $text, r.duration = $duration, r.views = $views, like_count = $like_count, | ||
| r.channel_title = $channel_title, r.updated_at = $updated_at, | ||
| r.keyphrases = $keyphrases, r.keyphrase_embedding = $keyphrase_embedding, | ||
| r.document_embedding = $document_embedding | ||
| ''', | ||
| rid=node["rid"], | ||
| title=node["title"], | ||
| description=node["description"], | ||
| description_full=node["description_full"], | ||
| text=node["text"], | ||
| duration=node["duration"], | ||
| views=node["views"], | ||
| like_count=node["like_count"], | ||
| channel_title=node["channel_title"], | ||
| updated_at=datetime.now().isoformat(), | ||
| keyphrases=[], | ||
| keyphrase_embedding="", | ||
| document_embedding="" | ||
| ) | ||
| """ | ||
|
|
||
| if update_embedding_values == True: # node.get("keyphrases") != None or node.get("document_embedding") != None or node.get("keyphrase_embedding") != None: | ||
| tx.run( | ||
| ''' | ||
| MATCH (r:Resource: Video) | ||
| WHERE r.rid = $rid | ||
| SET r.keyphrases = $keyphrases, r.keyphrase_embedding = $keyphrase_embedding, r.document_embedding = $document_embedding, | ||
| r.keyphrases_infos = $keyphrases_infos | ||
|
|
||
| ''', | ||
| rid=node["rid"], | ||
| keyphrases=node["keyphrases"] if "keyphrases" in node else [], | ||
| keyphrase_embedding=node["keyphrase_embedding"] if "keyphrase_embedding" in node else [], | ||
| document_embedding=node["document_embedding"] if "document_embedding" in node else [], | ||
| keyphrases_infos=node["keyphrases_infos"] if "keyphrases_infos" in node else "" | ||
| ) | ||
| else: | ||
| tx.run( | ||
| ''' | ||
| MERGE (r:Resource:Video {rid: $rid}) | ||
| ON CREATE SET | ||
| r.uri = $uri, r.title = $title, r.description = $description, r.description_full = $description_full, r.text = $text, | ||
| r.keyphrases = $keyphrases, r.document_embedding = $document_embedding, r.keyphrase_embedding = $keyphrase_embedding, | ||
| r.thumbnail = $thumbnail, r.duration = $duration, r.views = $views, | ||
| r.publish_time = $pub_time, r.channel_title = $channel_title, r.like_count = $like_count, | ||
| r.helpful_count = $helpful_count, r.not_helpful_count = $not_helpful_count, r.saves_count = $saves_count, | ||
| r.updated_at = $updated_at | ||
| ON MATCH SET | ||
| r.title = $title, r.description = $description, r.description_full = $description_full, r.text = $text, | ||
| r.keyphrases = $keyphrases, r.document_embedding = $document_embedding, r.keyphrase_embedding = $keyphrase_embedding, | ||
| r.thumbnail = $thumbnail, r.duration = $duration, r.views = $views, | ||
| r.publish_time = $pub_time, r.channel_title = $channel_title, r.like_count = $like_count, | ||
| r.updated_at = $updated_at | ||
| ''', | ||
| rid=node["id"], | ||
| uri="https://www.youtube.com/embed/%s?autoplay=1" % node["id"], | ||
| title=node["title"], | ||
| description=node["description"], | ||
| description_full=node["description_full"], | ||
| thumbnail="https://i.ytimg.com/vi/%s/hqdefault.jpg" % node["id"], | ||
| text=node["text"], | ||
| duration=node["duration"], | ||
| views=node["views"], | ||
| pub_time=node["publishTime"], | ||
| # similarity_score=node[recommendation_type] if recommendation_type in node.index else 0, | ||
| keyphrases=node["keyphrases"] if "keyphrases" in node else [], | ||
| keyphrase_embedding=node["keyphrase_embedding"] if "keyphrase_embedding" in node else [], | ||
| document_embedding=node["document_embedding"] if "document_embedding" in node else [], | ||
| helpful_count=node["helpful_count"] if "helpful_count" in node else 0, | ||
| not_helpful_count=node["not_helpful_count"] if "not_helpful_count" in node else 0, | ||
| saves_count=node["saves_count"] if "saves_count" in node else 0, | ||
| like_count=node["like_count"], | ||
| channel_title=node["channel_title"], | ||
| updated_at=datetime.now().isoformat(), | ||
| keyphrases_infos=node["keyphrases_infos"] if "keyphrases_infos" in node else "" | ||
| ) | ||
| except Exception as e: | ||
| print(e) | ||
| pass | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Harden video upsert: defaults, boolean checks, formatting, and exceptions.
- Use
.get()for optional fields to avoid KeyError. - Prefer simple truth checks over
== True. - Use f-strings for string building (not for logging).
- Replace bare
exceptwith logging the exception.
- if update_embedding_values == True: # node.get("keyphrases") != None or node.get("document_embedding") != None or node.get("keyphrase_embedding") != None:
+ if update_embedding_values:
@@
- keyphrases=node["keyphrases"] if "keyphrases" in node else [],
- keyphrase_embedding=node["keyphrase_embedding"] if "keyphrase_embedding" in node else [],
- document_embedding=node["document_embedding"] if "document_embedding" in node else [],
- keyphrases_infos=node["keyphrases_infos"] if "keyphrases_infos" in node else ""
+ keyphrases=node.get("keyphrases", []),
+ keyphrase_embedding=node.get("keyphrase_embedding", []),
+ document_embedding=node.get("document_embedding", []),
+ keyphrases_infos=node.get("keyphrases_infos", "")
)
else:
tx.run(
@@
- rid=node["id"],
- uri="https://www.youtube.com/embed/%s?autoplay=1" % node["id"],
+ rid=node["id"],
+ uri=f"https://www.youtube.com/embed/{node['id']}?autoplay=1",
@@
- thumbnail="https://i.ytimg.com/vi/%s/hqdefault.jpg" % node["id"],
+ thumbnail=f"https://i.ytimg.com/vi/{node['id']}/hqdefault.jpg",
@@
- keyphrases=node["keyphrases"] if "keyphrases" in node else [],
- keyphrase_embedding=node["keyphrase_embedding"] if "keyphrase_embedding" in node else [],
- document_embedding=node["document_embedding"] if "document_embedding" in node else [],
- helpful_count=node["helpful_count"] if "helpful_count" in node else 0,
- not_helpful_count=node["not_helpful_count"] if "not_helpful_count" in node else 0,
- saves_count=node["saves_count"] if "saves_count" in node else 0,
+ keyphrases=node.get("keyphrases", []),
+ keyphrase_embedding=node.get("keyphrase_embedding", []),
+ document_embedding=node.get("document_embedding", []),
+ helpful_count=node.get("helpful_count", 0),
+ not_helpful_count=node.get("not_helpful_count", 0),
+ saves_count=node.get("saves_count", 0),
@@
- updated_at=datetime.now().isoformat(),
- keyphrases_infos=node["keyphrases_infos"] if "keyphrases_infos" in node else ""
+ updated_at=datetime.now(timezone.utc).isoformat(),
+ keyphrases_infos=node.get("keyphrases_infos", "")
)
- except Exception as e:
- print(e)
- pass
+ except Exception as e:
+ logger.exception("create_or_update_video_resource failed: %s", e)📝 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.
| def create_or_update_video_resource(self, tx, node: dict, | |
| recommendation_type='', | |
| update_embedding_values=False, | |
| update_detail_found=False | |
| ): | |
| ''' | |
| Creating Resource YouTube | |
| r.similarity_score = $similarity_score, | |
| ''' | |
| # logger.info(" Creating Resource YouTube") | |
| try: | |
| """ | |
| if update_detail_found == True: | |
| tx.run( | |
| ''' | |
| MATCH (r:Resource: Video) | |
| WHERE r.rid = $rid | |
| SET r.title = $title, r.description = $description, r.description_full = $description_full, | |
| r.text = $text, r.duration = $duration, r.views = $views, like_count = $like_count, | |
| r.channel_title = $channel_title, r.updated_at = $updated_at, | |
| r.keyphrases = $keyphrases, r.keyphrase_embedding = $keyphrase_embedding, | |
| r.document_embedding = $document_embedding | |
| ''', | |
| rid=node["rid"], | |
| title=node["title"], | |
| description=node["description"], | |
| description_full=node["description_full"], | |
| text=node["text"], | |
| duration=node["duration"], | |
| views=node["views"], | |
| like_count=node["like_count"], | |
| channel_title=node["channel_title"], | |
| updated_at=datetime.now().isoformat(), | |
| keyphrases=[], | |
| keyphrase_embedding="", | |
| document_embedding="" | |
| ) | |
| """ | |
| if update_embedding_values == True: # node.get("keyphrases") != None or node.get("document_embedding") != None or node.get("keyphrase_embedding") != None: | |
| tx.run( | |
| ''' | |
| MATCH (r:Resource: Video) | |
| WHERE r.rid = $rid | |
| SET r.keyphrases = $keyphrases, r.keyphrase_embedding = $keyphrase_embedding, r.document_embedding = $document_embedding, | |
| r.keyphrases_infos = $keyphrases_infos | |
| ''', | |
| rid=node["rid"], | |
| keyphrases=node["keyphrases"] if "keyphrases" in node else [], | |
| keyphrase_embedding=node["keyphrase_embedding"] if "keyphrase_embedding" in node else [], | |
| document_embedding=node["document_embedding"] if "document_embedding" in node else [], | |
| keyphrases_infos=node["keyphrases_infos"] if "keyphrases_infos" in node else "" | |
| ) | |
| else: | |
| tx.run( | |
| ''' | |
| MERGE (r:Resource:Video {rid: $rid}) | |
| ON CREATE SET | |
| r.uri = $uri, r.title = $title, r.description = $description, r.description_full = $description_full, r.text = $text, | |
| r.keyphrases = $keyphrases, r.document_embedding = $document_embedding, r.keyphrase_embedding = $keyphrase_embedding, | |
| r.thumbnail = $thumbnail, r.duration = $duration, r.views = $views, | |
| r.publish_time = $pub_time, r.channel_title = $channel_title, r.like_count = $like_count, | |
| r.helpful_count = $helpful_count, r.not_helpful_count = $not_helpful_count, r.saves_count = $saves_count, | |
| r.updated_at = $updated_at | |
| ON MATCH SET | |
| r.title = $title, r.description = $description, r.description_full = $description_full, r.text = $text, | |
| r.keyphrases = $keyphrases, r.document_embedding = $document_embedding, r.keyphrase_embedding = $keyphrase_embedding, | |
| r.thumbnail = $thumbnail, r.duration = $duration, r.views = $views, | |
| r.publish_time = $pub_time, r.channel_title = $channel_title, r.like_count = $like_count, | |
| r.updated_at = $updated_at | |
| ''', | |
| rid=node["id"], | |
| uri="https://www.youtube.com/embed/%s?autoplay=1" % node["id"], | |
| title=node["title"], | |
| description=node["description"], | |
| description_full=node["description_full"], | |
| thumbnail="https://i.ytimg.com/vi/%s/hqdefault.jpg" % node["id"], | |
| text=node["text"], | |
| duration=node["duration"], | |
| views=node["views"], | |
| pub_time=node["publishTime"], | |
| # similarity_score=node[recommendation_type] if recommendation_type in node.index else 0, | |
| keyphrases=node["keyphrases"] if "keyphrases" in node else [], | |
| keyphrase_embedding=node["keyphrase_embedding"] if "keyphrase_embedding" in node else [], | |
| document_embedding=node["document_embedding"] if "document_embedding" in node else [], | |
| helpful_count=node["helpful_count"] if "helpful_count" in node else 0, | |
| not_helpful_count=node["not_helpful_count"] if "not_helpful_count" in node else 0, | |
| saves_count=node["saves_count"] if "saves_count" in node else 0, | |
| like_count=node["like_count"], | |
| channel_title=node["channel_title"], | |
| updated_at=datetime.now().isoformat(), | |
| keyphrases_infos=node["keyphrases_infos"] if "keyphrases_infos" in node else "" | |
| ) | |
| except Exception as e: | |
| print(e) | |
| pass | |
| def create_or_update_video_resource(self, tx, node: dict, | |
| recommendation_type='', | |
| update_embedding_values=False, | |
| update_detail_found=False | |
| ): | |
| ''' | |
| Creating Resource YouTube | |
| r.similarity_score = $similarity_score, | |
| ''' | |
| # logger.info(" Creating Resource YouTube") | |
| try: | |
| """ | |
| if update_detail_found == True: | |
| tx.run( | |
| ''' | |
| MATCH (r:Resource: Video) | |
| WHERE r.rid = $rid | |
| SET r.title = $title, r.description = $description, r.description_full = $description_full, | |
| r.text = $text, r.duration = $duration, r.views = $views, like_count = $like_count, | |
| r.channel_title = $channel_title, r.updated_at = $updated_at, | |
| r.keyphrases = $keyphrases, r.keyphrase_embedding = $keyphrase_embedding, | |
| r.document_embedding = $document_embedding | |
| ''', | |
| rid=node["rid"], | |
| title=node["title"], | |
| description=node["description"], | |
| description_full=node["description_full"], | |
| text=node["text"], | |
| duration=node["duration"], | |
| views=node["views"], | |
| like_count=node["like_count"], | |
| channel_title=node["channel_title"], | |
| updated_at=datetime.now(timezone.utc).isoformat(), | |
| keyphrases=[], | |
| keyphrase_embedding="", | |
| document_embedding="" | |
| ) | |
| """ | |
| if update_embedding_values: | |
| tx.run( | |
| ''' | |
| MATCH (r:Resource: Video) | |
| WHERE r.rid = $rid | |
| SET r.keyphrases = $keyphrases, | |
| r.keyphrase_embedding = $keyphrase_embedding, | |
| r.document_embedding = $document_embedding, | |
| r.keyphrases_infos = $keyphrases_infos | |
| ''', | |
| rid=node["rid"], | |
| keyphrases=node.get("keyphrases", []), | |
| keyphrase_embedding=node.get("keyphrase_embedding", []), | |
| document_embedding=node.get("document_embedding", []), | |
| keyphrases_infos=node.get("keyphrases_infos", "") | |
| ) | |
| else: | |
| tx.run( | |
| ''' | |
| MERGE (r:Resource:Video {rid: $rid}) | |
| ON CREATE SET | |
| r.uri = $uri, r.title = $title, r.description = $description, | |
| r.description_full = $description_full, r.text = $text, | |
| r.keyphrases = $keyphrases, r.document_embedding = $document_embedding, | |
| r.keyphrase_embedding = $keyphrase_embedding, | |
| r.thumbnail = $thumbnail, r.duration = $duration, | |
| r.views = $views, r.publish_time = $pub_time, | |
| r.channel_title = $channel_title, r.like_count = $like_count, | |
| r.helpful_count = $helpful_count, r.not_helpful_count = $not_helpful_count, | |
| r.saves_count = $saves_count, r.updated_at = $updated_at | |
| ON MATCH SET | |
| r.title = $title, r.description = $description, | |
| r.description_full = $description_full, r.text = $text, | |
| r.keyphrases = $keyphrases, r.document_embedding = $document_embedding, | |
| r.keyphrase_embedding = $keyphrase_embedding, | |
| r.thumbnail = $thumbnail, r.duration = $duration, | |
| r.views = $views, r.publish_time = $pub_time, | |
| r.channel_title = $channel_title, r.like_count = $like_count, | |
| r.updated_at = $updated_at | |
| ''', | |
| rid=node["id"], | |
| uri=f"https://www.youtube.com/embed/{node['id']}?autoplay=1", | |
| title=node["title"], | |
| description=node["description"], | |
| description_full=node["description_full"], | |
| thumbnail=f"https://i.ytimg.com/vi/{node['id']}/hqdefault.jpg", | |
| text=node["text"], | |
| duration=node["duration"], | |
| views=node["views"], | |
| pub_time=node["publishTime"], | |
| keyphrases=node.get("keyphrases", []), | |
| keyphrase_embedding=node.get("keyphrase_embedding", []), | |
| document_embedding=node.get("document_embedding", []), | |
| helpful_count=node.get("helpful_count", 0), | |
| not_helpful_count=node.get("not_helpful_count", 0), | |
| saves_count=node.get("saves_count", 0), | |
| like_count=node["like_count"], | |
| channel_title=node["channel_title"], | |
| updated_at=datetime.now(timezone.utc).isoformat(), | |
| keyphrases_infos=node.get("keyphrases_infos", "") | |
| ) | |
| except Exception as e: | |
| logger.exception("create_or_update_video_resource failed: %s", e) |
🧰 Tools
🪛 Ruff (0.12.2)
1816-1816: Missing return type annotation for public function create_or_update_video_resource
Add return type annotation: None
(ANN201)
1816-1816: Missing type annotation for function argument tx
(ANN001)
1817-1817: Missing type annotation for function argument recommendation_type
(ANN001)
1817-1817: Unused method argument: recommendation_type
(ARG002)
1818-1818: Boolean default positional argument in function definition
(FBT002)
1818-1818: Missing type annotation for function argument update_embedding_values
(ANN001)
1819-1819: Boolean default positional argument in function definition
(FBT002)
1819-1819: Missing type annotation for function argument update_detail_found
(ANN001)
1819-1819: Unused method argument: update_detail_found
(ARG002)
1855-1855: Avoid equality comparisons to True; use update_embedding_values: for truth checks
Replace with update_embedding_values
(E712)
1865-1865: Use node.get("keyphrases", []) instead of an if block
Replace with node.get("keyphrases", [])
(SIM401)
1866-1866: Use node.get("keyphrase_embedding", []) instead of an if block
Replace with node.get("keyphrase_embedding", [])
(SIM401)
1867-1867: Use node.get("document_embedding", []) instead of an if block
Replace with node.get("document_embedding", [])
(SIM401)
1868-1868: Use node.get("keyphrases_infos", "") instead of an if block
Replace with node.get("keyphrases_infos", "")
(SIM401)
1889-1889: Use format specifiers instead of percent format
Replace with format specifiers
(UP031)
1893-1893: Use format specifiers instead of percent format
Replace with format specifiers
(UP031)
1899-1899: Use node.get("keyphrases", []) instead of an if block
Replace with node.get("keyphrases", [])
(SIM401)
1900-1900: Use node.get("keyphrase_embedding", []) instead of an if block
Replace with node.get("keyphrase_embedding", [])
(SIM401)
1901-1901: Use node.get("document_embedding", []) instead of an if block
Replace with node.get("document_embedding", [])
(SIM401)
1902-1902: Use node.get("helpful_count", 0) instead of an if block
Replace with node.get("helpful_count", 0)
(SIM401)
1903-1903: Use node.get("not_helpful_count", 0) instead of an if block
Replace with node.get("not_helpful_count", 0)
(SIM401)
1904-1904: Use node.get("saves_count", 0) instead of an if block
Replace with node.get("saves_count", 0)
(SIM401)
1907-1907: datetime.datetime.now() called without a tz argument
(DTZ005)
1908-1908: Use node.get("keyphrases_infos", "") instead of an if block
Replace with node.get("keyphrases_infos", "")
(SIM401)
1910-1910: Do not catch blind exception: Exception
(BLE001)
1912-1912: Unnecessary pass statement
Remove unnecessary pass
(PIE790)
🤖 Prompt for AI Agents
In coursemapper-kg/recommendation/app/services/course_materials/db/neo4_db.py
around lines 1816 to 1913, the video upsert is fragile: replace direct indexing
like node["..."] with node.get("...") and provide sensible defaults (e.g. empty
list or 0) to avoid KeyError; change the conditional check from
update_embedding_values == True to a simple truthy check (if
update_embedding_values:); build uri and thumbnail using f-strings (e.g.
f"https://www.youtube.com/embed/{node.get('id')}?autoplay=1") instead of
%-formatting; and replace the current bare/print exception handling with a
proper logger call that logs the exception and stack (e.g. logger.exception or
logger.error with exc_info=True) rather than swallowing errors.
| def create_or_update_wikipedia_resource(self, tx, node, recommendation_type='', | ||
| update_embedding_values=False, | ||
| update_detail_found=False | ||
| ): | ||
| ''' | ||
| Creating Resource Wikipedia | ||
| ''' | ||
| # logger.info("Creating Resource Wikipedia") | ||
| try: | ||
| """ | ||
| if update_detail_found == True: | ||
| tx.run( | ||
| ''' | ||
| MATCH (r:Resource: Article) | ||
| WHERE r.rid = $rid | ||
| SET r.title = $title, r.abstract = $abstract, r.text = $text, updated_at = $updated_at, | ||
| r.keyphrases = $keyphrases, r.keyphrase_embedding = $keyphrase_embedding, | ||
| r.document_embedding = $document_embedding | ||
| ''', | ||
| rid=node["rid"], | ||
| title=node["title"], | ||
| abstract=node["abstract"], | ||
| text=node["text"], | ||
| updated_at=datetime.now().isoformat(), | ||
| keyphrases=node["keyphrases"] if "keyphrases" in node else [], | ||
| keyphrase_embedding=str(node["keyphrase_embedding"] if "keyphrase_embedding" in node else ""), | ||
| document_embedding=str(node["document_embedding"] if "document_embedding" in node else ""), | ||
| ) | ||
| """ | ||
|
|
||
| if update_embedding_values == True: # node.get("keyphrases") != None or node.get("document_embedding") != None or node.get("keyphrase_embedding") != None: | ||
| tx.run( | ||
| ''' | ||
| MATCH (r:Resource: Article) | ||
| WHERE r.rid = $rid | ||
| SET r.keyphrases = $keyphrases, r.keyphrase_embedding = $keyphrase_embedding, r.document_embedding = $document_embedding, | ||
| r.keyphrases_infos = $keyphrases_infos | ||
| ''', | ||
| rid=node["rid"], | ||
| keyphrases=node["keyphrases"] if "keyphrases" in node else [], | ||
| keyphrase_embedding=node["keyphrase_embedding"] if "keyphrase_embedding" in node else [], | ||
| document_embedding=node["document_embedding"] if "document_embedding" in node else [], | ||
| keyphrases_infos=node["keyphrases_infos"] if "keyphrases_infos" in node else "" | ||
| ) | ||
| else: | ||
| tx.run( | ||
| ''' | ||
| MERGE (r:Resource:Article {rid: $rid}) | ||
| ON CREATE SET | ||
| r.uri = $uri, r.title = $title, r.abstract = $abstract, r.text = $text, | ||
| r.keyphrases = $keyphrases, r.document_embedding = $document_embedding, r.keyphrase_embedding = $keyphrase_embedding, | ||
| r.helpful_count = $helpful_count, r.not_helpful_count = $not_helpful_count, r.saves_count = $saves_count, | ||
| r.updated_at = $updated_at | ||
| ON MATCH SET | ||
| r.uri = $uri, r.title = $title, r.abstract = $abstract, r.text = $text, | ||
| r.keyphrases = $keyphrases, r.document_embedding = $document_embedding, r.keyphrase_embedding = $keyphrase_embedding, | ||
| r.updated_at = $updated_at | ||
| ''', | ||
| rid=node["id"], | ||
| uri=node["id"], | ||
| title=node["title"], | ||
| abstract=node["abstract"], | ||
| keyphrases=node["keyphrases"] if "keyphrases" in node else [], | ||
| text=node["text"], | ||
| # similarity_score=node[recommendation_type] if recommendation_type in node.index else 0, | ||
| keyphrase_embedding=node["keyphrase_embedding"] if "keyphrase_embedding" in node else [], | ||
| document_embedding=node["document_embedding"] if "document_embedding" in node else [], | ||
| helpful_count=node["helpful_count"] if "helpful_count" in node else 0, | ||
| not_helpful_count=node["not_helpful_count"] if "not_helpful_count" in node else 0, | ||
| saves_count=node["saves_count"] if "saves_count" in node else 0, | ||
| updated_at=datetime.now().isoformat(), | ||
| keyphrases_infos=node["keyphrases_infos"] if "keyphrases_infos" in node else "" | ||
| ) | ||
| except Exception as e: | ||
| print(e) | ||
| pass | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Harden article upsert similarly; avoid bare exceptions and make timestamps UTC.
- if update_embedding_values == True: # node.get("keyphrases") != None or node.get("document_embedding") != None or node.get("keyphrase_embedding") != None:
+ if update_embedding_values:
@@
- keyphrases=node["keyphrases"] if "keyphrases" in node else [],
- keyphrase_embedding=node["keyphrase_embedding"] if "keyphrase_embedding" in node else [],
- document_embedding=node["document_embedding"] if "document_embedding" in node else [],
- keyphrases_infos=node["keyphrases_infos"] if "keyphrases_infos" in node else ""
+ keyphrases=node.get("keyphrases", []),
+ keyphrase_embedding=node.get("keyphrase_embedding", []),
+ document_embedding=node.get("document_embedding", []),
+ keyphrases_infos=node.get("keyphrases_infos", "")
)
else:
tx.run(
@@
- keyphrases=node["keyphrases"] if "keyphrases" in node else [],
+ keyphrases=node.get("keyphrases", []),
@@
- keyphrase_embedding=node["keyphrase_embedding"] if "keyphrase_embedding" in node else [],
- document_embedding=node["document_embedding"] if "document_embedding" in node else [],
- helpful_count=node["helpful_count"] if "helpful_count" in node else 0,
- not_helpful_count=node["not_helpful_count"] if "not_helpful_count" in node else 0,
- saves_count=node["saves_count"] if "saves_count" in node else 0,
- updated_at=datetime.now().isoformat(),
- keyphrases_infos=node["keyphrases_infos"] if "keyphrases_infos" in node else ""
+ keyphrase_embedding=node.get("keyphrase_embedding", []),
+ document_embedding=node.get("document_embedding", []),
+ helpful_count=node.get("helpful_count", 0),
+ not_helpful_count=node.get("not_helpful_count", 0),
+ saves_count=node.get("saves_count", 0),
+ updated_at=datetime.now(timezone.utc).isoformat(),
+ keyphrases_infos=node.get("keyphrases_infos", "")
)
- except Exception as e:
- print(e)
- pass
+ except Exception as e:
+ logger.exception("create_or_update_wikipedia_resource failed: %s", e)📝 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.
| def create_or_update_wikipedia_resource(self, tx, node, recommendation_type='', | |
| update_embedding_values=False, | |
| update_detail_found=False | |
| ): | |
| ''' | |
| Creating Resource Wikipedia | |
| ''' | |
| # logger.info("Creating Resource Wikipedia") | |
| try: | |
| """ | |
| if update_detail_found == True: | |
| tx.run( | |
| ''' | |
| MATCH (r:Resource: Article) | |
| WHERE r.rid = $rid | |
| SET r.title = $title, r.abstract = $abstract, r.text = $text, updated_at = $updated_at, | |
| r.keyphrases = $keyphrases, r.keyphrase_embedding = $keyphrase_embedding, | |
| r.document_embedding = $document_embedding | |
| ''', | |
| rid=node["rid"], | |
| title=node["title"], | |
| abstract=node["abstract"], | |
| text=node["text"], | |
| updated_at=datetime.now().isoformat(), | |
| keyphrases=node["keyphrases"] if "keyphrases" in node else [], | |
| keyphrase_embedding=str(node["keyphrase_embedding"] if "keyphrase_embedding" in node else ""), | |
| document_embedding=str(node["document_embedding"] if "document_embedding" in node else ""), | |
| ) | |
| """ | |
| if update_embedding_values == True: # node.get("keyphrases") != None or node.get("document_embedding") != None or node.get("keyphrase_embedding") != None: | |
| tx.run( | |
| ''' | |
| MATCH (r:Resource: Article) | |
| WHERE r.rid = $rid | |
| SET r.keyphrases = $keyphrases, r.keyphrase_embedding = $keyphrase_embedding, r.document_embedding = $document_embedding, | |
| r.keyphrases_infos = $keyphrases_infos | |
| ''', | |
| rid=node["rid"], | |
| keyphrases=node["keyphrases"] if "keyphrases" in node else [], | |
| keyphrase_embedding=node["keyphrase_embedding"] if "keyphrase_embedding" in node else [], | |
| document_embedding=node["document_embedding"] if "document_embedding" in node else [], | |
| keyphrases_infos=node["keyphrases_infos"] if "keyphrases_infos" in node else "" | |
| ) | |
| else: | |
| tx.run( | |
| ''' | |
| MERGE (r:Resource:Article {rid: $rid}) | |
| ON CREATE SET | |
| r.uri = $uri, r.title = $title, r.abstract = $abstract, r.text = $text, | |
| r.keyphrases = $keyphrases, r.document_embedding = $document_embedding, r.keyphrase_embedding = $keyphrase_embedding, | |
| r.helpful_count = $helpful_count, r.not_helpful_count = $not_helpful_count, r.saves_count = $saves_count, | |
| r.updated_at = $updated_at | |
| ON MATCH SET | |
| r.uri = $uri, r.title = $title, r.abstract = $abstract, r.text = $text, | |
| r.keyphrases = $keyphrases, r.document_embedding = $document_embedding, r.keyphrase_embedding = $keyphrase_embedding, | |
| r.updated_at = $updated_at | |
| ''', | |
| rid=node["id"], | |
| uri=node["id"], | |
| title=node["title"], | |
| abstract=node["abstract"], | |
| keyphrases=node["keyphrases"] if "keyphrases" in node else [], | |
| text=node["text"], | |
| # similarity_score=node[recommendation_type] if recommendation_type in node.index else 0, | |
| keyphrase_embedding=node["keyphrase_embedding"] if "keyphrase_embedding" in node else [], | |
| document_embedding=node["document_embedding"] if "document_embedding" in node else [], | |
| helpful_count=node["helpful_count"] if "helpful_count" in node else 0, | |
| not_helpful_count=node["not_helpful_count"] if "not_helpful_count" in node else 0, | |
| saves_count=node["saves_count"] if "saves_count" in node else 0, | |
| updated_at=datetime.now().isoformat(), | |
| keyphrases_infos=node["keyphrases_infos"] if "keyphrases_infos" in node else "" | |
| ) | |
| except Exception as e: | |
| print(e) | |
| pass | |
| def create_or_update_wikipedia_resource(self, tx, node, recommendation_type='', | |
| update_embedding_values=False, | |
| update_detail_found=False | |
| ): | |
| ''' | |
| Creating Resource Wikipedia | |
| ''' | |
| # logger.info("Creating Resource Wikipedia") | |
| try: | |
| """ | |
| if update_detail_found == True: | |
| tx.run( | |
| ''' | |
| MATCH (r:Resource: Article) | |
| WHERE r.rid = $rid | |
| SET r.title = $title, r.abstract = $abstract, r.text = $text, updated_at = $updated_at, | |
| r.keyphrases = $keyphrases, r.keyphrase_embedding = $keyphrase_embedding, | |
| r.document_embedding = $document_embedding | |
| ''', | |
| rid=node["rid"], | |
| title=node["title"], | |
| abstract=node["abstract"], | |
| text=node["text"], | |
| updated_at=datetime.now().isoformat(), | |
| keyphrases=node["keyphrases"] if "keyphrases" in node else [], | |
| keyphrase_embedding=str(node["keyphrase_embedding"] if "keyphrase_embedding" in node else ""), | |
| document_embedding=str(node["document_embedding"] if "document_embedding" in node else ""), | |
| ) | |
| """ | |
| if update_embedding_values: | |
| tx.run( | |
| ''' | |
| MATCH (r:Resource: Article) | |
| WHERE r.rid = $rid | |
| SET r.keyphrases = $keyphrases, r.keyphrase_embedding = $keyphrase_embedding, r.document_embedding = $document_embedding, | |
| r.keyphrases_infos = $keyphrases_infos | |
| ''', | |
| rid=node["rid"], | |
| - keyphrases=node["keyphrases"] if "keyphrases" in node else [], | |
| - keyphrase_embedding=node["keyphrase_embedding"] if "keyphrase_embedding" in node else [], | |
| - document_embedding=node["document_embedding"] if "document_embedding" in node else [], | |
| keyphrases=node.get("keyphrases", []), | |
| keyphrase_embedding=node.get("keyphrase_embedding", []), | |
| document_embedding=node.get("document_embedding", []), | |
| keyphrases_infos=node.get("keyphrases_infos", "") | |
| ) | |
| else: | |
| tx.run( | |
| ''' | |
| MERGE (r:Resource:Article {rid: $rid}) | |
| ON CREATE SET | |
| r.uri = $uri, r.title = $title, r.abstract = $abstract, r.text = $text, | |
| r.keyphrases = $keyphrases, r.document_embedding = $document_embedding, r.keyphrase_embedding = $keyphrase_embedding, | |
| r.helpful_count = $helpful_count, r.not_helpful_count = $not_helpful_count, r.saves_count = $saves_count, | |
| r.updated_at = $updated_at | |
| ON MATCH SET | |
| r.uri = $uri, r.title = $title, r.abstract = $abstract, r.text = $text, | |
| r.keyphrases = $keyphrases, r.document_embedding = $document_embedding, r.keyphrase_embedding = $keyphrase_embedding, | |
| r.updated_at = $updated_at | |
| ''', | |
| rid=node["id"], | |
| uri=node["id"], | |
| title=node["title"], | |
| abstract=node["abstract"], | |
| keyphrases=node.get("keyphrases", []), | |
| text=node["text"], | |
| # similarity_score=node[recommendation_type] if recommendation_type in node.index else 0, | |
| - keyphrase_embedding=node["keyphrase_embedding"] if "keyphrase_embedding" in node else [], | |
| - document_embedding=node["document_embedding"] if "document_embedding" in node else [], | |
| - helpful_count=node["helpful_count"] if "helpful_count" in node else 0, | |
| - not_helpful_count=node["not_helpful_count"] if "not_helpful_count" in node else 0, | |
| - saves_count=node["saves_count"] if "saves_count" in node else 0, | |
| - updated_at=datetime.now().isoformat(), | |
| keyphrase_embedding=node.get("keyphrase_embedding", []), | |
| document_embedding=node.get("document_embedding", []), | |
| helpful_count=node.get("helpful_count", 0), | |
| not_helpful_count=node.get("not_helpful_count", 0), | |
| saves_count=node.get("saves_count", 0), | |
| updated_at=datetime.now(timezone.utc).isoformat(), | |
| keyphrases_infos=node.get("keyphrases_infos", "") | |
| ) | |
| - except Exception as e: | |
| - print(e) | |
| except Exception as e: | |
| logger.exception("create_or_update_wikipedia_resource failed: %s", e) |
🧰 Tools
🪛 Ruff (0.12.2)
1914-1914: Missing return type annotation for public function create_or_update_wikipedia_resource
Add return type annotation: None
(ANN201)
1914-1914: Missing type annotation for function argument tx
(ANN001)
1914-1914: Missing type annotation for function argument node
(ANN001)
1914-1914: Missing type annotation for function argument recommendation_type
(ANN001)
1914-1914: Unused method argument: recommendation_type
(ARG002)
1915-1915: Boolean default positional argument in function definition
(FBT002)
1915-1915: Missing type annotation for function argument update_embedding_values
(ANN001)
1916-1916: Boolean default positional argument in function definition
(FBT002)
1916-1916: Missing type annotation for function argument update_detail_found
(ANN001)
1916-1916: Unused method argument: update_detail_found
(ARG002)
1944-1944: Avoid equality comparisons to True; use update_embedding_values: for truth checks
Replace with update_embedding_values
(E712)
1953-1953: Use node.get("keyphrases", []) instead of an if block
Replace with node.get("keyphrases", [])
(SIM401)
1954-1954: Use node.get("keyphrase_embedding", []) instead of an if block
Replace with node.get("keyphrase_embedding", [])
(SIM401)
1955-1955: Use node.get("document_embedding", []) instead of an if block
Replace with node.get("document_embedding", [])
(SIM401)
1956-1956: Use node.get("keyphrases_infos", "") instead of an if block
Replace with node.get("keyphrases_infos", "")
(SIM401)
1976-1976: Use node.get("keyphrases", []) instead of an if block
Replace with node.get("keyphrases", [])
(SIM401)
1979-1979: Use node.get("keyphrase_embedding", []) instead of an if block
Replace with node.get("keyphrase_embedding", [])
(SIM401)
1980-1980: Use node.get("document_embedding", []) instead of an if block
Replace with node.get("document_embedding", [])
(SIM401)
1981-1981: Use node.get("helpful_count", 0) instead of an if block
Replace with node.get("helpful_count", 0)
(SIM401)
1982-1982: Use node.get("not_helpful_count", 0) instead of an if block
Replace with node.get("not_helpful_count", 0)
(SIM401)
1983-1983: Use node.get("saves_count", 0) instead of an if block
Replace with node.get("saves_count", 0)
(SIM401)
1984-1984: datetime.datetime.now() called without a tz argument
(DTZ005)
1985-1985: Use node.get("keyphrases_infos", "") instead of an if block
Replace with node.get("keyphrases_infos", "")
(SIM401)
1987-1987: Do not catch blind exception: Exception
(BLE001)
1989-1989: Unnecessary pass statement
Remove unnecessary pass
(PIE790)
| value="video" | ||
| [(ngModel)]="filteringParamsSavedTab.content_type" | ||
| inputId="video" | ||
| (onClick)="onContentTypeChange($event)"> | ||
| </p-radioButton> | ||
| <label for="video" class="ml-1">Video</label> | ||
| </div> | ||
| <div class="flex flex-row items-center"> | ||
| <p-radioButton | ||
| name="article" | ||
| value="article" | ||
| [(ngModel)]="filteringParamsSavedTab.content_type" | ||
| inputId="article" | ||
| (onClick)="onContentTypeChange($event)"> | ||
| </p-radioButton> | ||
| <label for="article" class="ml-1">Article</label> |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Confirm PrimeNG radio events.
Ensure p-radioButton actually emits onClick; if not, prefer (ngModelChange) to react to selection.
- (onClick)="onContentTypeChange($event)">
+ (ngModelChange)="onContentTypeChange($event)">
...
- (onClick)="onContentTypeChange($event)">
+ (ngModelChange)="onContentTypeChange($event)">📝 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.
| name="video" | |
| value="video" | |
| [(ngModel)]="filteringParamsSavedTab.content_type" | |
| inputId="video" | |
| (onClick)="onContentTypeChange($event)"> | |
| </p-radioButton> | |
| <label for="video" class="ml-1">Video</label> | |
| </div> | |
| <div class="flex flex-row items-center"> | |
| <p-radioButton | |
| name="article" | |
| value="article" | |
| [(ngModel)]="filteringParamsSavedTab.content_type" | |
| inputId="article" | |
| (onClick)="onContentTypeChange($event)"> | |
| </p-radioButton> | |
| <div class="flex flex-row items-center"> | |
| <p-radioButton | |
| name="video" | |
| value="video" | |
| [(ngModel)]="filteringParamsSavedTab.content_type" | |
| inputId="video" | |
| (ngModelChange)="onContentTypeChange($event)"> | |
| </p-radioButton> | |
| <label for="video" class="ml-1">Video</label> | |
| </div> | |
| <div class="flex flex-row items-center"> | |
| <p-radioButton | |
| name="article" | |
| value="article" | |
| [(ngModel)]="filteringParamsSavedTab.content_type" | |
| inputId="article" | |
| (ngModelChange)="onContentTypeChange($event)"> | |
| </p-radioButton> |
🤖 Prompt for AI Agents
In
webapp/src/app/pages/components/knowledge-graph/result-view/result-view.component.html
around lines 209 to 224, the template is binding (onClick) to PrimeNG
p-radioButton which does not reliably emit that event; replace the click binding
with (ngModelChange) so selection updates trigger the handler (e.g.
(ngModelChange)="onContentTypeChange($event)") and ensure the component's
onContentTypeChange accepts the new value (string) rather than a MouseEvent;
update both radio buttons accordingly to use the same ngModel and ngModelChange
handler.
| Total Articles: {{resourcesPagination?.nodes?.articles?.total_items}} | ||
| </div> |
There was a problem hiding this comment.
Wrong binding: Articles total gated by Videos total.
The label visibility for Articles is controlled by the Videos total.
- <div class="my-auto mx-1 text-color" *ngIf="resourcesPagination?.nodes?.videos?.total_items">
- Total Articles: {{resourcesPagination?.nodes?.articles?.total_items}}
+ <div class="my-auto mx-1 text-color" *ngIf="resourcesPagination?.nodes?.articles?.total_items">
+ Total Articles: {{ resourcesPagination?.nodes?.articles?.total_items }}📝 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.
| <div class="my-auto mx-1 text-color" *ngIf="resourcesPagination?.nodes?.videos?.total_items"> | |
| Total Articles: {{resourcesPagination?.nodes?.articles?.total_items}} | |
| <div class="my-auto mx-1 text-color" *ngIf="resourcesPagination?.nodes?.articles?.total_items"> | |
| Total Articles: {{ resourcesPagination?.nodes?.articles?.total_items }} |
🤖 Prompt for AI Agents
In
webapp/src/app/pages/components/knowledge-graph/result-view/result-view.component.html
around lines 346 to 347, the "Total Articles" label is conditionally shown using
resourcesPagination?.nodes?.videos?.total_items which hides the articles count
when videos are zero; change the *ngIf to check
resourcesPagination?.nodes?.articles?.total_items instead (use the articles
total_items null-safe path) so the Articles label visibility is driven by the
articles count.
| <ng-template [ngIf]="!showVideo" [ngIfElse]="watchvideo"> | ||
| <div class="" *ngFor="let videoElement of videoElements"> | ||
| <app-card-video | ||
| <div class="grid p-8 gap-8 sm:grid-cols-1 "> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
A11y: add list semantics and an accessible name for the icon-only button; mark strings for i18n
Improves screen-reader UX and localization readiness.
-<div class="grid p-8 gap-8 sm:grid-cols-1 ">
+<div class="grid p-8 gap-8 sm:grid-cols-1" role="list">
@@
- <app-card-video
+ <app-card-video
+ role="listitem"
@@
- <button id="backToList"
+ <button id="backToList"
pButton
type="button"
(click)="goBack()"
icon="pi pi-chevron-left"
- pTooltip="Go back to list"
+ pTooltip="Go back to list"
+ i18n-pTooltip="@@backToListTooltip"
+ aria-label="Go back to list"
+ i18n-aria-label="@@backToListAriaLabel"
class="bg-[#0277BD] p-button-rounded"
></button>Also applies to: 4-13, 22-29
🤖 Prompt for AI Agents
In
webapp/src/app/pages/components/knowledge-graph/videos/card-video-list/card-video-list.component.html
around line 1 (also applies to lines 4-13 and 22-29): the container and its
child items lack list semantics, the icon-only button has no accessible name,
and visible strings are not marked for i18n; fix by giving the container proper
list semantics (replace the generic div with a semantic <ul> or add
role="list"), ensure each item is a <li> or has role="listitem", add an
accessible name to the icon-only button (aria-label or aria-labelledby) with a
translatable string, and mark all user-facing text with the framework i18n
attributes (e.g., i18n or translate) so strings are ready for localization.
| <app-card-video | ||
| <div class="grid p-8 gap-8 sm:grid-cols-1 "> | ||
| <ng-template [ngIf]="!showVideo" [ngIfElse]="watchvideo"> | ||
| <div class="" *ngFor="let videoElement of videoElements"> |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Prefer structural *ngIf with else over [ngIf]/[ngIfElse] templates
Reduces extra DOM and improves readability.
- <ng-template [ngIf]="!showVideo" [ngIfElse]="watchvideo">
+ <ng-container *ngIf="!showVideo; else watchvideo">
...
- </ng-template>
+ </ng-container>Also applies to: 15-15
🤖 Prompt for AI Agents
In
webapp/src/app/pages/components/knowledge-graph/videos/card-video-list/card-video-list.component.html
around lines 2 and 15, the template uses the property bindings [ngIf] and
[ngIfElse]; replace those with the structural directive form (*ngIf="!showVideo;
else watchvideo") to reduce extra DOM and improve readability, and apply the
same replacement at line 15 where the pattern repeats; ensure the named
ng-template for "watchvideo" remains defined and referenced unchanged.
| <app-watch-video | ||
| [video]="video" | ||
| [notUnderstoodConcepts]="notUnderstoodConcepts" | ||
| (onWatchVideo)="readVideo($event)" | ||
| [videoElement]="videoElement" | ||
| [currentMaterial]="currentMaterial" | ||
| ></app-card-video> | ||
| </div> | ||
| </ng-template> | ||
| <ng-template #watchvideo> | ||
| <app-watch-video | ||
| [video]="video" | ||
| [notUnderstoodConcepts]="notUnderstoodConcepts" | ||
| [currentMaterial]="currentMaterial" | ||
| ></app-watch-video> | ||
| <button | ||
| id="backToList" | ||
| pButton | ||
| type="button" | ||
| (click)="goBack()" | ||
| icon="pi pi-chevron-left" | ||
| pTooltip="Go back to list" | ||
| class="bg-[#0277BD] p-button-rounded" | ||
| ></button> | ||
| </ng-template> | ||
| </div> | ||
| ></app-watch-video> | ||
| <button id="backToList" |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Guard against null/undefined video in the else block
Prevents runtime errors if video isn’t set when showVideo flips.
- <ng-template #watchvideo>
- <app-watch-video
- [video]="video"
- [notUnderstoodConcepts]="notUnderstoodConcepts"
- [currentMaterial]="currentMaterial"
- ></app-watch-video>
+ <ng-template #watchvideo>
+ <ng-container *ngIf="video as v">
+ <app-watch-video
+ [video]="v"
+ [notUnderstoodConcepts]="notUnderstoodConcepts"
+ [currentMaterial]="currentMaterial"
+ ></app-watch-video>
+ </ng-container>Confirm that video is always set before toggling showVideo = true.
📝 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.
| <ng-template #watchvideo> | |
| <app-watch-video | |
| [video]="video" | |
| [notUnderstoodConcepts]="notUnderstoodConcepts" | |
| (onWatchVideo)="readVideo($event)" | |
| [videoElement]="videoElement" | |
| [currentMaterial]="currentMaterial" | |
| ></app-card-video> | |
| </div> | |
| </ng-template> | |
| <ng-template #watchvideo> | |
| <app-watch-video | |
| [video]="video" | |
| [notUnderstoodConcepts]="notUnderstoodConcepts" | |
| [currentMaterial]="currentMaterial" | |
| ></app-watch-video> | |
| <button | |
| id="backToList" | |
| pButton | |
| type="button" | |
| (click)="goBack()" | |
| icon="pi pi-chevron-left" | |
| pTooltip="Go back to list" | |
| class="bg-[#0277BD] p-button-rounded" | |
| ></button> | |
| </ng-template> | |
| </div> | |
| ></app-watch-video> | |
| <ng-template #watchvideo> | |
| <ng-container *ngIf="video as v"> | |
| <app-watch-video | |
| [video]="v" | |
| [notUnderstoodConcepts]="notUnderstoodConcepts" | |
| [currentMaterial]="currentMaterial" | |
| ></app-watch-video> | |
| </ng-container> |
🤖 Prompt for AI Agents
In
webapp/src/app/pages/components/knowledge-graph/videos/card-video-list/card-video-list.component.html
around lines 16-21, the template renders <app-watch-video> in the
else/watchvideo block without guarding against a null/undefined video; either
add a truthy check to the template (e.g., wrap the <app-watch-video> with
*ngIf="video") so the component only renders when video is set, or ensure in the
component TS that you only set showVideo = true after verifying video is
non-null (throw or return early if not); implement one of these guards to
prevent runtime errors when showVideo flips.
Prevents IndexError: list index out of range when a video is invalid/private/deleted or filtered out.
There was a problem hiding this comment.
Actionable comments posted: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
coursemapper-kg/recommendation/app/services/course_materials/recommendation/youtube_service.py (2)
33-35: Do not set GOOGLE_APPLICATION_CREDENTIALS in codeRemove inline env mutation; configure via deployment (env/secret manager). Leaving it here risks leaking paths and breaks local/runtime separation.
- os.environ[ - "GOOGLE_APPLICATION_CREDENTIALS" - ] = "masterthesis-350015-47ab14d0b53b.json" + # GOOGLE_APPLICATION_CREDENTIALS must be provided by the environment (CI/K8s/secret store).
176-178: Avoid global string fill; fill per-column with typed defaultsGlobal "-1" converts numerics to strings.
- video_data = video_data.fillna("-1") + video_data = video_data.fillna({ + "duration": "0", + "views": 0, + "description_full": "", + "like_count": 0, + "channel_title": "", + "text": "", + })
♻️ Duplicate comments (3)
coursemapper-kg/recommendation/app/services/course_materials/recommendation/youtube_service.py (3)
39-52: Hard-coded API keys committed — rotate immediately and load from env/secret storeAll keys must be revoked and removed from source. Load a comma-separated list from env and fail fast if missing. Also drop the unused class-level client to avoid accidental usage.
- # DEVELOPER_KEY = os.environ.get("YOUTUBE_API_KEY") - DEVELOPER_KEY = "AIzaSyBphZOn7EJmPMmZwrB71aepaA5Rbuex9MU" - youtube = googleapiclient.discovery.build( - api_service_name, - api_version, - developerKey="AIzaSyClxnNwQ1x34pGioQazLlGxOjO9Fp2GGTY", - ) - DEVELOPER_KEYS = [ - "AIzaSyD_CGmR_Voq4DIV5okRaR6G8adoe-ZSZsM", - "AIzaSyClxnNwQ1x34pGioQazLlGxOjO9Fp2GGTY", - "AIzaSyADNntK6m7DbA6eZFYOa9Y8e6IYHykUUFE", - "AIzaSyBphZOn7EJmPMmZwrB71aepaA5Rbuex9MU", - "AIzaSyB2Wck31LUlgsqI7dgTcC2dMeeVXgb9TDI", - ] + # Comma-separated API keys from env (e.g., "k1,k2,k3"). Do NOT commit keys. + DEVELOPER_KEYS = [k.strip() for k in os.getenv("YOUTUBE_API_KEYS", "").split(",") if k.strip()] + if not DEVELOPER_KEYS: + raise RuntimeError("YOUTUBE_API_KEYS env var is required (comma-separated YouTube API keys).")
54-90: Retry/backoff logic is broken; fix loop, logging, and quota handling‘i’ never increments; retries never happen;
retry_count == 0is unreachable; use logger.exception and switch keys on quotaExceeded.- def search_youtube_videos(self, developer_keys, query, top_n=50, api_service_name="youtube", api_version="v3"): + def search_youtube_videos(self, developer_keys, query, top_n=50, api_service_name="youtube", api_version="v3"): """ Switching YouTube API keys """ - retry_count = 3 - retry_delay = 5 - i = 0 - for key in developer_keys: - try: - youtube = googleapiclient.discovery.build(api_service_name, api_version, developerKey=key) - request = youtube.search().list( - part="snippet", - maxResults=top_n, - type="video", - q=query, - relevanceLanguage="en", - ) - return request.execute(), youtube - except (ConnectionAbortedError, ConnectionResetError, timeout) as e: - logger.error("Error while getting the videos") - logger.error(e) - if i == retry_count - 1: - raise # re-raise the exception if all retries fail - delay = retry_delay * (2 ** i) # use a backoff algorithm to increase the delay - time.sleep(delay) - logger.info("New Try") - if retry_count == 0: - return None, None - except HttpError as e: - if e.resp.status == 403 and "quota" in str(e): - print(f"Quota exceeded for key: {key}. Trying next key...") - else: - raise e - raise Exception("All API keys have exceeded their quota.") + retry_count = 3 + retry_delay = 5 + top_n = min(int(top_n), 50) # API max + developer_keys = developer_keys or self.DEVELOPER_KEYS + for key in developer_keys: + for i in range(retry_count): + try: + youtube = googleapiclient.discovery.build(api_service_name, api_version, developerKey=key) + request = youtube.search().list( + part="snippet", + maxResults=top_n, + type="video", + q=query, + relevanceLanguage="en", + ) + return request.execute(), youtube + except (ConnectionAbortedError, ConnectionResetError, socket.timeout) as e: + logger.exception("Transient connection error on attempt %d/%d (key ****%s)", i + 1, retry_count, key[-4:]) + time.sleep(retry_delay * (2 ** i)) + continue + except HttpError as e: + status = getattr(getattr(e, "resp", None), "status", None) + msg = str(e).lower() + if status == 403 and ("quotaexceeded" in msg or "dailylimitexceeded" in msg or "quota" in msg): + logger.warning("Quota exceeded for key ****%s; switching key…", key[-4:]) + break # next key + raise + raise RuntimeError("All YouTube API keys exhausted or failed.")Additionally apply outside this range:
# at top of file import socket # replace 'from socket import *' # and remove the star import usage in excepts (done in diff above).
73-76: Log exceptions with traceback; avoid double error linesUse logger.exception once; also avoid relying on
timeoutfrom star-import.- except (ConnectionAbortedError, ConnectionResetError, timeout) as e: - logger.error("Error while getting the videos") - logger.error(e) + except (ConnectionAbortedError, ConnectionResetError, socket.timeout) as e: + logger.exception("Error while getting the videos")Outside this range, replace
from socket import *withimport socket.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
coursemapper-kg/recommendation/app/services/course_materials/recommendation/youtube_service.py(3 hunks)
🧰 Additional context used
🪛 Gitleaks (8.27.2)
coursemapper-kg/recommendation/app/services/course_materials/recommendation/youtube_service.py
47-47: Uncovered a GCP API key, which could lead to unauthorized access to Google Cloud services and data breaches.
(gcp-api-key)
48-48: Uncovered a GCP API key, which could lead to unauthorized access to Google Cloud services and data breaches.
(gcp-api-key)
49-49: Uncovered a GCP API key, which could lead to unauthorized access to Google Cloud services and data breaches.
(gcp-api-key)
50-50: Uncovered a GCP API key, which could lead to unauthorized access to Google Cloud services and data breaches.
(gcp-api-key)
51-51: Uncovered a GCP API key, which could lead to unauthorized access to Google Cloud services and data breaches.
(gcp-api-key)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Build container images / Build image (coursemapper-kg/preprocess)
- GitHub Check: Build container images / Build image (coursemapper-kg/concept-map)
- GitHub Check: Build container images / Build image (coursemapper-kg/recommendation)
- GitHub Check: Build container images / Build image (webapp)
🔇 Additional comments (1)
coursemapper-kg/recommendation/app/services/course_materials/recommendation/youtube_service.py (1)
193-199: Good: exception visibility improvedCatching HttpError and logging with traceback is appropriate.
| import logging | ||
| from log import LOG | ||
| import time | ||
| from googleapiclient.errors import HttpError |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Minor: clean imports
Now that HttpError is imported directly, drop import googleapiclient.errors. Also remove duplicate import time (present twice).
-from youtube_transcript_api import YouTubeTranscriptApi
+from youtube_transcript_api import YouTubeTranscriptApi
@@
-import googleapiclient.errors
+# googleapiclient.errors not needed; using HttpError directly
@@
-import time
+# remove duplicate 'import time' if present twiceCommittable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In
coursemapper-kg/recommendation/app/services/course_materials/recommendation/youtube_service.py
around line 15, tidy imports by removing the redundant module import and
duplicate import: delete the now-unnecessary "import googleapiclient.errors"
since HttpError is imported directly from googleapiclient.errors, and remove the
duplicated "import time" so time is only imported once; ensure no other
references depend on the removed module import.
| except HttpError as e: | ||
| if e.resp.status == 403 and "quota" in str(e): | ||
| print(f"Quota exceeded for key: {key}. Trying next key...") | ||
| else: | ||
| raise e |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Replace print with structured logging; broaden quota reasons
Use logger and check standard reasons.
- if e.resp.status == 403 and "quota" in str(e):
- print(f"Quota exceeded for key: {key}. Trying next key...")
+ if e.resp.status == 403 and any(r in str(e).lower() for r in ["quotaexceeded", "dailylimitexceeded", "quota"]):
+ logger.warning("Quota exceeded for key ****%s. Trying next key…", key[-4:])📝 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.
| except HttpError as e: | |
| if e.resp.status == 403 and "quota" in str(e): | |
| print(f"Quota exceeded for key: {key}. Trying next key...") | |
| else: | |
| raise e | |
| except HttpError as e: | |
| if e.resp.status == 403 and any(r in str(e).lower() for r in ["quotaexceeded", "dailylimitexceeded", "quota"]): | |
| logger.warning("Quota exceeded for key ****%s. Trying next key…", key[-4:]) | |
| else: | |
| raise e |
🤖 Prompt for AI Agents
In
coursemapper-kg/recommendation/app/services/course_materials/recommendation/youtube_service.py
around lines 84-88, replace the plain print with structured logging and broaden
the quota detection: use the module or instance logger (e.g., logger.warning) to
log a clear message that includes the key and exception details (include
exc_info or the exception object), and detect quota errors not only by checking
if "quota" is in the string but also by inspecting standard API error reasons
such as "quotaExceeded", "dailyLimitExceeded" or the HttpError response
body/reason; if a quota-related condition is detected, log the warning and
continue to the next key, otherwise re-raise the exception.
| response, youtube_api_sinlge = self.search_youtube_videos( | ||
| developer_keys=self.DEVELOPER_KEYS, query=concepts, top_n=top_n | ||
| ) |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Typo: youtube_api_sinlge → youtube_api_single
Fix naming for readability and to avoid propagating typos.
- response, youtube_api_sinlge = self.search_youtube_videos(
+ response, youtube_api_single = self.search_youtube_videos(
developer_keys=self.DEVELOPER_KEYS, query=concepts, top_n=top_n
)📝 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.
| response, youtube_api_sinlge = self.search_youtube_videos( | |
| developer_keys=self.DEVELOPER_KEYS, query=concepts, top_n=top_n | |
| ) | |
| response, youtube_api_single = self.search_youtube_videos( | |
| developer_keys=self.DEVELOPER_KEYS, query=concepts, top_n=top_n | |
| ) |
🤖 Prompt for AI Agents
In
coursemapper-kg/recommendation/app/services/course_materials/recommendation/youtube_service.py
around lines 104 to 106, there's a typo in the variable name
"youtube_api_sinlge" — rename it to "youtube_api_single" consistently where it's
assigned and anywhere else it's referenced to improve readability and avoid
further typos; update the function return unpacking to use youtube_api_single
and search for other occurrences of the misspelled identifier in the file and
replace them to keep names consistent.
| if len(response["items"]) == 0: | ||
| logger.info("No Video found for this input") | ||
| return [] | ||
| else: |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Guard against None/empty responses and return consistent type
Return an empty DataFrame (not list) to keep a stable return type.
- if len(response["items"]) == 0:
+ if not response or not response.get("items"):
logger.info("No Video found for this input")
- return []
+ return pd.DataFrame()📝 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.
| if len(response["items"]) == 0: | |
| logger.info("No Video found for this input") | |
| return [] | |
| else: | |
| if not response or not response.get("items"): | |
| logger.info("No Video found for this input") | |
| return pd.DataFrame() | |
| else: |
🤖 Prompt for AI Agents
In
coursemapper-kg/recommendation/app/services/course_materials/recommendation/youtube_service.py
around lines 108 to 111, the code returns an empty list when no videos are
found; change this to return an empty pandas DataFrame to maintain a consistent
return type. Guard against response being None or missing the "items" key before
accessing it (e.g., check response and "items" in response), and return
pandas.DataFrame() when there are no items or response is invalid so callers
always receive a DataFrame.
| for index, id in enumerate(df_ids["id"]): | ||
| # try: | ||
| # df_snippet["text"][index] = df_snippet["text"][index] + ". " + get_subtitles(id) | ||
| # df_snippet["text"][index] = df_snippet["text"][index] + ". " + get_subtitles(id) | ||
| # except (NoTranscriptFound, TranscriptsDisabled) as e: | ||
| # logger.error("No transcript found in english or transcript disabled for this video " | ||
| # "https://www.youtube.com/watch?v={} ".format(id)) | ||
| # logger.error("No transcript found in english or transcript disabled for this video " | ||
| # "https://www.youtube.com/watch?v={} ".format(id)) | ||
|
|
||
| try: | ||
| duration, views, description = self.get_video_details(id) | ||
| duration = re.findall(r"\d+", duration) | ||
| duration = ":".join(duration) | ||
| # print(id, duration, views) | ||
| duration_list.append(duration) | ||
| view_list.append(views) | ||
| description_list.append(description) | ||
| except Exception as e: | ||
| logger.error("Error while getting the videos details", e) | ||
| res = self.get_video_details(youtube_api_sinlge, id) | ||
| if res is None: | ||
| raise ValueError("No details returned for video id {}".format(id)) |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Avoid shadowing built-in id and improve error
Rename loop var to video_id; keep error message intact.
- for index, id in enumerate(df_ids["id"]):
+ for index, video_id in enumerate(df_ids["id"]):
@@
- res = self.get_video_details(youtube_api_sinlge, id)
+ res = self.get_video_details(youtube_api_single, video_id)
- if res is None:
- raise ValueError("No details returned for video id {}".format(id))
+ if res is None:
+ raise ValueError("No details returned for video id {}".format(video_id))📝 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.
| for index, id in enumerate(df_ids["id"]): | |
| # try: | |
| # df_snippet["text"][index] = df_snippet["text"][index] + ". " + get_subtitles(id) | |
| # df_snippet["text"][index] = df_snippet["text"][index] + ". " + get_subtitles(id) | |
| # except (NoTranscriptFound, TranscriptsDisabled) as e: | |
| # logger.error("No transcript found in english or transcript disabled for this video " | |
| # "https://www.youtube.com/watch?v={} ".format(id)) | |
| # logger.error("No transcript found in english or transcript disabled for this video " | |
| # "https://www.youtube.com/watch?v={} ".format(id)) | |
| try: | |
| duration, views, description = self.get_video_details(id) | |
| duration = re.findall(r"\d+", duration) | |
| duration = ":".join(duration) | |
| # print(id, duration, views) | |
| duration_list.append(duration) | |
| view_list.append(views) | |
| description_list.append(description) | |
| except Exception as e: | |
| logger.error("Error while getting the videos details", e) | |
| res = self.get_video_details(youtube_api_sinlge, id) | |
| if res is None: | |
| raise ValueError("No details returned for video id {}".format(id)) | |
| for index, video_id in enumerate(df_ids["id"]): | |
| # try: | |
| # df_snippet["text"][index] = df_snippet["text"][index] + ". " + get_subtitles(id) | |
| # except (NoTranscriptFound, TranscriptsDisabled) as e: | |
| # logger.error("No transcript found in english or transcript disabled for this video " | |
| # "https://www.youtube.com/watch?v={} ".format(id)) | |
| try: | |
| - res = self.get_video_details(youtube_api_sinlge, id) | |
| - if res is None: | |
| res = self.get_video_details(youtube_api_single, video_id) | |
| if res is None: | |
| raise ValueError("No details returned for video id {}".format(video_id)) |
🤖 Prompt for AI Agents
In
coursemapper-kg/recommendation/app/services/course_materials/recommendation/youtube_service.py
around lines 127 to 137, the loop variable currently named "id" shadows Python's
built-in id() and should be renamed to "video_id"; update the for statement and
every reference inside the loop (e.g., pass video_id to get_video_details and to
the error message string.format) so behavior and the existing error message
remain the same while avoiding built-in shadowing.
| # Keep your duration normalization | ||
| duration = re.findall(r"\d+", str(duration)) | ||
| duration = ":".join(duration) | ||
| except Exception: | ||
| # Fix logging formatter error and keep same message semantics | ||
| logger.exception("Error while getting the videos details for id %s", id) | ||
| # Append safe defaults to keep list lengths aligned with df_ids | ||
| duration = "0" | ||
| views = 0 | ||
| description = "" | ||
| like_count = 0 | ||
| channel_title = "" | ||
|
|
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Normalize duration properly; keep numeric types for counts
Parse ISO-8601 with isodate if available; coerce counts to ints.
- duration = re.findall(r"\d+", str(duration))
- duration = ":".join(duration)
+ # Prefer true ISO-8601 parsing if available; fallback kept
+ try:
+ import isodate
+ duration = int(isodate.parse_duration(str(duration)).total_seconds())
+ except Exception:
+ duration = ":".join(re.findall(r"\d+", str(duration)))
@@
- views = 0
+ views = 0
@@
- like_count = 0
+ like_count = 0And when appending (below), coerce to numeric (see next comment).
📝 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.
| # Keep your duration normalization | |
| duration = re.findall(r"\d+", str(duration)) | |
| duration = ":".join(duration) | |
| except Exception: | |
| # Fix logging formatter error and keep same message semantics | |
| logger.exception("Error while getting the videos details for id %s", id) | |
| # Append safe defaults to keep list lengths aligned with df_ids | |
| duration = "0" | |
| views = 0 | |
| description = "" | |
| like_count = 0 | |
| channel_title = "" | |
| # Keep your duration normalization | |
| - duration = re.findall(r"\d+", str(duration)) | |
| # Prefer true ISO-8601 parsing if available; fallback to regex join | |
| try: | |
| import isodate | |
| duration = int(isodate.parse_duration(str(duration)).total_seconds()) | |
| except Exception: | |
| duration = ":".join(re.findall(r"\d+", str(duration))) | |
| except Exception: | |
| # Fix logging formatter error and keep same message semantics | |
| logger.exception("Error while getting the videos details for id %s", id) | |
| # Append safe defaults to keep list lengths aligned with df_ids | |
| duration = "0" | |
| views = 0 | |
| description = "" | |
| like_count = 0 | |
| channel_title = "" |
🤖 Prompt for AI Agents
In
coursemapper-kg/recommendation/app/services/course_materials/recommendation/youtube_service.py
around lines 141 to 153, the duration and numeric fields are being treated as
strings and not normalized consistently; update the exception handling and
normal parsing to (1) parse ISO-8601 durations using isodate.parse_duration
(fall back to existing regex only if isodate is unavailable) and normalize to a
canonical "H:MM:SS" or "MM:SS" string for duration, (2) ensure counts like views
and like_count are coerced to int (use int(...) with safe default 0) and
description/channel_title stay as strings, and (3) keep the existing
logger.exception call but maintain its safe formatting; after fixing here,
ensure any append/population logic below appends ints for numeric fields and the
normalized duration string so list lengths and types remain consistent.
| # Assign lists (same behavior as before, now lengths aligned) | ||
| video_data["duration"] = pd.Series(duration_list) | ||
| video_data["views"] = pd.Series(view_list) | ||
| video_data["description_full"] = pd.Series(description_list) | ||
| video_data["like_count"] = pd.Series(like_count_list) | ||
| video_data["channel_title"] = pd.Series(channel_title_list) | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Preserve numeric dtypes for views/likes; avoid implicit object dtype
Cast with to_numeric and nullable Int64.
- video_data["views"] = pd.Series(view_list)
+ video_data["views"] = pd.to_numeric(pd.Series(view_list), errors="coerce").fillna(0).astype("Int64")
@@
- video_data["like_count"] = pd.Series(like_count_list)
+ video_data["like_count"] = pd.to_numeric(pd.Series(like_count_list), errors="coerce").fillna(0).astype("Int64")📝 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.
| # Assign lists (same behavior as before, now lengths aligned) | |
| video_data["duration"] = pd.Series(duration_list) | |
| video_data["views"] = pd.Series(view_list) | |
| video_data["description_full"] = pd.Series(description_list) | |
| video_data["like_count"] = pd.Series(like_count_list) | |
| video_data["channel_title"] = pd.Series(channel_title_list) | |
| # Assign lists (same behavior as before, now lengths aligned) | |
| video_data["duration"] = pd.Series(duration_list) | |
| video_data["views"] = pd.to_numeric(pd.Series(view_list), errors="coerce").fillna(0).astype("Int64") | |
| video_data["description_full"] = pd.Series(description_list) | |
| video_data["like_count"] = pd.to_numeric(pd.Series(like_count_list), errors="coerce").fillna(0).astype("Int64") | |
| video_data["channel_title"] = pd.Series(channel_title_list) |
🤖 Prompt for AI Agents
In
coursemapper-kg/recommendation/app/services/course_materials/recommendation/youtube_service.py
around lines 163 to 169, assigning views and like_count directly from lists
produces object dtype; convert these Series to numeric using
pandas.to_numeric(..., errors='coerce') and then cast to the nullable Integer
dtype .astype("Int64") so missing/non-numeric values become <NA> instead of
strings; update the assignment for video_data["views"] and
video_data["like_count"] to build Series from the lists, apply to_numeric with
errors='coerce', and then .astype("Int64").
| video_data["text"] = pd.DataFrame( | ||
| video_data["title"] + ". " + video_data["description"] | ||
| ) |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Assign Series, not DataFrame, when building text
Avoid wrapping in DataFrame; also coerce to str to be safe.
- video_data["text"] = pd.DataFrame(
- video_data["title"] + ". " + video_data["description"]
- )
+ video_data["text"] = video_data["title"].astype(str) + ". " + video_data["description"].astype(str)📝 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.
| video_data["text"] = pd.DataFrame( | |
| video_data["title"] + ". " + video_data["description"] | |
| ) | |
| video_data["text"] = video_data["title"].astype(str) + ". " + video_data["description"].astype(str) |
🤖 Prompt for AI Agents
In
coursemapper-kg/recommendation/app/services/course_materials/recommendation/youtube_service.py
around lines 171 to 173, the code wraps the concatenated title and description
in a DataFrame; instead assign a pandas Series and ensure both title and
description are coerced to strings. Replace the DataFrame construction with a
Series (e.g., pd.Series((video_data["title"].astype(str) + ". " +
video_data["description"].astype(str)))) so text is a Series of strings rather
than a DataFrame.
| def get_video_details(self, youtube_api_sinlge, video_id): | ||
| # print("get_video_details for id -------------------- ", video_id) | ||
| try: | ||
| duration = ( | ||
| r["items"][0]["contentDetails"]["duration"] | ||
| if r["items"][0]["contentDetails"]["duration"] | ||
| else 0 | ||
| ) | ||
| views = ( | ||
| r["items"][0]["statistics"]["viewCount"] | ||
| if r["items"][0]["statistics"]["viewCount"] | ||
| else 0 | ||
| ) | ||
| description = ( | ||
| r["items"][0]["snippet"]["description"] | ||
| if r["items"][0]["snippet"]["description"] | ||
| else "" | ||
| ) | ||
|
|
||
| except Exception as e: | ||
| print("---------------------------------------") | ||
| print(e) | ||
| # The number of views are not present for some videos and this leads to an exception. For this | ||
| # reason a default value of 0 views will be given that video. | ||
| views = 0 | ||
| duration = ( | ||
| r["items"][0]["contentDetails"]["duration"] | ||
| if r["items"][0]["contentDetails"]["duration"] | ||
| else 0 | ||
| ) | ||
| description = ( | ||
| r["items"][0]["snippet"]["description"] | ||
| if r["items"][0]["snippet"]["description"] | ||
| else "" | ||
| r = ( | ||
| # self.youtube.videos() | ||
| youtube_api_sinlge.videos() | ||
| .list( |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Typo and minimize API fields
Fix param name and narrow fields to what you actually read.
- def get_video_details(self, youtube_api_sinlge, video_id):
+ def get_video_details(self, youtube_api_single, video_id):
@@
- youtube_api_sinlge.videos()
+ youtube_api_single.videos()
.list(
- part="snippet,statistics,contentDetails",
+ part="snippet,statistics,contentDetails",
id=video_id,
- fields="items(statistics,contentDetails(duration),snippet)",
+ fields="items(statistics(viewCount,likeCount),contentDetails(duration),snippet(description,channelTitle))",
)📝 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.
| def get_video_details(self, youtube_api_sinlge, video_id): | |
| # print("get_video_details for id -------------------- ", video_id) | |
| try: | |
| duration = ( | |
| r["items"][0]["contentDetails"]["duration"] | |
| if r["items"][0]["contentDetails"]["duration"] | |
| else 0 | |
| ) | |
| views = ( | |
| r["items"][0]["statistics"]["viewCount"] | |
| if r["items"][0]["statistics"]["viewCount"] | |
| else 0 | |
| ) | |
| description = ( | |
| r["items"][0]["snippet"]["description"] | |
| if r["items"][0]["snippet"]["description"] | |
| else "" | |
| ) | |
| except Exception as e: | |
| print("---------------------------------------") | |
| print(e) | |
| # The number of views are not present for some videos and this leads to an exception. For this | |
| # reason a default value of 0 views will be given that video. | |
| views = 0 | |
| duration = ( | |
| r["items"][0]["contentDetails"]["duration"] | |
| if r["items"][0]["contentDetails"]["duration"] | |
| else 0 | |
| ) | |
| description = ( | |
| r["items"][0]["snippet"]["description"] | |
| if r["items"][0]["snippet"]["description"] | |
| else "" | |
| r = ( | |
| # self.youtube.videos() | |
| youtube_api_sinlge.videos() | |
| .list( | |
| def get_video_details(self, youtube_api_single, video_id): | |
| # print("get_video_details for id -------------------- ", video_id) | |
| try: | |
| r = ( | |
| # self.youtube.videos() | |
| youtube_api_single.videos() | |
| .list( | |
| part="snippet,statistics,contentDetails", | |
| id=video_id, | |
| fields="items(statistics(viewCount,likeCount),contentDetails(duration),snippet(description,channelTitle))", | |
| ) | |
| ) | |
| # …rest of method… |
| # Safe gets with defaults (keep same return semantics) | ||
| duration = cd.get("duration") if cd.get("duration") else 0 | ||
| views = st.get("viewCount") if st.get("viewCount") else 0 | ||
| description = sn.get("description") if sn.get("description") else "" | ||
| like_count = st.get("likeCount") if st.get("likeCount") else 0 | ||
| channel_title = sn.get("channelTitle") if sn.get("channelTitle") else "" | ||
|
|
||
| return duration, views, description, like_count, channel_title |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Return typed values; default safely
Coerce counts to int and keep empty strings for text fields.
- duration = cd.get("duration") if cd.get("duration") else 0
- views = st.get("viewCount") if st.get("viewCount") else 0
- description = sn.get("description") if sn.get("description") else ""
- like_count = st.get("likeCount") if st.get("likeCount") else 0
- channel_title = sn.get("channelTitle") if sn.get("channelTitle") else ""
+ duration = cd.get("duration") or 0
+ views = int(st.get("viewCount") or 0)
+ description = sn.get("description") or ""
+ like_count = int(st.get("likeCount") or 0)
+ channel_title = sn.get("channelTitle") or ""📝 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.
| # Safe gets with defaults (keep same return semantics) | |
| duration = cd.get("duration") if cd.get("duration") else 0 | |
| views = st.get("viewCount") if st.get("viewCount") else 0 | |
| description = sn.get("description") if sn.get("description") else "" | |
| like_count = st.get("likeCount") if st.get("likeCount") else 0 | |
| channel_title = sn.get("channelTitle") if sn.get("channelTitle") else "" | |
| return duration, views, description, like_count, channel_title | |
| # Safe gets with defaults (keep same return semantics) | |
| duration = cd.get("duration") or 0 | |
| views = int(st.get("viewCount") or 0) | |
| description = sn.get("description") or "" | |
| like_count = int(st.get("likeCount") or 0) | |
| channel_title = sn.get("channelTitle") or "" | |
| return duration, views, description, like_count, channel_title |
🤖 Prompt for AI Agents
In
coursemapper-kg/recommendation/app/services/course_materials/recommendation/youtube_service.py
around lines 210 to 217, the numeric fields are returned as-is and may be
strings or None; coerce the count/duration fields to ints and preserve empty
strings for text fields by using safe defaults. Replace the current assignments
with expressions that use the .get(... ) or fallback and coerce to int (e.g.
int(cd.get("duration") or 0), int(st.get("viewCount") or 0),
int(st.get("likeCount") or 0)) and keep description and channel_title as
sn.get("description") or "" and sn.get("channelTitle") or "" so return types are
int, int, str, int, str.
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
coursemapper-kg/recommendation/app/services/course_materials/GCN/gcn.py (2)
22-24: Don't disable encryption for Neo4j in production.
encrypted=Falseexposes data in transit. Make it configurable and default to True (or use aneo4j+sURI).Apply:
- self.driver = GraphDatabase.driver(neo4j_uri, - auth=(neo4j_user, neo4j_pass), - encrypted=False) + self.driver = GraphDatabase.driver( + neo4j_uri, + auth=(neo4j_user, neo4j_pass), + encrypted=getattr(Config, "NEO4J_ENCRYPTED", True), + )
87-97: Batch Neo4j writes via UNWIND to avoid N round-trips.One query per node will be slow at scale.
Apply:
- with self.driver.session() as session: - for i in range(final_embeddings.shape[0]): - id = idx[i] - f_embedding = final_embeddings[i] - embedding = ",".join(str(i) for i in f_embedding) - # Find a node in neo4j by its original id and save its final embedding into its "final_embedding" property - result = session.run("""MATCH (n) WHERE n.cid= $id or n.sid= $id - set n.final_embedding = $embedding RETURN n""", - id=id, - embedding=embedding) + rows = [ + {"id": str(idx[i]), "embedding": ",".join(map(str, final_embeddings[i]))} + for i in range(final_embeddings.shape[0]) + ] + with self.driver.session() as session: + session.run( + """ + UNWIND $rows AS row + MATCH (n) WHERE n.cid = row.id OR n.sid = row.id + SET n.final_embedding = row.embedding + """, + rows=rows, + )
♻️ Duplicate comments (1)
coursemapper-kg/recommendation/app/services/course_materials/GCN/gcn.py (1)
37-37: Prefer debug-level, shape-only logs over commenting out.Same feedback as before; keep lightweight observability.
Apply:
- # logger.info(idx_features.shape[0]) + logger.debug("idx_features count: %d", idx_features.shape[0]) @@ - # logger.info(features.A) + logger.debug("features shape: %s", features.shape) @@ - # logger.info(adj.A) + logger.debug("adj shape: %s", adj.shape) @@ - # logger.info(final_embeddings) + logger.debug("final_embeddings shape: %s", final_embeddings.shape)Also applies to: 41-41, 74-74, 83-83
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
coursemapper-kg/recommendation/app/services/course_materials/GCN/gcn.py(4 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Build container images / Build image (coursemapper-kg/preprocess)
- GitHub Check: Build container images / Build image (webapp)
- GitHub Check: Build container images / Build image (coursemapper-kg/concept-map)
- GitHub Check: Build container images / Build image (coursemapper-kg/recommendation)
🔇 Additional comments (2)
coursemapper-kg/recommendation/app/services/course_materials/GCN/gcn.py (2)
69-69: No-op formatting change.Safe to keep or drop per project style.
126-127: LGTM;where=supported since NumPy 1.17 and pinned version ==1.26.4 satisfies the requirement.
| @@ -66,11 +66,12 @@ def load_data(self): | |||
| dtype=np.float32, | |||
| ) | |||
| adj = np.around(adj, 2) | |||
There was a problem hiding this comment.
np.around densifies sparse matrices — major perf/memory bug.
Rounding the whole sparse matrix converts it to dense; use .data instead.
Apply:
- adj = np.around(adj, 2)
+ # Round only nonzero weights to keep sparsity
+ adj = adj.tocoo()
+ adj.data = np.around(adj.data, 2)📝 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.
| adj = np.around(adj, 2) | |
| # Round only nonzero weights to keep sparsity | |
| adj = adj.tocoo() | |
| adj.data = np.around(adj.data, 2) |
🤖 Prompt for AI Agents
In coursemapper-kg/recommendation/app/services/course_materials/GCN/gcn.py
around line 68, calling np.around on the entire sparse adjacency matrix
densifies it and causes major perf/memory issues; instead apply rounding only to
the sparse data array (the .data attribute) and keep the matrix as a sparse
type, i.e., update the matrix's internal data with rounded values and do not
convert or recreate a dense matrix.
| # matrix plus its unit matrix and transpose matrix to obtain the complete adjacency matrix | ||
| adj = adj + adj.T.multiply(adj.T > adj) - adj.multiply(adj.T > adj) | ||
|
|
||
| adj = self.normalize(adj) + sp.eye(adj.shape[0]) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add self-loops before normalization (GCN convention).
Current order changes weights vs. Kipf & Welling’s  = D̂^{-1/2}(A+I)D̂^{-1/2}.
Apply:
- adj = self.normalize(adj) + sp.eye(adj.shape[0])
+ # Add self-loops, then normalize
+ adj = self.normalize(adj + sp.eye(adj.shape[0]))📝 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.
| adj = self.normalize(adj) + sp.eye(adj.shape[0]) | |
| # Add self-loops, then normalize | |
| adj = self.normalize(adj + sp.eye(adj.shape[0])) |
🤖 Prompt for AI Agents
In coursemapper-kg/recommendation/app/services/course_materials/GCN/gcn.py
around line 73 the code adds self-loops after calling normalize which changes
the resulting weights versus the standard GCN convention; move the addition of
the identity so you add self-loops to adj before calling self.normalize (i.e.,
compute adj_with_loops = adj + sp.eye(adj.shape[0]) then call
self.normalize(adj_with_loops)), and ensure the normalize function computes
degrees and performs symmetric normalization on that augmented matrix.
Bumps nginxinc/nginx-unprivileged from 1.29.1-alpine to 1.29.3-alpine. --- updated-dependencies: - dependency-name: nginxinc/nginx-unprivileged dependency-version: 1.29.3-alpine dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com>
Bumps nginxinc/nginx-unprivileged from 1.29.0-alpine to 1.29.3-alpine. --- updated-dependencies: - dependency-name: nginxinc/nginx-unprivileged dependency-version: 1.29.3-alpine dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com>
Bumps [actions/checkout](https://github.com/actions/checkout) from 4 to 6. - [Release notes](https://github.com/actions/checkout/releases) - [Changelog](https://github.com/actions/checkout/blob/main/CHANGELOG.md) - [Commits](actions/checkout@v4...v6) --- updated-dependencies: - dependency-name: actions/checkout dependency-version: '6' dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com>
Update user agent in the response of search query + DNS API
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
webapp/Dockerfile (1)
20-32:⚠️ Potential issue | 🟡 MinorAdd
HEALTHCHECKto the runtime image.The final NGINX stage exposes port 4200 but has no healthcheck, reducing failure detection in container orchestration.
Suggested patch
FROM nginxinc/nginx-unprivileged:1.29.3-alpine @@ ENV NGINX_ENTRYPOINT_QUIET_LOGS=1 +HEALTHCHECK --interval=30s --timeout=3s --start-period=10s --retries=3 \ + CMD wget -q -O /dev/null http://127.0.0.1:4200/ || exit 1 EXPOSE 4200🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@webapp/Dockerfile` around lines 20 - 32, The Dockerfile final stage (FROM nginxinc/nginx-unprivileged:1.29.3-alpine) exposes port 4200 but lacks a HEALTHCHECK; add a HEALTHCHECK instruction after ENV NGINX_ENTRYPOINT_QUIET_LOGS and before or after EXPOSE 4200 that probes the running nginx (for example an HTTP GET against localhost:4200 or the nginx status endpoint) with sensible interval/retries/timeout settings so the container runtime can detect unhealthy instances; ensure the check runs as the unprivileged user (USER 101) or uses a simple curl/wget shell command available in the image, and reference the existing ENV NGINX_ENTRYPOINT_QUIET_LOGS and EXPOSE 4200 in the commit so reviewers can locate the change.proxy/Dockerfile (1)
1-7:⚠️ Potential issue | 🟡 MinorAdd a container
HEALTHCHECKfor runtime reliability.There is no healthcheck in this image, so orchestrators cannot detect unhealthy NGINX workers.
Suggested patch
FROM nginxinc/nginx-unprivileged:1.29.3-alpine USER root COPY --link ./nginx/conf.d/* /etc/nginx/conf.d/ +HEALTHCHECK --interval=30s --timeout=3s --start-period=10s --retries=3 \ + CMD wget -q -O /dev/null http://127.0.0.1:8000/ || exit 1 + EXPOSE 8000🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@proxy/Dockerfile` around lines 1 - 7, Add a Docker HEALTHCHECK to the Dockerfile so orchestrators can detect unhealthy NGINX workers: update the Dockerfile (the image line FROM nginxinc/nginx-unprivileged:1.29.3-alpine / COPY and EXPOSE 8000 context) to include a HEALTHCHECK instruction (using CMD-SHELL) that probes localhost:8000 (e.g., curl/wget --fail or ncat) with sensible parameters (interval, timeout, retries); place it after the COPY/EXPOSE lines and ensure it exits non-zero on failure so the container is marked unhealthy.coursemapper-kg/concept-map/src/services/annotation.py (1)
11-29:⚠️ Potential issue | 🟠 MajorInconsistent DBpedia Spotlight endpoint configuration across services.
The concept-map service uses environment variable configuration with a DNS override fallback mechanism, but the recommendation service modules hardcode the same DBpedia Spotlight endpoint in four separate locations without any configuration or DNS handling:
coursemapper-kg/recommendation/app/services/course_materials/kwp_extraction/dbpedia/dataAvailability.pycoursemapper-kg/recommendation/app/services/course_materials/kwp_extraction/dbpedia/concept_tagging.pycoursemapper-kg/recommendation/app/services/course_materials/kwp_extraction/dbpedia/concept_tagging_top_down.pycoursemapper-kg/recommendation/app/services/course_materials/kwp_extraction/dbpedia/concept_tagging Paul.pyAll use:
self.url = "https://api.dbpedia-spotlight.org/%s/annotate" % langMeanwhile, concept-map uses
Config.DBPEDIA_SPOTLIGHT_URL(from environment) and includes a DNS override mechanism to resolveapi.dbpedia-spotlight.orgto134.155.98.34.Impact: This creates a configuration management burden—any endpoint change requires editing multiple hardcoded locations in recommendation services, while concept-map uses environment configuration. The DNS override mechanism in concept-map suggests a specific deployment requirement (firewall, load balancer, specific IP) that recommendation services cannot benefit from.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@coursemapper-kg/concept-map/src/services/annotation.py` around lines 11 - 29, The recommendation modules hardcode the DBpedia Spotlight endpoint in multiple places (self.url = "https://api.dbpedia-spotlight.org/%s/annotate" % lang) instead of using the centralized configuration and DNS override used in concept-map; update each recommendation class that sets self.url (in dataAvailability.py, concept_tagging.py, concept_tagging_top_down.py, and concept_tagging Paul.py) to read the endpoint from the shared configuration (use Config.DBPEDIA_SPOTLIGHT_URL or an equivalent environment-backed config value) and remove the hardcoded literal, and ensure any deployment-specific DNS override logic (the dns_cache/override_dns/new_getaddrinfo mechanism) is provided centrally at application startup so the recommendation services inherit the same DNS behavior rather than implementing ad-hoc URLs.
♻️ Duplicate comments (1)
proxy/Dockerfile (1)
2-2:⚠️ Potential issue | 🟠 MajorPin the NGINX image to a digest, not a floating tag.
Line 2 still uses a mutable tag, which weakens reproducibility and supply-chain control.
#!/bin/bash # Verify and retrieve the digest for nginxinc/nginx-unprivileged:1.29.3-alpine set -euo pipefail token="$(curl -fsSL 'https://auth.docker.io/token?service=registry.docker.io&scope=repository:nginxinc/nginx-unprivileged:pull' | jq -r '.token')" curl -fsSI \ -H "Authorization: Bearer ${token}" \ -H "Accept: application/vnd.docker.distribution.manifest.v2+json" \ "https://registry-1.docker.io/v2/nginxinc/nginx-unprivileged/manifests/1.29.3-alpine" \ | awk 'BEGIN{IGNORECASE=1} /docker-content-digest/ {print $0}'Expected result: a
docker-content-digestheader you can pin inFROM nginxinc/nginx-unprivileged@sha256:....🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@proxy/Dockerfile` at line 2, Replace the floating image tag in the Dockerfile's FROM instruction (currently "FROM nginxinc/nginx-unprivileged:1.29.3-alpine") with the corresponding immutable digest; obtain the image digest using the Docker registry manifest endpoint (or the provided verification snippet) and update the line to use "FROM nginxinc/nginx-unprivileged@sha256:..." so the build is pinned to the specific content-addressable image.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/release.yml:
- Line 14: The workflow uses the mutable tag actions/checkout@v6 which is a
supply-chain risk; replace that tag with the full immutable commit SHA for the
v6 release (i.e., change uses: actions/checkout@v6 to uses:
actions/checkout@<FULL_COMMIT_SHA>) and add a trailing comment with the
human-friendly tag/version (e.g., # v6.x.y) for maintainability so reviewers can
see which release the SHA corresponds to.
In `@coursemapper-kg/concept-map/src/services/annotation.py`:
- Line 29: Apply the DNS override consistently by centralizing the DBpedia
Spotlight endpoint: create a shared configuration (e.g.,
DBPEDIA_SPOTLIGHT_HOST/DBPEDIA_SPOTLIGHT_URL) or shared init module that calls
override_dns('api.dbpedia-spotlight.org','134.155.98.34') and export the
canonical endpoint, then update annotation.py to import that shared init
(instead of calling override_dns inline) and change the other service that
hardcodes "https://api.dbpedia-spotlight.org/%s/annotate" (see
course_materials/.../kwp_extraction/dbpedia/concept_tagging.py) to use the
shared DBPEDIA_SPOTLIGHT_URL or host variable so all services (including
AnnotationService) resolve via the same override/config.
In `@webapp/Dockerfile`:
- Line 2: Replace the mutable base image tags in the Dockerfile with the
provided immutable digests: update the build stage FROM reference (currently
"node:24.0-slim" in the FROM line used by the build stage) to the pinned digest
"node:24.0-slim@sha256:083430e81f23ca4f309c6de17614d20706ddd544b2adc71fb9fdd86e2371360a",
and update the final runtime FROM reference (currently
"nginxinc/nginx-unprivileged:1.29.3-alpine") to the pinned digest
"nginxinc/nginx-unprivileged:1.29.3-alpine@sha256:5aea7cc516b419e3526f47dd1531be31a56a046cfe44754d94f9383e13e2ee99"
so rebuilds are deterministic.
In `@webserver/src/controllers/knowledgeGraph.controller.js`:
- Around line 515-522: Replace the dead commented axios call and update the
User-Agent header used in the axios.get request inside the code that performs
the Wikimedia API fetch: remove the leftover "// const response = await
axios.get(url);" line and change "User-Agent": "CourseMapper
(coursemapper@example.com)" to a real contact string (project URL or maintainer
email) per Wikimedia policy (e.g., "CourseMapper/1.0
(+https://yourproject.example.com)" or include a real maintainer email) in the
axios.get call so Wikimedia can contact the operator.
---
Outside diff comments:
In `@coursemapper-kg/concept-map/src/services/annotation.py`:
- Around line 11-29: The recommendation modules hardcode the DBpedia Spotlight
endpoint in multiple places (self.url =
"https://api.dbpedia-spotlight.org/%s/annotate" % lang) instead of using the
centralized configuration and DNS override used in concept-map; update each
recommendation class that sets self.url (in dataAvailability.py,
concept_tagging.py, concept_tagging_top_down.py, and concept_tagging Paul.py) to
read the endpoint from the shared configuration (use
Config.DBPEDIA_SPOTLIGHT_URL or an equivalent environment-backed config value)
and remove the hardcoded literal, and ensure any deployment-specific DNS
override logic (the dns_cache/override_dns/new_getaddrinfo mechanism) is
provided centrally at application startup so the recommendation services inherit
the same DNS behavior rather than implementing ad-hoc URLs.
In `@proxy/Dockerfile`:
- Around line 1-7: Add a Docker HEALTHCHECK to the Dockerfile so orchestrators
can detect unhealthy NGINX workers: update the Dockerfile (the image line FROM
nginxinc/nginx-unprivileged:1.29.3-alpine / COPY and EXPOSE 8000 context) to
include a HEALTHCHECK instruction (using CMD-SHELL) that probes localhost:8000
(e.g., curl/wget --fail or ncat) with sensible parameters (interval, timeout,
retries); place it after the COPY/EXPOSE lines and ensure it exits non-zero on
failure so the container is marked unhealthy.
In `@webapp/Dockerfile`:
- Around line 20-32: The Dockerfile final stage (FROM
nginxinc/nginx-unprivileged:1.29.3-alpine) exposes port 4200 but lacks a
HEALTHCHECK; add a HEALTHCHECK instruction after ENV NGINX_ENTRYPOINT_QUIET_LOGS
and before or after EXPOSE 4200 that probes the running nginx (for example an
HTTP GET against localhost:4200 or the nginx status endpoint) with sensible
interval/retries/timeout settings so the container runtime can detect unhealthy
instances; ensure the check runs as the unprivileged user (USER 101) or uses a
simple curl/wget shell command available in the image, and reference the
existing ENV NGINX_ENTRYPOINT_QUIET_LOGS and EXPOSE 4200 in the commit so
reviewers can locate the change.
---
Duplicate comments:
In `@proxy/Dockerfile`:
- Line 2: Replace the floating image tag in the Dockerfile's FROM instruction
(currently "FROM nginxinc/nginx-unprivileged:1.29.3-alpine") with the
corresponding immutable digest; obtain the image digest using the Docker
registry manifest endpoint (or the provided verification snippet) and update the
line to use "FROM nginxinc/nginx-unprivileged@sha256:..." so the build is pinned
to the specific content-addressable image.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 2bc197af-7679-4e4f-bf78-3d51846d0e78
📒 Files selected for processing (7)
.github/workflows/build.yml.github/workflows/release.ymlcoursemapper-kg/concept-map/src/graph_db.pycoursemapper-kg/concept-map/src/services/annotation.pyproxy/Dockerfilewebapp/Dockerfilewebserver/src/controllers/knowledgeGraph.controller.js
| steps: | ||
| - name: Checkout repository | ||
| uses: actions/checkout@v4 | ||
| uses: actions/checkout@v6 |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify that v6 exists and retrieve the exact commit SHA to pin.
# Read-only commands only.
set -euo pipefail
echo "Latest checkout tags (top 20):"
curl -fsSL "https://api.github.com/repos/actions/checkout/tags?per_page=20" | jq -r '.[].name'
echo
echo "Resolve v6 to commit SHA:"
curl -fsSL "https://api.github.com/repos/actions/checkout/git/ref/tags/v6" | jq -r '.object.sha'Repository: ude-soco/CourseMapper-webserver
Length of output: 172
🏁 Script executed:
#!/bin/bash
# Check if the workflow file exists and examine line 14 and surrounding context
if [ -f ".github/workflows/release.yml" ]; then
echo "=== Workflow file found ==="
echo "Lines 1-20 of .github/workflows/release.yml:"
head -20 ".github/workflows/release.yml" | cat -n
echo
echo "=== Full file (checking for permissions declarations) ==="
cat ".github/workflows/release.yml"
else
echo "Workflow file not found at .github/workflows/release.yml"
# List what's in .github/workflows if it exists
if [ -d ".github/workflows" ]; then
echo "Contents of .github/workflows/:"
ls -la ".github/workflows/"
fi
fiRepository: ude-soco/CourseMapper-webserver
Length of output: 1626
🌐 Web query:
actions/checkout v6 GitHub Actions tag
💡 Result:
The GitHub Actions actions/checkout v6 tag refers to the major version 6 of the official checkout action, used to check out repositories in workflows. The latest release under v6 is v6.0.2, released on January 9, 2026. Key details: - Tag: actions/checkout@v6 or specifically actions/checkout@v6.0.2 - Main changes in v6.0.0: Improved credential security by storing persist-credentials in $RUNNER_TEMP instead of .git/config (requires Actions Runner v2.329.0+ for Docker containers). No workflow changes needed for most users; git commands like fetch/push still work. - Usage example: - uses: actions/checkout@v6 with: repository: 'owner/repo' # optional ref: 'branch-or-tag' # optional - Requires Node.js 24 runtime support. - Official repository: https://github.com/actions/checkout (50 releases total, latest v6.0.2). As of 2026-04-21, v6 is the current major version.
Citations:
- 1: https://github.com/actions/checkout/tree/v6.0.0
- 2: https://github.com/actions/checkout
- 3: https://github.com/actions/checkout?tab=readme-ov-file
- 4: https://github.com/actions/checkout/blob/v6.0.2/CHANGELOG.md
- 5: https://github.com/actions/checkout/blob/HEAD/CHANGELOG.md
- 6: https://github.com/actions/checkout/releases
- 7: https://github.com/actions/checkout/tags
🌐 Web query:
GitHub Actions security best practices pin commit SHA mutable version tags
💡 Result:
GitHub Actions security best practices emphasize pinning actions to full-length commit SHAs instead of mutable version tags (like @v4 or @main) to prevent supply chain attacks where tags can be force-pushed to malicious commits. Key practices: - Use full 40-character SHA: - uses: actions/checkout@11bd719 # v4.2.2 Avoid: - uses: actions/checkout@v4 (mutable tag) - Verify SHA from original repository, not forks. - Add version comment for readability. - Prioritize high-risk/third-party actions. - Use Dependabot/Renovate for automated SHA updates on new releases. - Enable organization/enterprise policies to enforce SHA pinning (fails unpinned workflows). - Pin sub-actions in custom actions. - Combine with least-privilege GITHUB_TOKEN, OIDC for cloud access, code scanning. This ensures immutability: SHAs cannot change without content alteration, mitigating risks shown in real attacks like tj-actions/changed-files (CVE-2025-30066). Official GitHub Docs: Pinning to full SHA is the only immutable reference method.
Citations:
- 1: https://docs.github.com/en/actions/reference/security/secure-use
- 2: https://docs.github.com/en/actions/security-for-github-actions/security-guides/security-hardening-for-github-actions?learn=getting_started&learnProduct=actions
- 3: https://docs.github.com/actions/security-for-github-actions/security-guides/security-hardening-for-github-actions
- 4: https://docs.github.com/en/actions/how-tos/security-for-github-actions/security-guides/security-hardening-for-github-actions
- 5: https://docs.github.com/en/actions/security-for-github-actions/security-guides/security-hardening-for-github-actions
- 6: https://docs.github.com/en/actions/how-tos/security-for-github-actions/security-guides/security-hardening-for-github-actions?learn=getting_started
- 7: https://www.romainlespinasse.dev/posts/github-actions-commit-sha-pinning/
- 8: https://www.stepsecurity.io/blog/pinning-github-actions-for-enhanced-security-a-complete-guide
- 9: https://www.codewrecks.com/post/github/github-sha-pinning/
- 10: https://github.blog/changelog/2025-08-15-github-actions-policy-now-supports-blocking-and-sha-pinning-actions/
Pin actions/checkout to an immutable commit SHA, not a floating major tag.
Line 14 uses @v6, a mutable major version tag. In workflows with contents: write permissions, this creates supply-chain risk—tags can be force-pushed to malicious commits. Pin to the full commit SHA of the v6 release and include the version in a comment for maintainability.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/release.yml at line 14, The workflow uses the mutable tag
actions/checkout@v6 which is a supply-chain risk; replace that tag with the full
immutable commit SHA for the v6 release (i.e., change uses: actions/checkout@v6
to uses: actions/checkout@<FULL_COMMIT_SHA>) and add a trailing comment with the
human-friendly tag/version (e.g., # v6.x.y) for maintainability so reviewers can
see which release the SHA corresponds to.
| socket.getaddrinfo = new_getaddrinfo | ||
|
|
||
| override_dns('api.dbpedia-spotlight.org', '134.155.95.34') | ||
| override_dns('api.dbpedia-spotlight.org', '134.155.98.34') |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Verify the new DBpedia Spotlight IP and service availability
echo "=== Checking DNS resolution for api.dbpedia-spotlight.org ==="
nslookup api.dbpedia-spotlight.org || dig api.dbpedia-spotlight.org
echo -e "\n=== Testing connectivity to new IP 134.155.98.34 ==="
curl -I --connect-timeout 10 https://134.155.98.34/ 2>&1 || echo "Direct IP connection failed"
echo -e "\n=== Testing DBpedia Spotlight API on new IP ==="
curl -s --connect-timeout 10 \
-H "Accept: application/json" \
-H "Host: api.dbpedia-spotlight.org" \
"https://134.155.98.34/en/annotate?text=Berlin&confidence=0.5&support=20" 2>&1
echo -e "\n=== Testing DBpedia Spotlight API via hostname ==="
curl -s --connect-timeout 10 \
-H "Accept: application/json" \
"https://api.dbpedia-spotlight.org/en/annotate?text=Berlin&confidence=0.5&support=20" 2>&1Repository: ude-soco/CourseMapper-webserver
Length of output: 1644
🏁 Script executed:
# Search for DBpedia Spotlight references in the codebase
rg -i "dbpedia" --type py -B2 -A2Repository: ude-soco/CourseMapper-webserver
Length of output: 50387
🏁 Script executed:
# Search for other DNS override mechanisms
rg "override_dns|socket\.getaddrinfo" --type py -B2 -A2Repository: ude-soco/CourseMapper-webserver
Length of output: 1416
🏁 Script executed:
# Look for other services that might use the DBpedia API
fd -e py -type f | xargs rg "dbpedia-spotlight" -lRepository: ude-soco/CourseMapper-webserver
Length of output: 784
Apply DNS override consistently across all DBpedia Spotlight services.
The IP address change to 134.155.98.34 is functional and the service is accessible. However, the DNS override mechanism is applied only in this service, while other recommendation services in the codebase (e.g., coursemapper-kg/recommendation/app/services/course_materials/kwp_extraction/dbpedia/concept_tagging.py) hardcode the full URL without the DNS override. This creates an inconsistency where different services may resolve to different IPs:
AnnotationService(this file): Usesoverride_dns()to mapapi.dbpedia-spotlight.org→134.155.98.34- Recommendation services: Hardcode
https://api.dbpedia-spotlight.org/%s/annotatewith default DNS resolution
Consider either:
- Centralizing the DNS override in a shared initialization module imported by all services
- Using environment variables or configuration to define the DBpedia Spotlight endpoint consistently across all services
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@coursemapper-kg/concept-map/src/services/annotation.py` at line 29, Apply the
DNS override consistently by centralizing the DBpedia Spotlight endpoint: create
a shared configuration (e.g., DBPEDIA_SPOTLIGHT_HOST/DBPEDIA_SPOTLIGHT_URL) or
shared init module that calls
override_dns('api.dbpedia-spotlight.org','134.155.98.34') and export the
canonical endpoint, then update annotation.py to import that shared init
(instead of calling override_dns inline) and change the other service that
hardcodes "https://api.dbpedia-spotlight.org/%s/annotate" (see
course_materials/.../kwp_extraction/dbpedia/concept_tagging.py) to use the
shared DBPEDIA_SPOTLIGHT_URL or host variable so all services (including
AnnotationService) resolve via the same override/config.
| # syntax=docker/dockerfile:1.5 | ||
| FROM node:22.1-slim as build | ||
| # syntax=docker/dockerfile:1.15 | ||
| FROM node:24.0-slim as build |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Retrieve digests for both Docker Hub images/tags
set -euo pipefail
fetch_digest () {
local repo="$1"
local tag="$2"
local token
token="$(curl -fsSL "https://auth.docker.io/token?service=registry.docker.io&scope=repository:${repo}:pull" | jq -r '.token')"
echo "Digest for ${repo}:${tag}"
curl -fsSI \
-H "Authorization: Bearer ${token}" \
-H "Accept: application/vnd.docker.distribution.manifest.v2+json" \
"https://registry-1.docker.io/v2/${repo}/manifests/${tag}" \
| awk 'BEGIN{IGNORECASE=1} /docker-content-digest/ {print $0}'
echo
}
fetch_digest "library/node" "24.0-slim"
fetch_digest "nginxinc/nginx-unprivileged" "1.29.3-alpine"Repository: ude-soco/CourseMapper-webserver
Length of output: 355
🏁 Script executed:
cat -n webapp/Dockerfile | head -25Repository: ude-soco/CourseMapper-webserver
Length of output: 859
Pin both base images to immutable digests.
Lines 2 and 20 use mutable tags (node:24.0-slim, nginxinc/nginx-unprivileged:1.29.3-alpine), which makes rebuilds non-deterministic. Update to:
- Line 2:
FROM node:24.0-slim@sha256:083430e81f23ca4f309c6de17614d20706ddd544b2adc71fb9fdd86e2371360a as build - Line 20:
FROM nginxinc/nginx-unprivileged:1.29.3-alpine@sha256:5aea7cc516b419e3526f47dd1531be31a56a046cfe44754d94f9383e13e2ee99
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@webapp/Dockerfile` at line 2, Replace the mutable base image tags in the
Dockerfile with the provided immutable digests: update the build stage FROM
reference (currently "node:24.0-slim" in the FROM line used by the build stage)
to the pinned digest
"node:24.0-slim@sha256:083430e81f23ca4f309c6de17614d20706ddd544b2adc71fb9fdd86e2371360a",
and update the final runtime FROM reference (currently
"nginxinc/nginx-unprivileged:1.29.3-alpine") to the pinned digest
"nginxinc/nginx-unprivileged:1.29.3-alpine@sha256:5aea7cc516b419e3526f47dd1531be31a56a046cfe44754d94f9383e13e2ee99"
so rebuilds are deterministic.
| // const response = await axios.get(url); | ||
| const response = await axios.get(url, { | ||
| headers: { | ||
| // Use your app name + version + contact (email or URL) | ||
| "User-Agent": "CourseMapper (coursemapper@example.com)" | ||
| }, | ||
| timeout: 10000 | ||
| }); |
There was a problem hiding this comment.
Replace placeholder contact in User-Agent and drop the dead comment.
Wikimedia's User-Agent policy requires a legitimate way to contact the operator; coursemapper@example.com is a reserved placeholder domain (RFC 2606) and is non-routable. Requests may be throttled or blocked once traffic grows, and there is no way for Wikimedia ops to reach you. Use a real maintainer/project email or the project URL, and remove the leftover // const response = await axios.get(url); line.
🛠️ Proposed fix
- // const response = await axios.get(url);
- const response = await axios.get(url, {
- headers: {
- // Use your app name + version + contact (email or URL)
- "User-Agent": "CourseMapper (coursemapper@example.com)"
- },
- timeout: 10000
- });
+ const response = await axios.get(url, {
+ headers: {
+ // App name + version + contact (email or project URL), per Wikimedia UA policy
+ "User-Agent": "CourseMapper/1.0 (+https://github.com/ude-soco/CourseMapper-webserver)",
+ },
+ timeout: 10000,
+ });📝 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.
| // const response = await axios.get(url); | |
| const response = await axios.get(url, { | |
| headers: { | |
| // Use your app name + version + contact (email or URL) | |
| "User-Agent": "CourseMapper (coursemapper@example.com)" | |
| }, | |
| timeout: 10000 | |
| }); | |
| const response = await axios.get(url, { | |
| headers: { | |
| // App name + version + contact (email or project URL), per Wikimedia UA policy | |
| "User-Agent": "CourseMapper/1.0 (+https://github.com/ude-soco/CourseMapper-webserver)", | |
| }, | |
| timeout: 10000, | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@webserver/src/controllers/knowledgeGraph.controller.js` around lines 515 -
522, Replace the dead commented axios call and update the User-Agent header used
in the axios.get request inside the code that performs the Wikimedia API fetch:
remove the leftover "// const response = await axios.get(url);" line and change
"User-Agent": "CourseMapper (coursemapper@example.com)" to a real contact string
(project URL or maintainer email) per Wikimedia policy (e.g., "CourseMapper/1.0
(+https://yourproject.example.com)" or include a real maintainer email) in the
axios.get call so Wikimedia can contact the operator.
Bumps nginxinc/nginx-unprivileged from 1.29.3-alpine to 1.31.0-alpine. --- updated-dependencies: - dependency-name: nginxinc/nginx-unprivileged dependency-version: 1.31.0-alpine dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com>
Bumps nginxinc/nginx-unprivileged from 1.29.3-alpine to 1.31.0-alpine. --- updated-dependencies: - dependency-name: nginxinc/nginx-unprivileged dependency-version: 1.31.0-alpine dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com>
Bumps [docker/setup-qemu-action](https://github.com/docker/setup-qemu-action) from 3 to 4. - [Release notes](https://github.com/docker/setup-qemu-action/releases) - [Commits](docker/setup-qemu-action@v3...v4) --- updated-dependencies: - dependency-name: docker/setup-qemu-action dependency-version: '4' dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com>
Bumps [docker/login-action](https://github.com/docker/login-action) from 3 to 4. - [Release notes](https://github.com/docker/login-action/releases) - [Commits](docker/login-action@v3...v4) --- updated-dependencies: - dependency-name: docker/login-action dependency-version: '4' dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com>
Bumps [docker/setup-buildx-action](https://github.com/docker/setup-buildx-action) from 3 to 4. - [Release notes](https://github.com/docker/setup-buildx-action/releases) - [Commits](docker/setup-buildx-action@v3...v4) --- updated-dependencies: - dependency-name: docker/setup-buildx-action dependency-version: '4' dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com>
Bumps [docker/build-push-action](https://github.com/docker/build-push-action) from 6 to 7. - [Release notes](https://github.com/docker/build-push-action/releases) - [Commits](docker/build-push-action@v6...v7) --- updated-dependencies: - dependency-name: docker/build-push-action dependency-version: '7' dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com>
Bumps [docker/metadata-action](https://github.com/docker/metadata-action) from 5 to 6. - [Release notes](https://github.com/docker/metadata-action/releases) - [Commits](docker/metadata-action@v5...v6) --- updated-dependencies: - dependency-name: docker/metadata-action dependency-version: '6' dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com>
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
|
|
||
| - name: Set up QEMU | ||
| uses: docker/setup-qemu-action@v3 | ||
| uses: docker/setup-qemu-action@v4 |
Check warning
Code scanning / CodeQL
Unpinned tag for a non-immutable Action in workflow or composite action Medium
|
|
||
| - name: Set up Docker Buildx | ||
| uses: docker/setup-buildx-action@v3 | ||
| uses: docker/setup-buildx-action@v4 |
Check warning
Code scanning / CodeQL
Unpinned tag for a non-immutable Action in workflow or composite action Medium
| - name: Login to container registry | ||
| if: ${{ inputs.push }} | ||
| uses: docker/login-action@v3 | ||
| uses: docker/login-action@v4 |
Check warning
Code scanning / CodeQL
Unpinned tag for a non-immutable Action in workflow or composite action Medium
| - name: Get tagging metadata | ||
| id: meta | ||
| uses: docker/metadata-action@v5 | ||
| uses: docker/metadata-action@v6 |
Check warning
Code scanning / CodeQL
Unpinned tag for a non-immutable Action in workflow or composite action Medium
|
|
||
| - name: Build and push container image | ||
| uses: docker/build-push-action@v6 | ||
| uses: docker/build-push-action@v7 |
Check warning
Code scanning / CodeQL
Unpinned tag for a non-immutable Action in workflow or composite action Medium
Bumps nginxinc/nginx-unprivileged from 1.31.0-alpine to 1.31.2-alpine. --- updated-dependencies: - dependency-name: nginxinc/nginx-unprivileged dependency-version: 1.31.2-alpine dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com>
Bumps nginxinc/nginx-unprivileged from 1.31.0-alpine to 1.31.2-alpine. --- updated-dependencies: - dependency-name: nginxinc/nginx-unprivileged dependency-version: 1.31.2-alpine dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com>
Bumps [actions/checkout](https://github.com/actions/checkout) from 6 to 7. - [Release notes](https://github.com/actions/checkout/releases) - [Changelog](https://github.com/actions/checkout/blob/main/CHANGELOG.md) - [Commits](actions/checkout@v6...v7) --- updated-dependencies: - dependency-name: actions/checkout dependency-version: '7' dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com>
Bumps nginxinc/nginx-unprivileged from 1.31.2-alpine to 1.31.3-alpine. --- updated-dependencies: - dependency-name: nginxinc/nginx-unprivileged dependency-version: 1.31.3-alpine dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com>
Bumps nginxinc/nginx-unprivileged from 1.31.2-alpine to 1.31.3-alpine. --- updated-dependencies: - dependency-name: nginxinc/nginx-unprivileged dependency-version: 1.31.3-alpine dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com>
9 participants
Branching strategy: Everything gets merged into
devfirst, then merged again intomain.