Summarisation improvements implementation by Niharika0306 · Pull Request #684 · IBM/project-ai-services

Niharika0306 · 2026-04-28T04:56:24Z

Proposal plan for this PR : #621

dharaneeshvrd · 2026-04-28T16:52:24Z

+
+    # Get actual token count from the input text
+    input_tokens = await asyncio.to_thread(
+        lambda: len(tokenize_with_llm(content_text, llm_endpoint))


Hope there are no limitation on the content size that this API can handle, please validate once

+1 to Dharaneesh's comment:
Hope there are no limitation on the content size that this API can handle, please validate once

dharaneeshvrd · 2026-04-29T04:12:45Z

+    minimum_output_tokens = int(settings.summarize.minimum_summary_words / settings.common.llm.token_to_word_ratio_en)
+
+    # Hard limit: input + prompt + minimum_output must fit in context
+    max_allowed_input_tokens = (
+        settings.common.llm.granite_3_3_8b_instruct_context_length -
+        settings.summarize.summarization_prompt_token_count -
+        minimum_output_tokens
+    )


I think these can be defined in global level based on current config. No need to calculate it for every request.

+1. Also, add the calculation equation in comments so there is no confusion later.

I agree. Will do.

addressed. It is defined at module level now since they are derived values.

dharaneeshvrd · 2026-04-29T04:17:28Z

+    level_config = getattr(settings.summarize.summarization_levels, summary_level)
+
+    # Calculate ideal output tokens for this level
+    ideal_output_tokens = int(


Seems this one also not depends on the input tokens which means we can define it global level.

Addressed as part of next comment.

dharaneeshvrd · 2026-04-29T04:22:00Z

+        (1 + settings.summarize.summarization_coefficient * level_config.multiplier)
+    )
+
+    if available_output_tokens < ideal_output_tokens:


Can you please explain with a comment on which scenario this can happen?
But wondering why are we comparing with a generic output tokens with output tokens based on current input?
I feel we need to calculate the ideal token also based on input and compare, if it available is still less than the ideal count, than we can print this message IMO.

yes, the calculation was complex and wrong before. Its corrected now.

dharaneeshvrd · 2026-04-29T04:25:31Z

+
+    # Calculate ideal target based on input tokens and level multiplier
+    base_target_tokens = int(input_tokens * settings.summarize.summarization_coefficient)
+    ideal_target_tokens = int(base_target_tokens * level_config.multiplier)


I feel this should be used and the warning can be printed here

dharaneeshvrd · 2026-04-29T04:27:18Z

+    max_possible_words = int(available_output_tokens * settings.common.llm.token_to_word_ratio_en)
+    max_words = min(max_words, max_possible_words)
+
+    # Add small buffer to max_tokens


add a comment on why this buffer is needed

dharaneeshvrd · 2026-04-29T04:32:10Z

+    return target_word_count, min_words, max_words, max_tokens
+
+
+def compute_target_and_max_tokens(input_tokens: int, input_word_count: int, summary_length: Optional[int]):


input_word_count seems unused

dharaneeshvrd · 2026-04-29T04:42:47Z

+    return target_word_count, min_words, max_words, max_tokens
+
+
+def compute_target_and_max_tokens(input_tokens: int, input_word_count: int, summary_length: Optional[int]):


can we use the new token calculation approach in this method as well?
Just wanted to see the default method also uses token based limit calculation.

manalilatkar · 2026-04-29T06:19:44Z

+
+    # Get actual token count from the input text
+    input_tokens = await asyncio.to_thread(
+        lambda: len(tokenize_with_llm(content_text, llm_endpoint))


+1 to Dharaneesh's comment:
Hope there are no limitation on the content size that this API can handle, please validate once

manalilatkar · 2026-04-29T06:31:29Z

+            f"automatic length: {target_words} words"
+        )
+
+    messages = build_messages(content_text, target_words, min_words, max_words, has_length_spec)


Instead of adding a new variable called has_length_spec, this line can just be:
messages = build_messages(content_text, target_words, min_words, max_words, (summary_length is not None or summary_level is not None) )

manalilatkar · 2026-04-29T06:32:25Z

      "|-------|------|----------|-------------|\n"
      "| `text` | string | Yes | Plain text content to summarize |\n"
-      "| `length` | integer | No | Desired summary length in words  |\n"
+      "| `summary_level` | string | No | Abstraction level: 'brief', 'standard' (default), or 'detailed' |\n"


Didn't we agree to change the name of the parameter from summary_level -> level
Also in line no 275, 276, 281, 297, 300, 303.

right, forgot to reflect the changes here. Will do.

manalilatkar · 2026-04-29T06:48:48Z

+    minimum_output_tokens = int(settings.summarize.minimum_summary_words / settings.common.llm.token_to_word_ratio_en)
+
+    # Hard limit: input + prompt + minimum_output must fit in context
+    max_allowed_input_tokens = (
+        settings.common.llm.granite_3_3_8b_instruct_context_length -
+        settings.summarize.summarization_prompt_token_count -
+        minimum_output_tokens
+    )


+1. Also, add the calculation equation in comments so there is no confusion later.

Signed-off-by: Niharika Gurram <niharika.gurram1@ibm.com>

manalilatkar · 2026-04-30T08:03:34Z

+            400, "INVALID_PARAMETER",
+            "Cannot specify both 'summary_level' and 'length'. Please use only one."
+        )
+


@Niharika0306, this block of code from line no 159 to 194 becomes much simpler and cleaner ( only 3 lines ) if you create functions validate_input_word_count and compute_target_and_max_tokens that accept both length and level. Just a suggestion.

yeah, refactored a bit. Now there's only one instance of functions in case of both length and level.

Signed-off-by: Niharika Gurram <niharika.gurram1@ibm.com>

Niharika0306 mentioned this pull request Apr 28, 2026

summmarization improvement implementation #646

Closed

Niharika0306 added the squad/usecases label Apr 28, 2026

Niharika0306 requested review from dharaneeshvrd and manalilatkar April 28, 2026 11:15

Niharika0306 force-pushed the summarisation_changes_rebased branch from 817a0af to 995253a Compare April 28, 2026 13:08

dharaneeshvrd reviewed Apr 29, 2026

View reviewed changes

manalilatkar requested changes Apr 29, 2026

View reviewed changes

Summarisation improvements implementation

ba2ba6e

Signed-off-by: Niharika Gurram <niharika.gurram1@ibm.com>

manalilatkar reviewed Apr 30, 2026

View reviewed changes

address review comments

8df02fc

Signed-off-by: Niharika Gurram <niharika.gurram1@ibm.com>

Niharika0306 force-pushed the summarisation_changes_rebased branch from 995253a to 8df02fc Compare April 30, 2026 12:11

		return target_word_count, min_words, max_words, max_tokens


		def compute_target_and_max_tokens(input_tokens: int, input_word_count: int, summary_length: Optional[int]):

Conversation

Niharika0306 commented Apr 28, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

manalilatkar Apr 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

manalilatkar Apr 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

manalilatkar Apr 29, 2026 •

edited

Loading

manalilatkar Apr 29, 2026 •

edited

Loading