feat(content-api): Enrichment/Transcript object model, CRUD APIs, and Kafka wiring - #1303
Merged
Conversation
Introduces a new Enrichment object type to track AI-generated content (transcripts, captions) linked to a content node.
Introduces a new Transcript object type for per-language transcript/caption records, with a Draft/Processing/Review/Live/Failed lifecycle status.
Adds the Enrichment relation to content's config.json so a content node can be linked to its Enrichment node.
Adds createTranscript/updateTranscript/approveTranscript/rejectTranscript actor handlers and their manager implementation, wired through ContentActor and ContentController.
Adds transcript create/update/approve/reject routes and the enriched-metadata/media-transcription/media-multilingual kafka topic config to both content-service and knowlg-service.
Relation traversal attributes (e.g. for Transcript, Enrichment) can now be declared per object type via relationFields in its config.json, instead of only the hardcoded framework/default lists. Falls back to the previous defaults if relationFields isn't set or the schema fetch fails, so existing object types are unaffected.
…cript config Confirmed live: FrameworkValidator (mixed into every object type's generic node-creation validation chain) unconditionally reads frameworkCategories from that object type's own config.json via getStringList — missing the key entirely throws ConfigException.Missing immediately on node creation. Content's own config.json already declares this (as a real category list); Enrichment and Transcript don't use framework-based categorization, so an empty list is correct here.
… update, not Enrichment's create Confirmed live: "associatedTo is not allowed between Content and Enrichment". Relation validation (AssociationRelation.validateObjectTypes) checks the edge against the CURRENT node's own outRelationObjectTypes — built by BaseDefinitionNode.relationsSchema, which filters strictly on each relation's declared "direction". Enrichment's "usedByContent" is direction "in", so it's excluded from Enrichment's own out-relations list by definition, regardless of config content — adding it via Enrichment's own create request can never pass. Content's config.json already declares "enrichment" as direction "out", so the edge must originate from a Content update instead, which findOrCreateEnrichment now does as a follow-up step after creating the Enrichment node.
Confirmed live: setting primaryCategory (even via a default value) triggers a lookup against the platform's ObjectCategoryDefinition external-store for that (objectType, primaryCategory) combination — which doesn't exist for these internal node types and isn't meant to (no other simple node type — license, term, channel — declares primaryCategory at all). Enrichment/Transcript are internal AI-pipeline bookkeeping nodes, not user-facing categorizable content, so they shouldn't participate in that system either.
… schemaName
Confirmed live: all 4 transcript endpoints (create/update/approve/reject)
used body.getOrDefault(schemaName, ...) to pull the request payload out
of the JSON body — but schemaName is ContentController's class-level
field, hardcoded to "content". Since the documented/actual request
shape is {"request": {"transcript": {...}}}, this silently returned an
empty map every time, discarding the entire payload (artifactUrl,
languageCode, etc.) before it ever reached the actor — surfaced as
"artifactUrl is required and must be an http/https URL" even though a
valid artifactUrl was sent.
Confirmed live: source-language transcripts are created before their actual language is known (that's the whole point of transcription — faster-whisper detects it), so requiring languageCode/language at creation time fails validation on the exact path that needs them left unset until later. Target-language transcripts already supply a real languageCode at creation (transcript_approved.py), so this only affects source creation. Both fields stay in the schema and get filled in via transcript/update once faster-whisper detects the language.
…'s own update, not Transcript's create Confirmed live: "associatedTo is not allowed between Enrichment and Transcript" — identical root cause to the earlier Content->Enrichment fix. createTranscriptChildNode put "usedByEnrichment" (direction "in") on Transcript's own create request; relation validation checks the edge against the CREATING node's own outRelationObjectTypes, which excludes "in"-direction relations by definition. Enrichment's own config.json declares "transcripts" as direction "out", so the edge now gets added via a separate Enrichment update after the Transcript node is created — same pattern as findOrCreateEnrichment's Content update. Fixes both source and target-language transcript creation, since both paths share this function.
DataNode.read was called with an empty fields list for all 4 transcript endpoints, so node.getOutRelations() was never populated. readEnrichmentForContent relies on getOutRelations to find an existing Enrichment node, so it always returned None and findOrCreateEnrichment created a brand-new duplicate Enrichment node on every call instead of reusing the existing one.
content/v4/read's own 'enrichment' field is a denormalized snapshot taken when the Content->Enrichment edge was last touched, not a live join, so newly-linked Transcript nodes never show up there. This adds a dedicated read that fetches the Enrichment node directly (with its transcripts relation expanded) so callers can see current state without a raw JanusGraph query.
… objectType Hardcoding fields=['transcripts'] + filtering strictly on Transcript meant every future AI feature relation on Enrichment (e.g. quizzes) would need a code change here to show up. JanusGraphNodeUtil.getNode already fetches every out-edge regardless of the fields list, so grouping by endNodeObjectType instead makes this endpoint pick up new Enrichment relations automatically.
… file knowlg-service/conf/routes is a separate, manually-duplicated copy of content-api/content-service/conf/routes, not an include — the new GET /content/v4/enrichment/read/:id route only existed in the latter, so the actual deployed knowlg-service module 404'd on it.
JanusGraphNodeUtil.createRelation hardcodes getEndNodeMetadata()/ getStartNodeMetadata() to only ever contain description/status off the target vertex - confirmed via a direct JanusGraph query showing the edge itself carries zero properties. findSourceTranscriptRelation, findTranscriptRelationByLanguage and resolveTargetTranscriptRelation all filtered on sourceLanguage/languageCode read through that accessor, so they always fell back to the default and never matched - breaking updateTranscript, approveTranscript/rejectTranscript's default-target branch, and createTranscript's already-in-progress check. Replaced with readTranscriptChildren, which re-reads every Transcript child fully (same pattern syncEnrichmentTranscriptsFromNode already used), and pure findSourceTranscript/findTranscriptByLanguage/ resolveTargetTranscript helpers that filter on the real node metadata. Also fixed readEnrichment's children grouping the same way.
…ture.sequence Future.sequence needs BuildFrom for an immutable Seq; .asScala on a java.util.List yields a mutable.Buffer, which doesn't satisfy the declared Future[Seq[Node]] return type. Added .toSeq at each of the 3 call sites (readTranscriptChildren, readEnrichment, syncEnrichmentTranscriptsFromNode) - real compile error from CI, not caught locally since sbt/maven aren't available in this environment.
…hment read readEnrichment's 'enrichment' object included the denormalized 'transcripts' snapshot field (written by syncEnrichmentTranscriptsFromNode for isEcarReady's use) alongside the already-live 'children' section - same data shown twice, one stale and one live. Dropped from the 'enrichment' object; 'children' remains the single source of truth.
populateRelationMaps built every declared relationField key via rel.getEndNodeMetadata()/getStartNodeMetadata(), which is hardcoded in JanusGraphNodeUtil.createRelation to only ever contain description/status - every other key read as null and got included anyway, unlike serialize()'s top-level metadataMap which already filters nulls. Mirrors that existing filter here instead.
…maps" This reverts commit e71197f.
relationFields declared here get rendered via relationObjectAttributes for any relation pointing at Enrichment (Content's own 'enrichment' field in content/v4/read) - but Relation.getEndNodeMetadata() is hardcoded in JanusGraphNodeUtil.createRelation to only ever contain description/status, so every one of these keys always resolved to null and got shown anyway. Emptying the list stops the schema from promising fields this relation-read path can never populate.
Content->Enrichment is 1:1 by application logic (findOrCreateEnrichment finds-before-creates), but NodeUtil serializes every relation as a List regardless of actual cardinality - the shared graph engine has no cardinality concept and stays untouched. Unwraps 'enrichment' specifically in ContentActor's read/privateRead instead, Content-only. Also reworks readEnrichment: merges live child data directly onto the enrichment object under its real schema-declared field name (e.g. 'transcripts', via DefinitionNode.getRelationDefinitionMap's reverse relKey->fieldName map), replacing the stale snapshot at that same key, instead of a separate made-up 'children' wrapper keyed by objectType.
kafka.topics.enriched.metadata was hardcoded to sunbirddev.enriched.metadata, which does not match what enrichment-router actually consumes (dev.knowlg.enriched.content.metadata) - transcript/approve kafka publish (pushEnrichedMetadataApprovedEvent) would silently go nowhere. Added KAFKA_TOPICS_ENRICHED_METADATA override support (Typesafe Config env-substitution idiom - no such override existed anywhere in this file before). Devops repo still needs to actually set this env var and/or fix the default; the knowlg chart currently has no env-injection wired for this at all.
…opic" This reverts commit af7459b.
pushEnrichedMetadataApprovedEvent and pushTranscriptionRequestEvent built flat, ad-hoc JSON — inconsistent with the eid/ets/mid/actor/ context/object/edata envelope other jobs in knowledge-platform-jobs use for job-to-job events (e.g. VideoEnrichmentHelper.getStreamingEvent). Added a shared buildBeJobRequestEvent helper and switched both to it; business fields move into edata, channel into context.channel. The mirrored Python side (sunbird_ai_core.kafka.event_schemas) is updated in the same ai-pipeline change to parse this envelope. NOT compiled/run against a live stack in this session — verify via the project's normal sbt build + integration tests before merging.
Mirrors how mimeType is already sourced from the content node's own metadata instead of trusting the caller. Content node always has its own artifactUrl by the time a transcript is created against it, so requiring callers to pass it separately was redundant and inconsistent.
…richment join enrich=all or a comma list of relation field names (e.g. transcripts) merges the same live Content->Enrichment->Transcript join /content/v4/enrichment/read does into this response's enrichment key, avoiding a second round-trip. Omitted/blank enrich param is a no-op: no second-hop fetch, unchanged behavior for every existing caller.
ContentController.read gained a 4th param (enrich) in the previous commit; EventController extends it and had to match the new signature or the whole content-controllers module fails to compile.
One Transcript node is always exactly one language (languageCode is already a single string, sourceLanguage a per-node bool) — array made sense for Content.language (multi-language content) but was copied here without justification, and produced double-nested arrays on the write side (job wrote [name], schema/serialize treated it as multi-valued).
v3's read shares the same ContentActor.read as v4 (both dispatch the readContent action) — just needed the param threaded through v3's controller/routes the same way.
Both schemas require code but neither node type has a human author to
supply one, so it was just duplicating the generated identifier — dead
weight, no distinct meaning. Now: Enrichment -> {contentId}_enrichment,
Transcript -> {contentId}_{languageCode} (or _source before detection),
matching how code is actually used elsewhere on the platform (a
human-readable slug, not an identifier alias).
…ript fetchEnrichmentMetadata builds its maps from raw node.getMetadata, which never carries identifier (that's node.getIdentifier, normally injected by NodeUtil.serialize, bypassed here). Without it, a caller had no way to discover a specific Transcript's real id to pass as approveTranscript/ rejectTranscript's body-level transcriptId for anything but the default source transcript.
…ove/reject Was an optional body field read only by approve/reject (silently falling back to the source transcript if missing); updateTranscript didn't read it at all and always hit the source transcript regardless of intent. Moves transcriptId into the URL path (same nested-resource shape as getBookmarkHierarchy's :identifier/:bookmarkId) so the target is unambiguous at the routing layer — a request without it now fails to match the route at all, instead of silently acting on the wrong transcript.
Contributor
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
chethann007
marked this pull request as draft
August 3, 2026 11:21
…ment object APIs
Introduces /content/v4/object/{create,update,approve,reject} as the sole
generic entry points for any Enrichment child object, dispatched via a
Strategy-pattern EnrichmentObjectHandler registry (TranscriptObjectHandler
today; future types like Summary plug in without touching ContentController
or ContentActor). updateObject absorbs every graph mutation ai-pipeline's
jobs used to perform directly against JanusGraph — Draft->Processing,
Processing->Review/Live/Failed, target-language Draft creation, and the
ECAR-rebuild cascade — so job status reporting now happens exclusively
through these HTTP APIs.
chethann007
force-pushed
the
feat/video-transcription
branch
from
August 5, 2026 18:30
63e0ba7 to
9acab8e
Compare
…bject Groups all Enrichment-related routes under one /content/v4/enrichment/* segment, consistent with the existing /content/v4/enrichment/read route, instead of a bare /content/v4/object/* that didn't signal what it belonged to.
…ranscript child DefinitionNode.setRelationship treats a submitted relation-field value as the complete desired set for that relation name — anything not resubmitted is diffed into deletedRelations and removed. createTranscriptChildNode's Enrichment update only ever sent the newly-created Transcript's identifier, so linking a second (or third) language wiped every previously-linked Transcript out of Enrichment.transcripts, breaking multilingual generation (sourceTranscriptUrl came back empty since the source Transcript's own edge had been silently dropped by the most recent target-language Draft creation).
The exists-query allow-list predates the Enrichment node's existence in the
schema, so `{"exists": ["enrichment"]}` was rejected with ERR_INVALID_EXISTS
even though the underlying OpenSearch field is queryable.
…ed shape
buildTranscriptJson wrote {"start":"0","end":"2.5","text":"..."} (quoted
strings, no "id") for a human-edited transcript, but ai-pipeline's
segments_from_dicts expects {"id": int, "start": float, "end": float,
"text": str} per segment. If a human-edited transcript later became the
multilingual source (sourceTranscriptUrl), parsing it crashed with
KeyError('id'). Now emits the same shape as the AI-generated transcript.json.
chethann007
changed the title
chethann007
marked this pull request as ready for review
August 13, 2026 09:39
sntiwari1
reviewed
Aug 13, 2026
sntiwari1
left a comment
sntiwari1
left a comment
Contributor
There was a problem hiding this comment.
Automated review summary
Reviewed via /code-review (effort: high). This PR adds a generic Enrichment/Transcript object model (schemas, ~940-line TranscriptManager, 5 new ContentActor operations, Kafka wiring) to support the ai-pipeline transcription/translation flow.
Overall: the feature design (generic Enrichment CRUD, per-object-type handler registry, opt-in enrich join on read) is solid, but the new TranscriptManager has several correctness gaps around discarded Futures, unguarded nulls, a dead code path, and a read-then-write race on relation updates. There's also no test coverage for any of the new logic. Details inline below.
Findings
- Dead code / always-fails path —
isUploadacceptsfileUrl-only requests, butcreateFromUploadonly reads thefilekey, so that branch always throwsERR_MISSING_FILE. - Discarded Futures —
DataNode.update(...)results aren't chained/awaited in several spots (updateBySegmentEdit,createFromGeneration,createFromUpload), so DB write failures are silently swallowed and callers get a success response regardless. - Unchecked null → NPE —
buildTranscriptJson'sescapeJson(seg.text)NPEs on an explicit"text": nullsegment instead of failing with aClientException. - Kafka failure after successful DB write —
pushEnrichedMetadataApprovedEvent/pushTranscriptionRequestEventrun synchronously post-update with no try/catch; a Kafka outage turns an already-persisted approve into a client-visible 5xx. - Read-then-write race on relations —
createTranscriptChildNode/findOrCreateEnrichmentresubmit a snapshot of existing relations read earlier in the request; concurrent creates for the same content can silently drop each other's newly-linked Transcript. - Local temp file never deleted — the uploaded VTT file path in
createFromUploadis never cleaned up, unlike the siblinguploadTranscriptFileshelper'sfinally { ...delete() }. - No test coverage — none of the ~940 new lines in
TranscriptManageror the 5 newContentActoroperations have corresponding ScalaTest specs; existing specs were only touched to fix aread()signature change. - Hardcoded temp path —
requestObjectFormDatahardcodes"/tmp"instead of reading it viaPlatform, inconsistent withTranscriptManager's owncontent.upload.temp_locationconfig key.
🤖 Generated with Claude Code
…PE, unisolated Kafka publish, and a relation-write race
Several real bugs in TranscriptManager, all from PR review:
- createFromUpload: isUpload (createObject) routes here for either "file"
(multipart) or "fileUrl" (JSON body), but this only ever read "file" -
a fileUrl-only request always hit ERR_MISSING_FILE. Now downloads the
URL (via SafeUrlValidator + FileUtils.copyURLToFile) when "file" is
absent, and deletes that downloaded temp file in a finally block (a
real multipart "file" is the caller's own temp file, not ours to
delete).
- Several DataNode.update(...) calls had their returned Future discarded
instead of chained (updateBySegmentEdit, createFromGeneration's
aiFeatures update, createFromUpload's two updates,
syncEnrichmentTranscriptsFromNode's snapshot write,
syncAndMaybeBuildEcar's ecarUrl write) - a failed write in any of these
still produced a success response/return value, since nothing observed
the failed Future. All now flatMap/await before proceeding.
- buildTranscriptJson/buildVttContent: seg.getOrDefault("text", "") only
substitutes when the key is absent, not when it's present with an
explicit null value - escapeJson(null) NPE'd on String.replace instead
of failing as a validated 400. Now Option(seg.get("text")).getOrElse("").
- pushTranscriptionRequestEvent/pushEnrichedMetadataApprovedEvent: the
Kafka publish ran synchronously after the DB write it announces had
already succeeded, with no isolation - a broker hiccup turned an
already-successful write into a 5xx, and a client retry would then hit
ERR_TRANSCRIPT_IN_PROGRESS/ERR_TRANSCRIPT_NOT_IN_REVIEW, masking that
the original call actually worked. Both now catch and log instead of
propagating.
- createTranscriptChildNode: relied on an existingChildIds snapshot the
caller read earlier in the same request before writing
Enrichment.transcripts (DefinitionNode.setRelationship treats the
submitted list as complete, diffing anything unlisted into
deletedRelations) - two concurrent creates for different languageCodes
could each start from the same stale snapshot, and the second write
would silently drop the first's newly-linked Transcript. Now re-reads
the Enrichment node fresh immediately before this write instead,
narrowing (not fully eliminating - that needs real locking) the race
window from the whole request to just this one read-then-write.
requestObjectFormData hardcoded "/tmp" for the multipart upload temp
path, even though TranscriptManager reads the conceptually-equivalent
setting via Platform.getString("content.upload.temp_location",
"/tmp/content") a few classes over. Per configuration-discipline,
paths like this should go through Platform with a sensible default so
environments where /tmp isn't writable/desired can override it.
None of the ~940 lines in TranscriptManager or the 5 new ContentActor operations (create/update/approve/reject/readEnrichment) had any test coverage. The DataNode/graph/Kafka-dependent methods would need a real mocking harness this codebase doesn't have for Scala objects (PowerMock-style static mocking); scoping to what's actually testable today, this covers the pure helper functions instead - loosened from private to private[mgr] for that - including regression cases for the null-text NPE and isEcarReady's status-gating edge cases fixed alongside this.
…apshot write syncEnrichmentTranscriptsFromNode wrote a denormalized per-child snapshot (relationFields-scoped metadata, no "identifier" key) under the key "transcripts" on the Enrichment node. But Enrichment's own config.json declares "transcripts" as a real graph relation (relations.transcripts, direction out, objects [Transcript]) - writing plain metadata under that name gets misread as a relation-set update whose entries can't resolve an end node (no "identifier"), NPEing in AssociationRelation.validate. This write was always broken; it just silently failed before, because the previous commit's fix (awaiting DataNode.update's Future instead of discarding it) is what first surfaced it - every Transcript status update now failed with a 500 after this deploy, since updateCompletion/updateFailed both flatMap through syncAndMaybeBuildEcar -> syncEnrichmentTranscripts. Confirmed nothing depends on the persisted value: real reads (readEnrichment/fetchEnrichmentMetadata) reconstruct the equivalent view fresh from the actual relation every call, never from a stored snapshot. Stops attempting the write; keeps computing and returning the in-memory snapshot isEcarReady still needs.
pallakartheekreddy
approved these changes
Aug 14, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds a generic Enrichment/Transcript object model to knowlg-service and content-api — the platform-side counterpart to ai-pipeline's new
caption-generator/enrichment-routerFlink jobs. A Content node gets one Enrichment node holding typed AI-derived child objects (Transcript today), with generic (not per-object-type) CRUD APIs and Kafka events driving the transcription/multilingual-translation pipeline.Key changes
EnrichmentandTranscriptnode schemas (schemas/enrichment/1.0,schemas/transcript/1.0) and theContent -> Enrichmentgraph relation.create/update/approve/reject) under/content/v4/enrichment/object/*, replacing an earlier per-object-type Transcript API design — same 4 endpoints work for any future child type.GET /content/v4/enrichment/read/:idand an opt-inenrichparam oncontent/v3|v4/readthat embeds a live Enrichment join.BE_JOB_REQUESTKafka envelope emitted on Enrichment/Transcript lifecycle events, driving ai-pipeline's transcription and multilingual dispatch.enriched.metadatakafka topic support inapplication.conf.enrichmentto the exists-field allow-list so{"exists": ["enrichment"]}queries work.languagestring not array), transcript.json shape matching between human-edited and AI-generated captions — see individual commit messages.Test plan
transcription -> multilingual translation -> search)